Skip to content

Conversation

@galderz
Copy link
Contributor

@galderz galderz commented May 31, 2024

Fixes https://bugs.openjdk.org/browse/CODETOOLS-7903740


Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

  • CODETOOLS-7903740: JMH: Perf event validation not working with skid options (Bug - P4)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmh.git pull/132/head:pull/132
$ git checkout pull/132

Update a local copy of the PR:
$ git checkout pull/132
$ git pull https://git.openjdk.org/jmh.git pull/132/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 132

View PR using the GUI difftool:
$ git pr show -t 132

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jmh/pull/132.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented May 31, 2024

👋 Welcome back galder! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented May 31, 2024

@galderz This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

7903740: JMH: Perf event validation not working with skid options

Reviewed-by: shade

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 1 new commit pushed to the master branch:

  • c2931c9: 7903722: JMH: Add xctrace-based perfnorm profiler for macOS

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@shipilev) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the rfr Pull request is ready for review label May 31, 2024
@mlbridge
Copy link

mlbridge bot commented May 31, 2024

@shipilev shipilev changed the title Remove perf event modifiers to validate if they are supported 7903740: Remove perf event modifiers to validate if they are supported May 31, 2024
@shipilev shipilev changed the title 7903740: Remove perf event modifiers to validate if they are supported 7903740: JMH: Perf event validation not working with skid options May 31, 2024
@mlbridge
Copy link

mlbridge bot commented Jun 1, 2024

Mailing list message from Ian Rogers on jmh-dev:

An alternative to cycles:p is to use cycles:P (note the case) which
means use the maximum precision level that is supported. It will give
more accurate sampling than cycles:p when possible and works with perf
stat:
```
$ sudo perf stat -e cycles:P true

Performance counter stats for 'true':

900,546 cycles:P

0.001261127 seconds time elapsed

0.001401000 seconds user
0.000000000 seconds sys
```

Thanks,
Ian

On Fri, May 31, 2024 at 10:53?AM Galder Zamarre?o <galder at openjdk.org> wrote:

@bridgekeeper
Copy link

bridgekeeper bot commented Jun 29, 2024

@galderz This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

Copy link
Member

@shipilev shipilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so the point for initialization checks to fail early, before we actually get to executing benchmarks. What would happen when event with option is not supported by perf record? Suppose events is supported and events:ppp is not. We check events and it works, we proceed to using events:ppp at runtime and fail.

Since there is an apparent discrepancy between what perf record and perf stat accept, I think we need to validate with perf record then. Something easy, like perf record -e ... echo 1 would likely work well?

@galderz
Copy link
Contributor Author

galderz commented Jul 25, 2024

@shipilev Makes sense, I'll look into validating that.

@galderz
Copy link
Contributor Author

galderz commented Aug 14, 2024

@shipilev Updated the PR with your proposed fix.

@shipilev
Copy link
Member

There are GHA test failures, not sure if they are due to this fix, please check?

@galderz
Copy link
Contributor Author

galderz commented Aug 15, 2024

Sure, checking...

@galderz
Copy link
Contributor Author

galderz commented Aug 16, 2024

Hmmm, I think the error is caused because profiler supported check now passes when running in CI, whereas in the past this tests didn't attempt to run.

Although you cannot see the Profiler is not supported or cannot be enabled, skipping test message in successful JMH CI runs (e.g. here), you can see that the test runs in 0.218 sec and so it must be passing because it sees the profiler is not supported.

With my change, it seems now the test thinks the profiler is supported, but then no secondary results are obtained.

I'm unsure how to deal with this yet.

@galderz
Copy link
Contributor Author

galderz commented Aug 16, 2024

@shipilev Are you sure these issues are also present in the main branch? I don't see any main branch CI runs and I don't see any recent CI jobs other than the one here.

I wonder if something has changed in the way CI environments run with regards to perf. In the May run I pointed above linux perf asm tests where not really executing. That means that perf stat --event cycles --log-fd 2 echo 1 was outright failing or it was returning <not supported>, but that seems to work fine now, see this test I've just run today.

It would be useful to have a way to see if main branch is really affected by the CI failures here. I can't see how it's not.

@galderz
Copy link
Contributor Author

galderz commented Aug 16, 2024

Actually my test shows the issue with switching to perf record. With perf stat we validate whether that event is supported and my test shows that it prints:

2024-08-16T07:50:50.3850275Z  Performance counter stats for 'echo 1':
2024-08-16T07:50:50.3850840Z 
2024-08-16T07:50:50.3851431Z    <not supported>      cycles                                                                
2024-08-16T07:50:50.3852100Z 
2024-08-16T07:50:50.3852392Z        0.002036823 seconds time elapsed
2024-08-16T07:50:50.3852825Z 
2024-08-16T07:50:50.3853024Z        0.000000000 seconds user
2024-08-16T07:50:50.3853730Z        0.002091000 seconds sys

Since not supported is in the output, the profiler test does not execute.

The perf record output on the other hand does not include that output so it assumes it's fine:

2024-08-16T07:50:51.5384315Z ##[group]Run perf record --event cycles echo 1
2024-08-16T07:50:51.5384952Z �[36;1mperf record --event cycles echo 1�[0m
2024-08-16T07:50:51.5451005Z shell: /usr/bin/bash -e {0}
2024-08-16T07:50:51.5451385Z ##[endgroup]
2024-08-16T07:50:51.6709500Z WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,
2024-08-16T07:50:51.6710972Z check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.
2024-08-16T07:50:51.6711978Z 
2024-08-16T07:50:51.6712829Z Samples in kernel functions may not be resolved if a suitable vmlinux
2024-08-16T07:50:51.6714013Z file is not found in the buildid cache or in the vmlinux path.
2024-08-16T07:50:51.6714727Z 
2024-08-16T07:50:51.6715312Z Samples in kernel modules won't be resolved at all.
2024-08-16T07:50:51.6715889Z 
2024-08-16T07:50:51.6724799Z PERFILE2��������@�������������������������������G����������������7�a����������������������������������������������������������������������������������������������������������������������������P�����T���������@���fv-az1567-205���������������������������������������������������P�����T���������@���6.5.0-1025-azure������������������������������������������������P�����T���������@���6.5.13����������������������������������������������������������P�����T���������@���x86_64����������������������������������������������������������P�����������������������P�����T���������@���AMD EPYC 7763 64-Core Processor���������������������������������P�����T�	�������@���AuthenticAMD,25,1,1���������������������������������������������P�������
2024-08-16T07:50:51.6727231Z If some relocation was applied (e.g. kexec) symbols may be misresolved
2024-08-16T07:50:51.6727979Z even with a suitable vmlinux or kallsyms file.
2024-08-16T07:50:51.6728384Z 
2024-08-16T07:50:51.6730695Z �������0�������P�������������������@���/usr/lib/linux-azure-6.5-tools-6.5.0-1025/perf������������������@���record����������������������������������������������������������@���--event���������������������������������������������������������@���cycles����������������������������������������������������������@���echo������������������������������������������������������������@���1���������������������������������������������������������������P����������������������������������������������G����������������7�a������������������������������������������������������������������������������������������������@���cpu-clock���������������������������������������������������������������������������������������P�����\�
2024-08-16T07:50:51.6733242Z �����������@���0-3�����������������������������������������������������������������@���0-1�������������������������������������������������������������@���2-3�������������������������������������������������������������������������������������������������@���0-3�����������������������������������������������������������������������������P�����l�����������������0���������������@���0-3�������������������������������������������������������������P����������������������@���cpu�����������������������������������������������������������������@���software��������������������������������������������������������	���@���uprobe��������������������������������������������������������������@���breakpoint����������������������������������������������������������@���tracepoint����������������������������������������������������������@���kprobe����������������������������������������������������������
2024-08-16T07:50:51.6734474Z Couldn't record kernel reference relocation symbol
2024-08-16T07:50:51.6735191Z Symbol resolution may be skewed if relocation was used (e.g. kexec).
2024-08-16T07:50:51.6735777Z Check /proc/kallsyms permission or run as root.
2024-08-16T07:50:51.8070024Z 1
2024-08-16T07:50:51.8071031Z [ perf record: Woken up 1 times to write data ]
2024-08-16T07:50:51.8073958Z ���@���msr�������������������������������������������������������������P����� �������������������������P�����X���������������������������������������������������������������������������������P�������������������P�������������������P�������������������P�������������������P������� �������O�����8���������l�W4����O"`{����������������������������E���������������������������������������c	������������������������������c	������������������������������c	������������������������������c	������N�����8�����������������cpu-clock�����������������������I�����(���������c	����������������������J���������������������P�����������������bpf_trampoline_6442559410�������������������������������������8�c	��c	��perf-exec�������������������������������R������������ 0�c	��c	��echo����c	��c	���,��-�����������
2024-08-16T07:50:51.8075631Z �����p�c	��c	������IV���@������� ��������������,��������le�������������/usr/bin/echo���c	��c	������-�����������
2024-08-16T07:50:51.8076240Z [ perf record: Captured and wrote 0.000 MB (null) ]
2024-08-16T07:50:51.8077114Z �������c	��c	���PY�`������������ ����������������������oٮ�������������/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2��c	��c	��0	��-�����������
2024-08-16T07:50:51.8078299Z �����h�c	��c	����������� ����������������������������������������������[vdso]��c	��c	���F��-�����������
2024-08-16T07:50:52.6748187Z �������c	��c	����"�`����P������������������������������J��G������������/usr/lib/x86_64-linux-gnu/libc.so.6�����c	��c	������-�����������	�����0�Y�F�����c	��c	��=���-�������������������	�����0�ǖ������c	��c	������-�������������������	�����0�ʻC�����c	��c	��ǚ��-�������������������������8�c	��`	��c	��`	���t��-���c	��c	���r��-�����������D�������

Looking at the perf record output I don't see any specific signal to see if perf record works for that event or not.

@galderz
Copy link
Contributor Author

galderz commented Aug 16, 2024

I'm playing around with this idea to see if it works: do a perf record and if that doesn't fail, see with perf report --stdio if any events are in the perf.data file.

@galderz
Copy link
Contributor Author

galderz commented Aug 16, 2024

I think my idea works.

On my linux box, for cycles, I get:

$ perf record --event cycles echo 1
1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.016 MB perf.data (7 samples) ]

$ perf report --stdio | grep 'cycles' | wc -l
1

For cycles:p I get:

❯ perf record --event cycles:p echo 1
1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.016 MB perf.data (8 samples) ]

$ perf report --stdio | grep 'cycles:p' | wc -l
1

Then on CI (see test) I see:

$ perf record --event cycles echo 1
...
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.000 MB (null) ]

$ perf report --stdio | grep 'cycles' | wc -l
incompatible file format (rerun with -v to learn more)
0

@galderz
Copy link
Contributor Author

galderz commented Aug 16, 2024

Ok, let's see what CI thinks about this.

It checks if the perf record command shows that no data was recorded. This only works if we explicitly tell perf record to record to an output file, otherwise it seems no perf.data is generated regardless, and the command output always shows that no data was captured.

@galderz
Copy link
Contributor Author

galderz commented Aug 16, 2024

Hmmmm that doesn't fully work on CI :\ (see output here). Doing further investigations.

@galderz
Copy link
Contributor Author

galderz commented Sep 6, 2024

Ian, I've pushed changes based on your feedback. I think that does the job very nicely.

@galderz
Copy link
Contributor Author

galderz commented Sep 6, 2024

Wooo hoo, all tests passing, @shipilev can you have another look?

@shipilev
Copy link
Member

Well, I just went to my m7g.16xlarge instance, and:

% java -jar benchmarks.jar -prof perfasm -f 1 JMHSample_35_Profilers.Atomic
 
Profilers failed to initialize, exiting.
Perf event count not a number for one of events [cycles]: [Can't parse sample, err = -14
0x224c [0x30]: failed to process type: 9
Error:
failed to process sample
0
]

It works without the patch. I think checking code is being too smart for its own good? Reiterating my previous suggestion: let's not try to parse the recordings, and instead "just" parse the output perf record -e incorrect-event-blah for testing the events.

@shipilev
Copy link
Member

shipilev commented Sep 24, 2024

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.

@galderz
Copy link
Contributor Author

galderz commented Oct 10, 2024

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.

Makes sense and I agree with you, but my previous efforts on this PR show that the CI is getting in the way of what you said above. E.g. see what happened in https://github.com/galderz/jmh/actions/runs/10384414105/job/28751371192, where the check passes by accident but then the profiler contains no data, so the test fails.

@galderz galderz closed this Oct 10, 2024
@galderz
Copy link
Contributor Author

galderz commented Oct 10, 2024

