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

Switch DDScopeEvent ids to long and remove hex String caching #1550

Merged
merged 1 commit into from
Jun 10, 2020

Conversation

bantonsson
Copy link
Contributor

Switch the DDScopeEvent ids to long and remove hex String caching in DDId.

Based on top of #1538

@bantonsson bantonsson requested a review from a team as a code owner June 5, 2020 15:22
@bantonsson bantonsson self-assigned this Jun 5, 2020
@bantonsson bantonsson added the tag: performance Performance related changes label Jun 5, 2020
@randomanderson randomanderson requested a review from a team June 5, 2020 15:54
@randomanderson randomanderson changed the base branch from master to hdafgard/lean-scope-event June 5, 2020 15:55
@randomanderson
Copy link
Contributor

I think someone from the profiling team needs to approve this. If this affects backend processing. It's my understanding that ScopeEvent is written to the JFR recording.

I also changed the base branch to the PR to make the specific changes here clearer.

Copy link
Contributor

@hdafgard hdafgard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, and is verified as working with the profiling back-end.

@bantonsson
Copy link
Contributor Author

@randomanderson Sorry for not adding more context in the PR description. I should have mentioned that this had been discussed with the profiling team, and needs to wait until a backend that handles it is deployed. BTW, I actually prefer having master as the base branch, since it's there I will merge it, and only look at the relevant commit(s) in the files changed view instead.

@hdafgard Are you saying that it works with the currently deployed profiling backend, or the soon to be deployed profiling backend?

@hdafgard
Copy link
Contributor

hdafgard commented Jun 8, 2020

@hdafgard Are you saying that it works with the currently deployed profiling backend, or the soon to be deployed profiling backend?

Sorry, I meant to say that it works with the fix that is yet to be deployed.

@randomanderson randomanderson changed the base branch from hdafgard/lean-scope-event to master June 8, 2020 14:02
@bantonsson bantonsson force-pushed the bantonsson/scopevent-long-ids branch from ec1a501 to 5292d87 Compare June 10, 2020 13:19
@bantonsson bantonsson merged commit 8c63d72 into master Jun 10, 2020
@bantonsson bantonsson deleted the bantonsson/scopevent-long-ids branch June 10, 2020 14:29
@github-actions github-actions bot added this to the 0.55.0 milestone Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: performance Performance related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants