-
Notifications
You must be signed in to change notification settings - Fork 566
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
Contention from io.helidon.metrics.Registry.getSortedMetrics() #1570
Comments
Sample Stack for the Counterjava.lang.Thread.State: BLOCKED (on object monitor) |
Sample Stack for the Timer
|
|
I have basic question . Why there is a need for sort, for each attempt to update the metric? We don't seem to have it when we use the Counter via the API ( counter=MetricRegistry.counter("name") , counter.inc() ) This is now created as a separate issue |
Repeated the same tests with the fix from #1581 from helidon-1.x branch. With the fix there is about the tests showed 20% improvement in Avg RT. Created the new issue #1595 for tracking this |
We some tests to understand the overhead of using metrics via both programmatic API's as well as annotations.
While didn't see much overhead while using programmatic API (MetricRegistry.counter, MetricRegistry.timer etc.) , we do see overhead while using annotations @timed , @ Counted etc.
The primarily contention is coming from the the implementation of synchronized method io.helidon.metrics.Registry.getSortedMetrics() , which is performing a sort using TreeMap. From the thread dumps, we see that most threads are BLOCKED on this method waiting for the monitor. The code path seems to end up sorting , all of them while holding the monitor.
private synchronized SortedMap<String, V> getSortedMetrics(MetricFilter filter, Class metricClass) {
Map<String, V> collected = allMetrics.entrySet()
.stream()
.filter(it -> metricClass.isAssignableFrom(it.getValue().getClass()))
.filter(it -> filter.matches(it.getKey(), it.getValue()))
.collect(Collectors.toMap(Map.Entry::getKey, it -> metricClass.cast(it.getValue())));
return new TreeMap<>(collected);
}
The text was updated successfully, but these errors were encountered: