Add corretto-8 to test matrix #1274

fandreuz · 2025-04-30T13:03:42Z

Description

In this PR I extend the test matrix to run Java integration tests on JDK 8.

Related issues

Fixes #1213

Motivation and context

We should test JDK 8 as well.

How has this been tested?

GHA

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

test/test/jfr/JfrTests.java

fandreuz · 2025-04-30T14:08:32Z

test/test/recovery/RecoveryTests.java


        out = p.profile("-d 3 -e cpu -o collapsed --safe-mode 31");
-        Assert.isGreater(out.ratio("unknown_Java"), 0.1);
+        Assert.isGreater(out.ratio("unknown_Java"), 0.05);


Does 0.1 have any special meaning? The smallest value I've seen so far is 0.08, but this feels like it should be a simple > 0 unless we have a specific value in mind

Can you show profile when the test fails?

test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg 4 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg 1 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg;java/lang/Integer.doubleValue 5 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg;java/lang/Float.doubleValue 1 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg 53 [unknown_Java] 27 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg;java/lang/Integer.doubleValue 6 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg;java/lang/Long.doubleValue 23 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg 54 test/recovery/Numbers.main;test/recovery/Numbers.loop 47 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg 52 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg;java/lang/Long.doubleValue 15 test/recovery/Numbers.main;test/recovery/Numbers.loop 7 test/recovery/Numbers.main;test/recovery/Numbers.loop;test/recovery/Numbers.avg 4

fandreuz · 2025-05-07T08:33:25Z

@apangin do you have any other input on the changes in this PR?

test/test/jfr/Ttsp.java

src/arguments.cpp

apangin · 2025-05-19T18:49:15Z

test/test/jfr/JfrTests.java

-    @Test(mainClass = JfrMultiModeProfiling.class, agentArgs = "start,all,alloc=100,file=%f.jfr", os = Os.LINUX)
-    @Test(mainClass = JfrMultiModeProfiling.class, agentArgs = "start,all,file=%f.jfr", os = Os.MACOS, arch = Arch.ARM64)
-    @Test(mainClass = JfrMultiModeProfiling.class, agentArgs = "start,all,lock=10ms,file=%f.jfr", os = Os.MACOS, arch = Arch.ARM64)
+    @Test(mainClass = JfrMultiModeProfiling.class, agentArgs = "start,all,file=%f.jfr", os = Os.LINUX, jvmVer = {11, Integer.MAX_VALUE})


Why 11+? Is it because of live profiling not supported on JDK 8?
I think --all should still work on JDK 8, although without live object profiling.

Yes, failure here: https://github.com/async-profiler/async-profiler/actions/runs/15116002786/job/42486783966#step:10:113

I'll fix the failure in this PR

test/test/jfr/Ttsp.java

fandreuz · 2025-05-20T09:18:22Z

All comments addressed

apangin · 2025-05-20T12:04:56Z

