[jdk17u-dev] RFR: 8275868: ciReplay: Inlining fails with "unloaded signature classes" due to wrong protection domains

[Martin Doerr]

On Wed, 27 Mar 2024 09:01:21 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

> I had to do some trivial resolves. 
> 
> I add some refactorings from "8262912: ciReplay: replay does not simulate unresolved classes"
> to the edited test files to make the test compile. I also had to fix the path to WhiteBox.
> 
> I also added two methods to query names to ciEnv from
> "8271911: replay compilations of methods which use JSR292 (easy cases)".
> this calls further into dyno_name(). I disabled this, it is not needed as long as 8262912 is not backported.
> 
> Test reproduces the issue as expected.
> 
> For follow up 8276227 see dependent pull request.

Looks basically good. Please see my suggestions.

src/hotspot/share/ci/ciEnv.cpp line 1201:

> 1199: // Returns the class name as a fallback if no location is found.
> 1200: const char *ciEnv::replay_name(const InstanceKlass* ik) const {
> 1201:   const char* name = nullptr; // dyno_name(ik);

I'd appreciate to have a comment why `dyno_name(ik)` is commented out. Maybe "JDK-8271911 is not in JDK 17, so we fall back to using the class name below."

test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java line 163:

> 161:             if (needCoreDump) {
> 162:                 // CiReplayBase$TestMain needs to be quoted because of shell eval
> 163:                 options.add("-XX:CompileOnly='" + TestMain.class.getName() + "::" + getTestMethod() + "'");

Shouldn't we use your new `getTestClass()` from your 2nd commit?

test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java line 169:

> 167:                                 ProcessTools.createTestJvm(options.toArray(new String[0]))));
> 168:             } else {
> 169:                 options.add("-XX:CompileOnly=" + TestMain.class.getName() + "::" + getTestMethod());

Same here.

-------------

PR Review: https://git.openjdk.org/jdk17u-dev/pull/2334#pullrequestreview-1963604200
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/2334#discussion_r1541295591
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/2334#discussion_r1541285548
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/2334#discussion_r1541286687