Skip to content

Cache hash code of immutable values of type Tags & KeyValues #5881

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mstyura
Copy link
Contributor

@mstyura mstyura commented Feb 3, 2025

Tags objects are often used as keys in HashMap causing non-negligible time consumed in repeated computation of hash codes - since objects are immutable it's possible to compute and cache hash code values.
Similar caching technique is used inside JDK for a String objects. The key difference here I've decided to handle zero hash codes in a way which only adds 1 int filed to and object.

Copy link
Member

@jonatan-ivanov jonatan-ivanov left a comment

Choose a reason for hiding this comment

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

Tags objects are often used as keys in HashMap

Could you please show us where they are used directly?

Meter.Id is used as a key and it uses tags.hashCode() but I don't think Micrometer uses Tags or KeyValues as a key directly and I don't think users should do that either. Please show us a use-case if you disagree.

Because Meter.Id uses tags.hashCode(), I think the optimization might be valid, not sure about the performance impact, maybe we should check this with our JMH tests.

I guess KeyValue is not used like this but if we do this with Tags, I think it's god to keep them in-sync.

@mstyura
Copy link
Contributor Author

mstyura commented Feb 4, 2025

I was solely looking at micrometer thru datadog profiler and seen this (initially there was slightly different filter where I've missed this relationship between Id and Tags):
image
and I've missed that fact, that Tags.hashCode is actually called from Id.hashCode.
So to fix this 1% of total service CPU time spent id calculating hash codes it might make sense to cache the hash codes of Id instead of tags.
This high CPU usage might be due to the nature of service I've monitored, but I think micrometer as generic library can provide more optimal performance in such scenario.
The only unfortunate thing I see in implementing caching of hash code inside Meter.Id is that this class is not final and could it be the problem in case some tricky implemented ancestors?

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