src/profiler.cpp


    if (_event_mask & EM_ALLOC) {
+        if (args._all && VM::hotspot_version() < 11) {
+            Log::debug("'live' option was quietly removed, only supported on OpenJDK 11+");


This is not accurate: _live is also supported with OpenJ9.
Why not checking _all in AllocTracer::check?

apangin · 2025-05-20T12:07:21Z

test/test/jfr/JfrTests.java

+    private static boolean checkJdkVersionEarlierThan11() {
+        String javaVersion = System.getProperty("java.version");
+        return javaVersion.startsWith("1.") || Integer.parseInt(javaVersion.split("\\.")[0]) < 11;
+    }


This returns the version of the host JDK (test runner), which may differ from the JDK version used by the test itself.
You may find the version from the JFR recording.

test/test/jfr/Ttsp.java

apangin · 2025-05-20T12:14:36Z

src/allocTracer.cpp

    if (args._live) {
+        // This engine is only going to be selected in Profiler::selectAllocEngine
+        // when can_generate_sampled_object_alloc_events is not available, i.e.
+        // JDK<11.


Just came across this yesterday: https://lkml.org/lkml/2020/5/29/1038
I'm fully with Linus on this issue.

From one extreme to another :)
There are no strict requirements how long a line should be. Just keep it in a shape that looks fine and reads fine.

// This engine is only going to be selected in Profiler::selectAllocEngine // when can_generate_sampled_object_alloc_events is not available, i.e. JDK<11.

jdk8

2dd28b9

fandreuz changed the title ~~JDK~~ Add corretto-8 to test matrix Apr 30, 2025

fandreuz commented Apr 30, 2025

View reviewed changes

test/test/jfr/JfrTests.java Outdated Show resolved Hide resolved

fandreuz changed the title ~~Add corretto-8 to test matrix~~ [DRAFT] Add corretto-8 to test matrix Apr 30, 2025

fandreuz force-pushed the sim-270-jdk8 branch from 1999daa to 2dd28b9 Compare April 30, 2025 13:49

lower

bf10255

fandreuz force-pushed the sim-270-jdk8 branch from 0c99ae0 to bf10255 Compare April 30, 2025 14:07

fandreuz commented Apr 30, 2025

View reviewed changes

fandreuz changed the title ~~[DRAFT] Add corretto-8 to test matrix~~ Add corretto-8 to test matrix Apr 30, 2025

fandreuz force-pushed the sim-270-jdk8 branch from d49e1ee to bf10255 Compare May 7, 2025 08:33

fandreuz added 3 commits May 7, 2025 08:43

trigger

f66e60b

1ms

0c9e77b

re-implement ttsp

399d167

fandreuz marked this pull request as draft May 19, 2025 14:36

fandreuz added 9 commits May 19, 2025 14:41

cc test

aea231c

Merge branch 'master' into sim-270-jdk8

3475d41

fix

ff08ee9

fix

8abcdf5

fix

895bf15

Merge branch 'fix-jfr-tests-java8' into sim-270-jdk8

11a106f

11+

b020f73

docs and check

e42ffd2

cc

32f764e

fandreuz commented May 19, 2025

View reviewed changes

test/test/jfr/Ttsp.java Show resolved Hide resolved

fandreuz marked this pull request as ready for review May 19, 2025 16:08

apangin reviewed May 19, 2025

View reviewed changes

src/arguments.cpp Show resolved Hide resolved

fandreuz added 2 commits May 19, 2025 18:27

duplicate

0b57c6c

Merge branch 'master' into sim-270-jdk8

1235c7f

apangin reviewed May 19, 2025

View reviewed changes

fandreuz added 2 commits May 20, 2025 00:19

fmt

363e56d

Merge branch 'master' into sim-270-jdk8

8e05af3

fandreuz marked this pull request as draft May 20, 2025 08:44

check live jdk11

f3560ee

fandreuz force-pushed the sim-270-jdk8 branch from d3784df to f3560ee Compare May 20, 2025 08:53

fix tests

1b0e019

fandreuz marked this pull request as ready for review May 20, 2025 09:18

free

8ad5e57

apangin reviewed May 20, 2025

View reviewed changes

fandreuz added 4 commits May 20, 2025 13:28

fmt

045908b

move to check

738677e

take from jfr

790b086

fix comment shape

e42181b

apangin merged commit 3bbab49 into async-profiler:master May 20, 2025
18 checks passed

roy-soumadipta pushed a commit to roy-soumadipta/async-profiler that referenced this pull request Jun 20, 2025

Add corretto-8 to test matrix (async-profiler#1274)

908a6a8

Add corretto-8 to test matrix #1274

Add corretto-8 to test matrix #1274

Uh oh!

Conversation

fandreuz commented Apr 30, 2025

Description

Related issues

Motivation and context

How has this been tested?

Uh oh!

Uh oh!

fandreuz Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fandreuz commented May 7, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fandreuz May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fandreuz commented May 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fandreuz Apr 30, 2025 •

edited

Loading

fandreuz May 20, 2025 •

edited

Loading