-
Notifications
You must be signed in to change notification settings - Fork 621
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
fix: disable vt proto pools #3034
Conversation
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.
The code is simpler and the impact looks good. I only left one remark about a change in a generated file.
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 for getting some real numbers on this ❤️
LGTM, after we adapt the generation
Yeah, thanks for pointing this out guys! That's a leftover – we had a dependency in the JFR parser, so I hacked it quickly for testing and forgot to rollback. I removed the dependency and the generated code for pools. Next step is to write/revive our very own efficient pprof parser |
Should fix #3024
I tested distributors without VT protobuf pools and I think that it's probably better to disable them altogether:
Version w/o pools deployed at ~10:10:
The impact on GC is understood and expected: