Skip to content
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

Adding in jfr thread statistics event. #20948

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adpopescu
Copy link
Contributor

Adding in the thread stats event. I noticed some issues with the thread park as well due to the removal of the index and count variables so I added those back in.

@adpopescu
Copy link
Contributor Author

@tajila Please review.
Thank you.

@tajila
Copy link
Contributor

tajila commented Jan 16, 2025

I noticed some issues with the thread park as well due to the removal of the index and count variables so I added those back in.

Which issues did you see?

@adpopescu
Copy link
Contributor Author

adpopescu commented Jan 16, 2025

For me, the event never actually appeared properly in JMC and it kept throwing an error. I'm assuming it's because the buffer that the data was being put in, was not the correct size and therefore some sections were overwritten or omitted. Regardless the buffer that's being used with calculateRequiredBufferSize() in JFRChunkWriter.hpp is still currently being used so I can't remove the indexes yet? I had forgotten to put the size of the thread park events into the buffer calculation. After this change it seems to appear correctly now.

@tajila
Copy link
Contributor

tajila commented Jan 16, 2025

Oh I see. _threadParkCount is being used, this shouldn't have changed. But addThreadParkEntry doesnt need to return anything, as the return value is never read.

@tajila
Copy link
Contributor

tajila commented Jan 16, 2025

Can you put the threadpark changes in a separate PR. We need to get those in ASAP for the next release. The threadstats changes are not as urgent.

@adpopescu
Copy link
Contributor Author

Ah yes I see that now... Ok I will change that back and put them in another PR.

@adpopescu adpopescu changed the title Adding in jfr thread stat event and fixes for thread park. Adding in jfr thread statistics event. Jan 16, 2025
@adpopescu
Copy link
Contributor Author

Removed the thread park fixes. Might have to rebase this PR once those are in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants