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

Cache toString() for MetricKey and MetricName in instance variables #123

Conversation

FuRyanf
Copy link

@FuRyanf FuRyanf commented Aug 29, 2024

This PR addresses the performance overhead caused by repeated string concatenation in the toString() method of the MetricName and MetricKey classes. The issue was identified as a significant contributor to CPU usage during metric processing in Beam’s Samza integration.

To mitigate this, the toString() results for MetricName and MetricKey are now cached. By storing the computed string representation on first access, we eliminate redundant concatenation operations in subsequent calls. This change reduces the computational cost associated with metrics processing, leading to improved performance in scenarios with frequent metric updates.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • [] Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • [] Update CHANGES.md with noteworthy changes.
  • [] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI.

@github-actions github-actions bot added the java label Aug 29, 2024
@xinyuiscool
Copy link

It seems quite unusual to add filed to a AutoValue. Instead, I think you can try this: https://github.com/google/auto/blob/main/value/userguide/howto.md#memoize_hash_tostring

Btw, can we also put this patch against open source? It's kind of core change, and oss have a lot of tests running against it.

Thanks.

@FuRyanf FuRyanf force-pushed the FuRyanf/cacheMetricKeyAndMetricNameToString branch from ded4303 to a7a4293 Compare August 30, 2024 00:04
@github-actions github-actions bot added the build label Aug 30, 2024
@github-actions github-actions bot removed the build label Aug 30, 2024
@FuRyanf FuRyanf closed this Sep 3, 2024
@FuRyanf FuRyanf reopened this Sep 3, 2024
@FuRyanf FuRyanf closed this Sep 3, 2024
@FuRyanf FuRyanf deleted the FuRyanf/cacheMetricKeyAndMetricNameToString branch September 13, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants