RFR: 7903740: JMH: Perf event validation not working with skid options [v6]
Aleksey Shipilev
shade at openjdk.org
Tue Sep 24 18:42:50 UTC 2024
On Fri, 6 Sep 2024 11:33:36 GMT, Galder Zamarreño <galder at openjdk.org> wrote:
>> Fixes https://bugs.openjdk.org/browse/CODETOOLS-7903740
>
> Galder Zamarreño has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
>
> - Add -q to avoid additional perf record messages
> - Pipe through perf report --stats instead
> - Merge branch 'master' into topic.validate-perf-event-without-modifier
> - Check all events with perf report in a single command
> - Remove previous approach
> - Use trim to remove any empty spaces or carriage returns
> - Do not make it quiet otherwise there's no output
> - Try perf report from specific file
> - Remove old approaches
> - Explicitly define an output file for perf record validation
> - ... and 5 more: https://git.openjdk.org/jmh/compare/ce58c088...069447e1
That is to say, I am willing to accept the simple patch like:
diff --git a/jmh-core/src/main/java/org/openjdk/jmh/profile/LinuxPerfAsmProfiler.java b/jmh-core/src/main/java/org/openjdk/jmh/profile/LinuxPerfAsmProfiler.java
index c948532f..69d74a86 100644
--- a/jmh-core/src/main/java/org/openjdk/jmh/profile/LinuxPerfAsmProfiler.java
+++ b/jmh-core/src/main/java/org/openjdk/jmh/profile/LinuxPerfAsmProfiler.java
@@ -46,7 +46,7 @@ public class LinuxPerfAsmProfiler extends AbstractPerfAsmProfiler {
public LinuxPerfAsmProfiler(String initLine) throws ProfilerException {
super(initLine, "cycles");
- String[] senseCmd = { PerfSupport.PERF_EXEC, "stat", "--event", Utils.join(requestedEventNames, ","), "--log-fd", "2", "echo", "1" };
+ String[] senseCmd = { PerfSupport.PERF_EXEC, "record", "-q", "--event", Utils.join(requestedEventNames, ","), "-o", "-", "echo", "1"};
Collection<String> failMsg = Utils.tryWith(senseCmd);
if (!failMsg.isEmpty()) {
...and everything else needs _a very hard justification and a lot of testing_ to make sure it does not break in the cases it is supposed to work. The fact the PR code fails on my first actual try does not inspire confidence, to be honest.
Remember: the capabilities check in constructor is an opportunistic check, it can pass by accident, and users would discover their profiler does not really work later when looking at the results. The check should _NOT_ fail by accident, when the profiler would actually work, if not for a misbehaving capability check.
-------------
PR Comment: https://git.openjdk.org/jmh/pull/132#issuecomment-2372028278
More information about the jmh-dev
mailing list