-
Notifications
You must be signed in to change notification settings - Fork 26.4k
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
optimize performance for MetricsFilter #12329
optimize performance for MetricsFilter #12329
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix ci problem @wxbty PATL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, write a unit test by the way?
Codecov Report
@@ Coverage Diff @@
## 3.2 #12329 +/- ##
============================================
+ Coverage 67.38% 69.35% +1.96%
+ Complexity 22 2 -20
============================================
Files 3551 1607 -1944
Lines 153785 66267 -87518
Branches 23666 9719 -13947
============================================
- Hits 103629 45958 -57671
+ Misses 40460 15851 -24609
+ Partials 9696 4458 -5238 see 1983 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlbumenJ Good idea, Is this ok? Tested to improve performance by 2% than cache in ReflectionUtils |
…' into 3.2-add-metrics-event-type-cache
Would it be better to put the match method in util, and use Object for the event type? It is not the responsibility of the current class |
Okay, Moved |
|
||
public abstract class AbstractMetricsListener<E extends MetricsEvent> implements MetricsListener<E> { | ||
|
||
private final Map<Class<?>, Boolean> eventMatchCache = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ConcurrentHashMap here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
…' into 3.2-add-metrics-event-type-cache
Kudos, SonarCloud Quality Gate passed! |
ReflectionUtils add event type cache, reduce the time consumption of MetricsFilter
What is the purpose of the change
Brief changelog
Verifying this change
Checklist