Closed the PR by mistake.

@galderz galderz reopened this Oct 10, 2024
@galderz
Copy link
Contributor Author

galderz commented Oct 10, 2024

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.

I will give your suggestion a go and see if LinuxPerfAsmProfilerTest passes on CI with that fix.

@galderz
Copy link
Contributor Author

galderz commented Oct 11, 2024

Just pushed this:

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.j>
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()) {

org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest runs fine in my local env:

Running org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 21.079 sec - in org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest

The time it takes to run the test hints that something actually happened, rather than skipping the test, but we can verify it in the test output:

$ cat ./target/surefire-reports/org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest-output.txt
...
Secondary result "org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest.work:asm":
PrintAssembly processed: 70607 total address lines.
Perf output processed (skipped 8.197 seconds):
 Column 1: cycles (2717 events)

Also, running with skid option works fine:

$ java -jar target/benchmarks.jar JMHSample_01 -f 1 -i 1 -r 1 -wi 1 -w 1 -prof perfasm:events=cycles:p
...
Secondary result "org.openjdk.jmh.samples.JMHSample_01_HelloWorld.wellHelloThere:asm":
PrintAssembly processed: 59617 total address lines.
Perf output processed (skipped 5.870 seconds):
 Column 1: cycles (cycles:p) (713 events)

Now let's wait and see what CI says.

@galderz
Copy link
Contributor Author

galderz commented Oct 11, 2024

@shipilev CI on GH actions doesn't like the patch:

Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 18.797 sec <<< FAILURE! - in org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest
test(org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest)  Time elapsed: 18.66 sec  <<< ERROR!
java.lang.IllegalStateException: Profile does not contain the required frame: PrintAssembly processed: 7 total address lines.
Perf output processed (skipped 6.249 seconds):
 Column 1: cycles (0 events)

WARNING: No hottest code region above the threshold (10.00%) for disassembly.
Use "hotThreshold" profiler option to lower the filter threshold.

....[Hottest Regions]...............................................................................
....................................................................................................
          <totals>

....[Hottest Methods (after inlining)]..............................................................
....................................................................................................
          <totals>

....[Distribution by Source]........................................................................
....................................................................................................
          <totals>

WARNING: The perf event count is suspiciously low (0). The performance data might be
inaccurate or misleading. Try to do the profiling again, or tune up the sampling frequency.
With some profilers on Mac OS X, System Integrity Protection (SIP) may prevent profiling.
In such case, temporarily disabling SIP with 'csrutil disable' might help.

	at org.openjdk.jmh.it.profilers.LinuxPerfAsmProfilerTest.test(LinuxPerfAsmProfilerTest.java:61)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:264)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:153)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:124)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)

Do you have suggestions on how to address this? This is what I've been battling with all throughout this patch, finding a way for the test to pass on both CI on GH actions and locally, and as you can see, it's not easy to find something that works in all environments.

@shipilev
Copy link
Member

shipilev commented Oct 11, 2024

Maybe the GH runner does not have PMU enabled? Given that it works in manual runs, I think it is fine if we just fix the test. It is not worth the hassle to fight GH infra, IMO. If there are no events gathered, don't fail the test. See how Windows perfasm test does it:

if (!checkDisassembly(out) && someEventsCaptured(out)) {

I am going to test the patch locally too.

@shipilev
Copy link
Member

I am going to test the patch locally too.

My local tests work well too. So the only thing left is to fix the test, and we are done!

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Oct 11, 2024
@galderz
Copy link
Contributor Author

galderz commented Oct 11, 2024

/integrate

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Oct 11, 2024
@openjdk
Copy link

openjdk bot commented Oct 11, 2024

@galderz
Your change (at version f569100) is now ready to be sponsored by a Committer.

@shipilev
Copy link
Member

/sponsor

@openjdk
Copy link

openjdk bot commented Oct 11, 2024

Going to push as commit 7c6da95.
Since your change was applied there has been 1 commit pushed to the master branch:

  • c2931c9: 7903722: JMH: Add xctrace-based perfnorm profiler for macOS

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Oct 11, 2024
@openjdk openjdk bot closed this Oct 11, 2024
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Oct 11, 2024
@openjdk
Copy link

openjdk bot commented Oct 11, 2024

@shipilev @galderz Pushed as commit 7c6da95.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

2 participants