-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GR-48683] Add JFR event OldObjectSample. #8251
Conversation
* Sample objects and arrays that are possible leaks. * Sampling done in allocation slow path. * It uses a custom priority queue ordered by span. * The sampler uses a fixed-sized array at runtime to keep the samples in memory. By default this is 256 elements but it's configurable via `-XX:FlightRecorderOptions`. * Samples are scavenged when the queue is full. * Checking if objects are garbage collected is done using WeakReferences. * Added JFR repository for serializing and emitting old object information. * Added plain object and array tests. * Added tests that the sample queue to be full. * Added old object description tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good. Thanks for the thorough refactoring and in particular thanks for showing how the testing backdoor can be used. There are not many examples of that in the oracle/graal source tree unfortunately.
There are some style issues that should be fixed. |
One final thing @christianhaeubl, in case we want to backport this work to another GraalVM version, I was wondering if you're changes and my PR could be squashed and we put us both as co-authors? |
c3ea022
to
8420407
Compare
f924a0d
to
fd2ee1a
Compare
On our side, we prefer having individual commits. I guess it would make backporting easier if we got rid of the merge commit that is between your and my commit. However, for that I would need to rebase this branch, which changes the hash of your commit and puts me as a co-author on your commit. As far as I know, this is not preferred by Red Hat. |
Ok, let's leave it as is then. |
fd2ee1a
to
4b9a750
Compare
4b9a750
to
ce93064
Compare
ce93064
to
59c4c0b
Compare
@roberttoyonaga, @galderz : I pushed one more commit where I did the following changes:
Please double-check and let me know if you find any issues. |
59c4c0b
to
454f46e
Compare
All looks good, but I see CI still failing with style issues. |
454f46e
to
bd4970c
Compare
parseFlightRecorderLogging(); | ||
parseFlightRecorderOptions(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do have to do parse again here if we already did it in the previous hook?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I removed it.
bd4970c
to
dad3798
Compare
Thanks @christianhaeubl! |
Based on #6743.