Memory Mode: Adding 3rd and last part support for synchronous instruments - exponential histogram#6136
Conversation
…moryMode.java Co-authored-by: jack-berg <[email protected]>
…lder.java Co-authored-by: jack-berg <[email protected]>
…t1' into memory-mode-sync-instruments-part1
…t: Exponential Histogram
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6136 +/- ##
=========================================
Coverage 91.03% 91.04%
Complexity 5648 5648
=========================================
Files 619 619
Lines 16443 16443
Branches 1663 1663
=========================================
+ Hits 14969 14970 +1
Misses 1010 1010
+ Partials 464 463 -1 ☔ View full report in Codecov by Sentry. |
|
@jack-berg Ready for review |
|
The markdown links check fails, but I don't know how to fix it. |
jack-berg
left a comment
There was a problem hiding this comment.
Just one small recommendation.
| InstantiationException, | ||
| IllegalAccessException { | ||
| instrumentTester = | ||
| testInstrumentType.instrumentTesterClass.getDeclaredConstructor().newInstance(); |
There was a problem hiding this comment.
#nit: If you change instrumentTesterClass from Class<? extends InstrumentTester> to Supplier<InstrumentTester>, and configure each element in the TestInstrumentType enum with a factory method for creating new instances of InstrumentTester:
public enum TestInstrumentType {
ASYNC_COUNTER(AsyncCounterTester::new),
EXPONENTIAL_HISTOGRAM(ExponentialHistogramTester::new);
final Supplier<InstrumentTester> instrumentTesterSupplier;
TestInstrumentType(Supplier<InstrumentTester> instrumentTesterSupplier) {
this.instrumentTesterSupplier = instrumentTesterSupplier;
}
..
There was a problem hiding this comment.
Thanks. Much better.
I did it with an abstract method since error prone recommended it, and it does seem cleaner.
I also added the ProfileBenchmark and linked to it from the test
Added ProfileBenchmark.
|
@jack-berg All set |
Epic
This is the 4th PR out of several that implement #5105. See the complete plan here.
Specifically, this is the 3rd and last part that completes the implementation of
MemoryModefor the synchronous instrument: exponential histogram.What was done here?
InstrumentGarbageCollectionBenchmarkandInstrumentGarbageCollectionBenchmarkTest.I will convert it from draft to PR once the previous PR is merged.