-
Notifications
You must be signed in to change notification settings - Fork 307
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
7903455: JMH: Add "mempool" profiler #99
Conversation
👋 Welcome back ecaspole! A progress list of the required criteria for merging this PR into |
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.
This looks useful!
I think the whole thing can be simpler, though:
public class MemPoolProfiler implements InternalProfiler {
static final double BYTES_PER_KIB = 1024D;
@Override
public String getDescription() {
return "Memory pool/footprint profiling via standard MBeans";
}
@Override
public void beforeIteration(BenchmarkParams benchmarkParams, IterationParams iterationParams) {
for (MemoryPoolMXBean b : ManagementFactory.getMemoryPoolMXBeans()) {
b.resetPeakUsage();
}
}
@Override
public Collection<? extends Result> afterIteration(BenchmarkParams benchmarkParams, IterationParams iterationParams, IterationResult result) {
List<Result> results = new ArrayList<>();
long sumCodeHeap = 0L;
long sum = 0L;
for (MemoryPoolMXBean bean : ManagementFactory.getMemoryPoolMXBeans()) {
long used = bean.getPeakUsage().getUsed();
if (bean.getName().contains("CodeHeap")) {
sumCodeHeap += used;
}
sum += used;
results.add(new ScalarResult(Defaults.PREFIX + "mempool." + bean.getName() + ".used", used / BYTES_PER_KIB, "KiB", AggregationPolicy.MAX));
}
results.add(new ScalarResult(Defaults.PREFIX + "mempool.total.codeheap.used", sumCodeHeap / BYTES_PER_KIB, "KiB", AggregationPolicy.MAX));
results.add(new ScalarResult(Defaults.PREFIX + "mempool.total.used", sum / BYTES_PER_KIB, "KiB", AggregationPolicy.MAX));
return results;
}
}
In addition to this version being more readable, it also does only a single pass through the beans, which means sums and individual metrics would always agree.
Please also make sure to pass GHA tests: go to https://github.com/ericcaspole/jmh/actions and enable the workflows there. |
Thanks for your changes, it is better with your version. |
Webrevs
|
results.add(new ScalarResult(Defaults.PREFIX + "mempool.total.used", sum / BYTES_PER_KIB, "KiB", AggregationPolicy.MAX)); | ||
return results; | ||
} | ||
} |
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.
Need a new line at the end of file.
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.
Looks good, feel free to push once GHA are clean.
(If we need something else from this profiler, we can do that separately)
@ericcaspole This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
/integrate |
Going to push as commit 8716778. |
@ericcaspole Pushed as commit 8716778. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
I started off to show only the code cache related size, but I don't see any reason to not print all the available mbean pool info. We often adapt app or library code to run as JMH, to use all its benefits. In those cases seeing the code cache stats is more interesting than say, for a regular JDK API JMH benchmark. I verified the CodeHeap stats match well with +PrintCodeCache. There is mbean pool data for all the GCs as well.
Example output:
Benchmark (iterations) Mode Cnt Score Error Units
SpillCode.testSpillForManyInts 10 avgt 5 705.433 ± 9.570 ns/op
SpillCode.testSpillForManyInts:·CodeHeap 'non-nmethods'.used 10 avgt 5 1139.000 kB
SpillCode.testSpillForManyInts:·CodeHeap 'non-profiled nmethods'.used 10 avgt 5 209.750 kB
SpillCode.testSpillForManyInts:·CodeHeap 'profiled nmethods'.used 10 avgt 5 924.875 kB
SpillCode.testSpillForManyInts:·Compressed Class Space.used 10 avgt 5 403.375 kB
SpillCode.testSpillForManyInts:·G1 Eden Space.used 10 avgt 5 8192.000 kB
SpillCode.testSpillForManyInts:·G1 Old Gen.used 10 avgt 5 8160.000 kB
SpillCode.testSpillForManyInts:·G1 Survivor Space.used 10 avgt 5 ≈ 0 kB
SpillCode.testSpillForManyInts:·Metaspace.used 10 avgt 5 3617.320 kB
SpillCode.testSpillForManyInts:·Total CodeHeap.used 10 avgt 5 2267.000 kB
SpillCode.testSpillForManyInts:·Total MemPools.used 10 avgt 5 22639.695 kB
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmh.git pull/99/head:pull/99
$ git checkout pull/99
Update a local copy of the PR:
$ git checkout pull/99
$ git pull https://git.openjdk.org/jmh.git pull/99/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 99
View PR using the GUI difftool:
$ git pr show -t 99
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jmh/pull/99.diff
Webrev
Link to Webrev Comment