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

Improve metrics interceptor performance; avoid MetricID creations - 2.x #1602

Merged
merged 4 commits into from
Mar 31, 2020
Merged

Improve metrics interceptor performance; avoid MetricID creations - 2.x #1602

merged 4 commits into from
Mar 31, 2020

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented Mar 31, 2020

Resolves #1595

The existing metrics annotation interceptor creates a new MetricID each time an intercepted method is invoked, in order to help locate the relevant metric to update. This is expensive because the MetricID constructor consults config to look for global tags and an app identifier tag.

This PR avoids the expense of creating all those MetricID objects by:

  1. Allocating a single "dummy" MetricID to gather the global and app tags.
  2. Building and consulting maps -- one map for each metric type (counter, timer, etc.) -- that associates each annotationElement with the metric instance to be updated when that element is intercepted. On the first intercepted call for an element, the revised code:
    1. Retrieves from the Registry all metric IDs that share the same metric name.
    2. Finds the metric ID from those that matches the metric ID inferred from the intercepted element.
    3. Retrieves the corresponding metric from the registry.
    4. Adds that metric to the map.

Assumptions/possible issues:

  1. The Element is the correct unique identifier to use in the map to relevant metric. This seems right; remember that each metric type has its own map.
  2. Collecting the global and app tags from a single MetricID created when the Interceptor is instantiated is OK. Because config is not dynamic, this should be fine and the global and app tags obtained via config would not change as the app runs. The harvested tags should match the global and app tags in any MetricID created at any time during the app's lifetime.
  3. This PR makes public one existing package-private method on our Registry implementation class in the SE metrics project. This does increase the exposed surface area of our Registry but users should be using MetricRegistry and not Registry directly anyway. Also, using the newly-exposed method helps to streamline the code that locates and caches the metric associated with an interception point on the first usage.

Our tests and the TCK pass.

In informal experience with a new unit test, the time to execute 10,000 calls to an intercepted site dropped from 1.25 s to about .06 - .08 s.

…oking up the metric associated with each call to an intercepted call; instead, cache the metric with each annotation site
@tjquinno tjquinno added the 2.x Issues for 2.x version branch label Mar 31, 2020
@tjquinno tjquinno requested a review from spericas March 31, 2020 00:03
@tjquinno tjquinno self-assigned this Mar 31, 2020
@tjquinno tjquinno merged commit ab64814 into helidon-io:master Mar 31, 2020
@tjquinno tjquinno deleted the metric-id-fix-2.x branch March 31, 2020 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Issues for 2.x version branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

contention from MetricUtil.getMetricID() when using Metrics using annotations
2 participants