Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v6]
On Wed, 17 Jan 2024 19:52:32 GMT, Tom Rodriguez wrote: >> The changes for JDK-8287061 didn't update the SA decoding logic and there >> are other places where the decoding has gotten out of sync with HotSpot. >> Some of them can't be tested because they are part of JVMCI but I've added a >> directed test for the JDK-8287061 code and a more brute force test that >> tries to decode everything. > > Tom Rodriguez has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments I merged with master and reran all tests which were clean - PR Comment: https://git.openjdk.org/jdk/pull/17407#issuecomment-2086158736
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v7]
> The changes for JDK-8287061 didn't update the SA decoding logic and there are > other places where the decoding has gotten out of sync with HotSpot. Some of > them can't be tested because they are part of JVMCI but I've added a directed > test for the JDK-8287061 code and a more brute force test that tries to > decode everything. Tom Rodriguez 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 10 additional commits since the last revision: - Merge branch 'master' into tkr-hsdb-assert - Merge branch 'master' into tkr-hsdb-assert - Review comments - Remove testdebuginfodecode command - Update src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java Co-authored-by: Andrey Turbanov - Update src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java Co-authored-by: Andrey Turbanov - Pass the proper options to the lingered app - Fix problem list and correct jtreg arguments - Add missing files - 8318682: SA decoding of scalar replaced objects is broken - Changes: - all: https://git.openjdk.org/jdk/pull/17407/files - new: https://git.openjdk.org/jdk/pull/17407/files/f25c92ef..3cafefc6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17407=06 - incr: https://webrevs.openjdk.org/?repo=jdk=17407=05-06 Stats: 663952 lines in 9058 files changed: 132224 ins; 168721 del; 363007 mod Patch: https://git.openjdk.org/jdk/pull/17407.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17407/head:pull/17407 PR: https://git.openjdk.org/jdk/pull/17407
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v6]
On Wed, 17 Jan 2024 19:52:32 GMT, Tom Rodriguez wrote: >> The changes for JDK-8287061 didn't update the SA decoding logic and there >> are other places where the decoding has gotten out of sync with HotSpot. >> Some of them can't be tested because they are part of JVMCI but I've added a >> directed test for the JDK-8287061 code and a more brute force test that >> tries to decode everything. > > Tom Rodriguez has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments Let's merge this soon Tom. - PR Comment: https://git.openjdk.org/jdk/pull/17407#issuecomment-2082605948
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v6]
On Wed, 17 Jan 2024 19:52:32 GMT, Tom Rodriguez wrote: >> The changes for JDK-8287061 didn't update the SA decoding logic and there >> are other places where the decoding has gotten out of sync with HotSpot. >> Some of them can't be tested because they are part of JVMCI but I've added a >> directed test for the JDK-8287061 code and a more brute force test that >> tries to decode everything. > > Tom Rodriguez has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments Marked as reviewed by cslucas (Author). - PR Review: https://git.openjdk.org/jdk/pull/17407#pullrequestreview-1828474714
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
On Wed, 17 Jan 2024 20:02:44 GMT, Tom Rodriguez wrote: >> It's not -v that is making it do that. It does it without -v also. It can be >> a very slow operation, and I recently was running into cases where even >> after 10 minutes it was not done, so I gave up waiting. I'm about to file a >> CR to make it so it does not do that by default, and add a -l argument to >> optionally enable it. This will make it consistent with what bin/jstack is >> doing. I already have the fix ready. Just need to get the CR filed and and >> publish the PR. > > Ok. I was assuming that the `-v` option was like the `-l` option. Anyway, > TestDebugInfoDecode doesn't use jstack so it doesn't need the problem list > entry. I think generally speaking we problem list all SA tests with generational ZGC, but probably those that don't deal with the heap at all (which means they don't even use jstack) are ok. - PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456488468
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v6]
On Wed, 17 Jan 2024 19:52:32 GMT, Tom Rodriguez wrote: >> The changes for JDK-8287061 didn't update the SA decoding logic and there >> are other places where the decoding has gotten out of sync with HotSpot. >> Some of them can't be tested because they are part of JVMCI but I've added a >> directed test for the JDK-8287061 code and a more brute force test that >> tries to decode everything. > > Tom Rodriguez has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments Marked as reviewed by cjplummer (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17407#pullrequestreview-1828194783
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v6]
On Wed, 17 Jan 2024 19:52:32 GMT, Tom Rodriguez wrote: >> The changes for JDK-8287061 didn't update the SA decoding logic and there >> are other places where the decoding has gotten out of sync with HotSpot. >> Some of them can't be tested because they are part of JVMCI but I've added a >> directed test for the JDK-8287061 code and a more brute force test that >> tries to decode everything. > > Tom Rodriguez has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments So I can get any approvals for this? Testing was clean and is rerunning after the refactoring. - PR Comment: https://git.openjdk.org/jdk/pull/17407#issuecomment-1896672894
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
On Wed, 17 Jan 2024 19:52:47 GMT, Chris Plummer wrote: >> No. ClhsdbTestAllocationMerge is using `jstack -v` which doesn't work with >> ZGC because the `-v` part needs to iterate the heap looking for >> AbstractQueuedSynchronizers which doesn't work with ZGC and the SA. > > It's not -v that is making it do that. It does it without -v also. It can be > a very slow operation, and I recently was running into cases where even after > 10 minutes it was not done, so I gave up waiting. I'm about to file a CR to > make it so it does not do that by default, and add a -l argument to > optionally enable it. This will make it consistent with what bin/jstack is > doing. I already have the fix ready. Just need to get the CR filed and and > publish the PR. Ok. I was assuming that the `-v` option was like the `-l` option. Anyway, TestDebugInfoDecode doesn't use jstack so it doesn't need the problem list entry. - PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456412077
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
On Wed, 17 Jan 2024 19:44:38 GMT, Tom Rodriguez wrote: >> test/hotspot/jtreg/ProblemList-generational-zgc.txt line 108: >> >>> 106: serviceability/sa/sadebugd/ClhsdbAttachToDebugServer.java 8307393 >>> generic-all >>> 107: serviceability/sa/sadebugd/ClhsdbTestConnectArgument.java 8307393 >>> generic-all >>> 108: serviceability/sa/ClhsdbTestAllocationMerge.java 8307393 >>> generic-all >> >> Do you need to add TestDebugInfoDecode.java? > > No. ClhsdbTestAllocationMerge is using `jstack -v` which doesn't work with > ZGC because the `-v` part needs to iterate the heap looking for > AbstractQueuedSynchronizers which doesn't work with ZGC and the SA. It's not -v that is making it do that. It does it without -v also. It can be a very slow operation, and I recently was running into cases where even after 10 minutes it was not done, so I gave up waiting. I'm about to file a CR to make it so it does not do that by default, and add a -l argument to optionally enable it. This will make it consistent with what bin/jstack is doing. I already have the fix ready. Just need to get the CR filed and and publish the PR. - PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456400105
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
On Wed, 17 Jan 2024 19:34:29 GMT, Chris Plummer wrote:
>> Tom Rodriguez has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove testdebuginfodecode command
>
> test/hotspot/jtreg/serviceability/sa/TestDebugInfoDecode.java line 109:
>
>> 107: if (args == null || args.length == 0) {
>> 108: try {
>> 109: theApp = new LingeredApp();
>
> Is there a reason why previously you had used LingeredAppWithEnum and now you
> are using LingeredApp?
Both were chosen as a result of copy/paste from existing tests and it honestly
doesn't matter. The Xcomp is what's producing nmethods so the actual test
doesn't matter.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456392560
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
On Wed, 17 Jan 2024 17:35:39 GMT, Tom Rodriguez wrote: >> The changes for JDK-8287061 didn't update the SA decoding logic and there >> are other places where the decoding has gotten out of sync with HotSpot. >> Some of them can't be tested because they are part of JVMCI but I've added a >> directed test for the JDK-8287061 code and a more brute force test that >> tries to decode everything. > > Tom Rodriguez has updated the pull request incrementally with one additional > commit since the last revision: > > Remove testdebuginfodecode command I pushed the removal of the empty maps as well. - PR Comment: https://git.openjdk.org/jdk/pull/17407#issuecomment-1896561935
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v6]
> The changes for JDK-8287061 didn't update the SA decoding logic and there are > other places where the decoding has gotten out of sync with HotSpot. Some of > them can't be tested because they are part of JVMCI but I've added a directed > test for the JDK-8287061 code and a more brute force test that tries to > decode everything. Tom Rodriguez has updated the pull request incrementally with one additional commit since the last revision: Review comments - Changes: - all: https://git.openjdk.org/jdk/pull/17407/files - new: https://git.openjdk.org/jdk/pull/17407/files/07eefccd..f25c92ef Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17407=05 - incr: https://webrevs.openjdk.org/?repo=jdk=17407=04-05 Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17407.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17407/head:pull/17407 PR: https://git.openjdk.org/jdk/pull/17407
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
On Wed, 17 Jan 2024 19:40:03 GMT, Chris Plummer wrote: >> Tom Rodriguez has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove testdebuginfodecode command > > test/hotspot/jtreg/ProblemList-generational-zgc.txt line 108: > >> 106: serviceability/sa/sadebugd/ClhsdbAttachToDebugServer.java 8307393 >> generic-all >> 107: serviceability/sa/sadebugd/ClhsdbTestConnectArgument.java 8307393 >> generic-all >> 108: serviceability/sa/ClhsdbTestAllocationMerge.java 8307393 >> generic-all > > Do you need to add TestDebugInfoDecode.java? No. ClhsdbTestAllocationMerge is using `jstack -v` which doesn't work with ZGC because the `-v` part needs to iterate the heap looking for AbstractQueuedSynchronizers which doesn't work with ZGC and the SA. - PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456389516
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v4]
On Tue, 16 Jan 2024 22:47:04 GMT, Chris Plummer wrote: >> Tom Rodriguez has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Update >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java >> >>Co-authored-by: Andrey Turbanov >> - Update >> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java >> >>Co-authored-by: Andrey Turbanov > > test/hotspot/jtreg/serviceability/sa/ClhsdbTestAllocationMerge.java line 58: > >> 56: Map> expStrMap = new HashMap<>(); >> 57: Map> unExpStrMap = new HashMap<>(); >> 58: test.run(theApp.getPid(), cmds, expStrMap, unExpStrMap); > > You can just pass `null` for the two Map args, and no need to import > java.util.HashMap or java.util.Map. This still needs to be taken care of. - PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456380163
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
On Wed, 17 Jan 2024 17:35:39 GMT, Tom Rodriguez wrote:
>> The changes for JDK-8287061 didn't update the SA decoding logic and there
>> are other places where the decoding has gotten out of sync with HotSpot.
>> Some of them can't be tested because they are part of JVMCI but I've added a
>> directed test for the JDK-8287061 code and a more brute force test that
>> tries to decode everything.
>
> Tom Rodriguez has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove testdebuginfodecode command
test/hotspot/jtreg/ProblemList-generational-zgc.txt line 108:
> 106: serviceability/sa/sadebugd/ClhsdbAttachToDebugServer.java 8307393
> generic-all
> 107: serviceability/sa/sadebugd/ClhsdbTestConnectArgument.java 8307393
> generic-all
> 108: serviceability/sa/ClhsdbTestAllocationMerge.java 8307393
> generic-all
Do you need to add TestDebugInfoDecode.java?
test/hotspot/jtreg/serviceability/sa/TestDebugInfoDecode.java line 109:
> 107: if (args == null || args.length == 0) {
> 108: try {
> 109: theApp = new LingeredApp();
Is there a reason why previously you had used LingeredAppWithEnum and now you
are using LingeredApp?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456382492
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456375716
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v5]
> The changes for JDK-8287061 didn't update the SA decoding logic and there are > other places where the decoding has gotten out of sync with HotSpot. Some of > them can't be tested because they are part of JVMCI but I've added a directed > test for the JDK-8287061 code and a more brute force test that tries to > decode everything. Tom Rodriguez has updated the pull request incrementally with one additional commit since the last revision: Remove testdebuginfodecode command - Changes: - all: https://git.openjdk.org/jdk/pull/17407/files - new: https://git.openjdk.org/jdk/pull/17407/files/1a4e625e..07eefccd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17407=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17407=03-04 Stats: 223 lines in 3 files changed: 119 ins; 103 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17407.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17407/head:pull/17407 PR: https://git.openjdk.org/jdk/pull/17407
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v4]
On Tue, 16 Jan 2024 22:36:55 GMT, Chris Plummer wrote:
>> Tom Rodriguez has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Update
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java
>>
>>Co-authored-by: Andrey Turbanov
>> - Update
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java
>>
>>Co-authored-by: Andrey Turbanov
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java
> line 1644:
>
>> 1642: }
>> 1643: },
>> 1644: new Command("testdebuginfodecode", "testdebuginfodecode",
>> false) {
>
> This doesn't belong in clhsdb. You can do all of this directly in the test if
> you launch it properly.
> See for example `TestObjectAlignment.java`.
Yes that's much better. I've updated the test and renamed it. Please rereview.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1456153937
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v4]
On Mon, 15 Jan 2024 16:03:03 GMT, Tom Rodriguez wrote:
>> The changes for JDK-8287061 didn't update the SA decoding logic and there
>> are other places where the decoding has gotten out of sync with HotSpot.
>> Some of them can't be tested because they are part of JVMCI but I've added a
>> directed test for the JDK-8287061 code and a more brute force test that
>> tries to decode everything.
>
> Tom Rodriguez has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Update
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java
>
>Co-authored-by: Andrey Turbanov
> - Update
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java
>
>Co-authored-by: Andrey Turbanov
I can't really review the implementation details because this requires compiler
expertise I don't have. I did review the parts that were understandable without
compiler knowledge and made a few suggestions.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java line
1644:
> 1642: }
> 1643: },
> 1644: new Command("testdebuginfodecode", "testdebuginfodecode",
> false) {
This doesn't belong in clhsdb. You can do all of this directly in the test if
you launch it properly.
See for example `TestObjectAlignment.java`.
test/hotspot/jtreg/serviceability/sa/ClhsdbTestAllocationMerge.java line 58:
> 56: Map> expStrMap = new HashMap<>();
> 57: Map> unExpStrMap = new HashMap<>();
> 58: test.run(theApp.getPid(), cmds, expStrMap, unExpStrMap);
You can just pass `null` for the two Map args, and no need to import
java.util.HashMap or java.util.Map.
test/hotspot/jtreg/serviceability/sa/ClhsdbTestDebugInfodDecode.java line 52:
> 50: System.out.println("Started LingeredAppWithEnum with pid " +
> theApp.getPid());
> 51:
> 52: List cmds = List.of("testdebuginfodecode");
This will need to change given my suggestion to instead include the contents of
clshdb command within this test.
test/hotspot/jtreg/serviceability/sa/ClhsdbTestDebugInfodDecode.java line 58:
> 56: Map> expStrMap = new HashMap<>();
> 57: Map> unExpStrMap = new HashMap<>();
> 58: test.run(theApp.getPid(), cmds, expStrMap, unExpStrMap);
You can just pass null for the two Map args, and no need to import
java.util.HashMap or java.util.Map.
-
Changes requested by cjplummer (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17407#pullrequestreview-1825478257
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1454151509
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1454163583
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1454171208
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1454166498
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v4]
> The changes for JDK-8287061 didn't update the SA decoding logic and there are > other places where the decoding has gotten out of sync with HotSpot. Some of > them can't be tested because they are part of JVMCI but I've added a directed > test for the JDK-8287061 code and a more brute force test that tries to > decode everything. Tom Rodriguez has updated the pull request incrementally with two additional commits since the last revision: - Update src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java Co-authored-by: Andrey Turbanov - Update src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java Co-authored-by: Andrey Turbanov - Changes: - all: https://git.openjdk.org/jdk/pull/17407/files - new: https://git.openjdk.org/jdk/pull/17407/files/49477222..1a4e625e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17407=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17407=02-03 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17407.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17407/head:pull/17407 PR: https://git.openjdk.org/jdk/pull/17407
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v3]
On Mon, 15 Jan 2024 05:37:02 GMT, Tom Rodriguez wrote:
>> The changes for JDK-8287061 didn't update the SA decoding logic and there
>> are other places where the decoding has gotten out of sync with HotSpot.
>> Some of them can't be tested because they are part of JVMCI but I've added a
>> directed test for the JDK-8287061 code and a more brute force test that
>> tries to decode everything.
>
> Tom Rodriguez has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Pass the proper options to the lingered app
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/CommandProcessor.java line
1662:
> 1660: }
> 1661: if (blob instanceof NMethod) {
> 1662: NMethod nm = (NMethod) blob;
Suggestion:
NMethod nm = (NMethod) blob;
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/NMethod.java line 286:
> 284: for (Address p = scopesPCsBegin(); p.lessThan(scopesPCsEnd()); p =
> p.addOffsetTo(pcDescSize)) {
> 285: PCDesc pd = new PCDesc(p);
> 286: if (pd.getPCOffset() == -1) {
Suggestion:
if (pd.getPCOffset() == -1) {
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1452005032
PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1452004835
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v3]
> The changes for JDK-8287061 didn't update the SA decoding logic and there are > other places where the decoding has gotten out of sync with HotSpot. Some of > them can't be tested because they are part of JVMCI but I've added a directed > test for the JDK-8287061 code and a more brute force test that tries to > decode everything. Tom Rodriguez has updated the pull request incrementally with one additional commit since the last revision: Pass the proper options to the lingered app - Changes: - all: https://git.openjdk.org/jdk/pull/17407/files - new: https://git.openjdk.org/jdk/pull/17407/files/278377fd..49477222 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17407=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17407=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17407.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17407/head:pull/17407 PR: https://git.openjdk.org/jdk/pull/17407
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v2]
On Fri, 12 Jan 2024 21:46:13 GMT, Vladimir Kozlov wrote: >> Tom Rodriguez has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix problem list and correct jtreg arguments > > test/hotspot/jtreg/serviceability/sa/ClhsdbTestAllocationMerge.java line 36: > >> 34: * @requires vm.hasSA >> 35: * @library /test/lib >> 36: * @run main/othervm -XX:CompileThresholdScaling=0.01 >> -XX:CompileCommand=compileonly,compiler.c2.HeapDumper::testIt >> -XX:CompileCommand=exclude,compiler.c2.HeapDumper::dummy >> ClhsdbTestAllocationMerge > > `LingeredAppWithAllocationMerge` instead of `compiler.c2.HeapDumper` Those options are actually in the wrong place since they actually need to be passed to the lingered app but it also seems like they are wrong. I'd confirmed that the the test as written actually failed without the SA changes but when I added these arguments to the launch of the lingered app it no longer failed. So since it properly detects the problem without any arguments I'm just going to drop them from the @run directive. - PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1451932539
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken [v2]
> The changes for JDK-8287061 didn't update the SA decoding logic and there are > other places where the decoding has gotten out of sync with HotSpot. Some of > them can't be tested because they are part of JVMCI but I've added a directed > test for the JDK-8287061 code and a more brute force test that tries to > decode everything. Tom Rodriguez has updated the pull request incrementally with one additional commit since the last revision: Fix problem list and correct jtreg arguments - Changes: - all: https://git.openjdk.org/jdk/pull/17407/files - new: https://git.openjdk.org/jdk/pull/17407/files/c0618b46..278377fd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17407=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17407=00-01 Stats: 3 lines in 2 files changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17407.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17407/head:pull/17407 PR: https://git.openjdk.org/jdk/pull/17407
Re: RFR: 8318682: SA decoding of scalar replaced objects is broken
On Fri, 12 Jan 2024 21:26:36 GMT, Tom Rodriguez wrote: > The changes for JDK-8287061 didn't update the SA decoding logic and there are > other places where the decoding has gotten out of sync with HotSpot. Some of > them can't be tested because they are part of JVMCI but I've added a directed > test for the JDK-8287061 code and a more brute force test that tries to > decode everything. Thank you, Tom, for updating SA for new EA code. test/hotspot/jtreg/serviceability/sa/ClhsdbTestAllocationMerge.java line 36: > 34: * @requires vm.hasSA > 35: * @library /test/lib > 36: * @run main/othervm -XX:CompileThresholdScaling=0.01 > -XX:CompileCommand=compileonly,compiler.c2.HeapDumper::testIt > -XX:CompileCommand=exclude,compiler.c2.HeapDumper::dummy > ClhsdbTestAllocationMerge `LingeredAppWithAllocationMerge` instead of `compiler.c2.HeapDumper` - PR Review: https://git.openjdk.org/jdk/pull/17407#pullrequestreview-1819108372 PR Review Comment: https://git.openjdk.org/jdk/pull/17407#discussion_r1450953352
RFR: 8318682: SA decoding of scalar replaced objects is broken
The changes for JDK-8287061 didn't update the SA decoding logic and there are other places where the decoding has gotten out of sync with HotSpot. Some of them can't be tested because they are part of JVMCI but I've added a directed test for the JDK-8287061 code and a more brute force test that tries to decode everything. - Commit messages: - Add missing files - 8318682: SA decoding of scalar replaced objects is broken Changes: https://git.openjdk.org/jdk/pull/17407/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17407=00 Issue: https://bugs.openjdk.org/browse/JDK-8318682 Stats: 416 lines in 13 files changed: 400 ins; 4 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17407.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17407/head:pull/17407 PR: https://git.openjdk.org/jdk/pull/17407
