-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Avoid DistributionStatisticsConfig creation when retrieving timers #6661
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
Conversation
In the case where the Timer/LongTaskTimer builder is not used, the DistributionStatisticsConfig is necessarily one with the default configuration being passed to registerMeterIfNecessary. We can avoid creating and configuring a DistributionStatisticsConfig that will always have the same config by keeping a static default one to use in these cases. In performance critical areas the API to register Timer/LongTaskTimer that doesn't use the Builder should be preferred for this reason. The usage in DefaultMeterObservationHandler has been updated accordingly, which should improve the performance and allocations for all applications using it.
Benchmark/profiling analysis of the changesRetrieve existing timer benchmark - BeforeRetrieve existing timer benchmark - AfterThese changes save 8 nanoseconds per operation retrieving an existing timer (the method was renamed to Retrieve existing timer allocation profiling - BeforeRetrieve existing timer allocation profiling - AfterPerhaps interesting to note, you can see from the allocation profiling (and confirm with JIT compilation logs) that even before these changes there was no allocation for the Timer Builder itself - that allocation gets eliminated. It is two Doubles getting boxed for the minimum/maximum expected values config on the DistributionStatisticsConfig, its Builder, and itself getting allocated, which is avoided after these changes. The lambda getting allocated is a capturing lambda where the configured PauseDetector is being passed. In a separate PR from this, we can look at eliminating this in the case the default PauseDetector (no-op implementation) is being used. |
|
I did consider trying to eliminate creating a DistributionStatisticsConfig even when the Timer Builder is used, if nothing on it is called that changes the defaults for the DSC. While perhaps possible, it would be significantly more complicated than this change, partially because |
The capture was more than just the PauseDetector, which required more changes to avoid, but I've opened #6670 for this. |
In the case where the Timer/LongTaskTimer builder is not used, the DistributionStatisticsConfig is necessarily one with the default configuration being passed to registerMeterIfNecessary. We can avoid creating and configuring a DistributionStatisticsConfig that will always have the same config by keeping a static default one to use in these cases.
In performance critical areas the API to register Timer/LongTaskTimer that doesn't use the Builder should be preferred for this reason.
The usage in DefaultMeterObservationHandler has been updated accordingly, which should improve the performance and allocations for all applications using it.