Integrated: 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. This pull request has now been integrated. Changeset: b96b38c2 Author: Tom Rodriguez URL: https://git.openjdk.org/jdk/commit/b96b38c2c9a310d5fe49b2eda3e113a71223c7d1 Stats: 427 lines in 13 files changed: 413 ins; 4 del; 10 mod 8318682: SA decoding of scalar replaced objects is broken Reviewed-by: cjplummer, cslucas - 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 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: 8328135: javax/management/remote/mandatory/loading/MissingClassTest.java fails on libgraal
On Thu, 14 Mar 2024 10:46:10 GMT, Doug Simon wrote: > This PR increases a timeout in `MissingClassTest.java` to handle slight > slower compilation on a fastdebug build when using `-Xcomp`. > Testing on mach5 shows that the increase from 60 s to 90 s resolves the > timeouts. Marked as reviewed by never (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18297#pullrequestreview-1937156027
Re: RFR: 8327136: javax/management/remote/mandatory/notif/NotifReconnectDeadlockTest.java fails on libgraal
On Fri, 1 Mar 2024 17:24:02 GMT, Doug Simon wrote: > To account for slightly slower compile times on libgraal + fastdebug + > `-Xcomp`, this PR increases a timeout in `NotifReconnectDeadlockTest.java` > from 2000 ms to 3000 ms. > With this change, the test now reliably passes. Looks good - Marked as reviewed by never (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18085#pullrequestreview-1911730736
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: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 [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]
> 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]
> 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
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
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v5]
On Wed, 28 Jun 2023 21:21:11 GMT, Doug Simon wrote: >> The VMSupport class is required for translating an exception between the >> HotSpot and libgraal heaps. >> Loading it lazily can result in a loading exception, obscuring the exception >> being translated. >> To avoid this, VMSupport is loaded eagerly along with the other vmClasses. > > Doug Simon has updated the pull request incrementally with one additional > commit since the last revision: > > [skip ci] handle pending HotSpot exception closer to site causing exception That's more clear to me. Thanks! - Marked as reviewed by never (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14641#pullrequestreview-1504385398
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v4]
On Tue, 27 Jun 2023 21:29:53 GMT, Doug Simon wrote: >> The VMSupport class is required for translating an exception between the >> HotSpot and libgraal heaps. >> Loading it lazily can result in a loading exception, obscuring the exception >> being translated. >> To avoid this, VMSupport is loaded eagerly along with the other vmClasses. > > Doug Simon has updated the pull request incrementally with one additional > commit since the last revision: > > revert to lazy loading of VMSupport Right, I was forgetting about the virtual nature of this code. Why can't the pending exception handling be in the respective virtual? The partition between virtual and shared is kind of ugly. - PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1611888908
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v4]
On Tue, 27 Jun 2023 21:29:53 GMT, Doug Simon wrote:
>> The VMSupport class is required for translating an exception between the
>> HotSpot and libgraal heaps.
>> Loading it lazily can result in a loading exception, obscuring the exception
>> being translated.
>> To avoid this, VMSupport is loaded eagerly along with the other vmClasses.
>
> Doug Simon has updated the pull request incrementally with one additional
> commit since the last revision:
>
> revert to lazy loading of VMSupport
I don't think pushing it down that far improves things. encode always precedes
decode so why not resolve it before the encode call.
// Resolve VMSupport class explicitly as HotSpotJVMCI::compute_offsets
// may not have been called.
Klass* vmSupport =
SystemDictionary::resolve_or_fail(vmSymbols::jdk_internal_vm_VMSupport(), true,
THREAD);
int res = 0;
if (!HAS_PENDING_EXCEPTION) {
res = encode(THREAD, vmSupport, buffer, buffer_size);
}
src/hotspot/share/jvmci/jvmciEnv.cpp line 402:
> 400: }
> 401: int res = encode(THREAD, buffer, buffer_size);
> 402: if (_from_env != nullptr && !_from_env->is_hotspot() &&
> _from_env->has_pending_exception()) {
Why is this check before the `HAS_PENDING_EXCEPTION` check? Couldn't you end
up returning with a pending exception in this path?
-
PR Comment: https://git.openjdk.org/jdk/pull/14641#issuecomment-1610335668
PR Review Comment: https://git.openjdk.org/jdk/pull/14641#discussion_r1244461845
Integrated: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x
On Tue, 20 Jun 2023 18:05:09 GMT, Tom Rodriguez wrote: > This is a minor fix to core file reading on macos x. I can confirm that > after this fix I can run the problem listed SA core file tests on Ventura. This pull request has now been integrated. Changeset: 269852b9 Author: Tom Rodriguez URL: https://git.openjdk.org/jdk/commit/269852b90634aa43d4d719c93563608e42792fc6 Stats: 17 lines in 2 files changed: 5 ins; 0 del; 12 mod 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x Reviewed-by: cjplummer, sspitsyn - PR: https://git.openjdk.org/jdk/pull/14569
Re: RFR: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x [v4]
On Mon, 26 Jun 2023 18:09:24 GMT, Tom Rodriguez wrote: >> This is a minor fix to core file reading on macos x. I can confirm that >> after this fix I can run the problem listed SA core file tests on Ventura. > > Tom Rodriguez has updated the pull request incrementally with one additional > commit since the last revision: > > Update copyright Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/14569#issuecomment-1610129056
Re: RFR: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x [v4]
> This is a minor fix to core file reading on macos x. I can confirm that > after this fix I can run the problem listed SA core file tests on Ventura. Tom Rodriguez has updated the pull request incrementally with one additional commit since the last revision: Update copyright - Changes: - all: https://git.openjdk.org/jdk/pull/14569/files - new: https://git.openjdk.org/jdk/pull/14569/files/bd45a311..1eed618c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14569=03 - incr: https://webrevs.openjdk.org/?repo=jdk=14569=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14569.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14569/head:pull/14569 PR: https://git.openjdk.org/jdk/pull/14569
Re: RFR: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x [v2]
On Mon, 26 Jun 2023 05:06:13 GMT, Chris Plummer wrote:
>> Tom Rodriguez has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Flatten nested ifs
>> - Adjust ordering of message
>> - Adjust printing
>
> src/jdk.hotspot.agent/macosx/native/libsaproc/ps_core.c line 302:
>
>> 300: // The base of the library is offset by a random amount which
>> ends up as a load command with a
>> 301: // filesize of 0. This must be ignored otherwise the base
>> address of the library is wrong.
>> 302: if (segcmd.filesize != 0 && add_map_info(ph, fd, segcmd.fileoff,
>> segcmd.vmaddr, segcmd.vmsize, segcmd.flags) == NULL) {
>
> I actually preferred this part with two `if` statements. Seems easier to
> read that way.
Ok. I reverted it to 2 ifs.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14569#discussion_r1242483550
Re: RFR: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x [v3]
> This is a minor fix to core file reading on macos x. I can confirm that > after this fix I can run the problem listed SA core file tests on Ventura. Tom Rodriguez has updated the pull request incrementally with one additional commit since the last revision: Adjust for review - Changes: - all: https://git.openjdk.org/jdk/pull/14569/files - new: https://git.openjdk.org/jdk/pull/14569/files/2ebe4d09..bd45a311 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14569=02 - incr: https://webrevs.openjdk.org/?repo=jdk=14569=01-02 Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14569.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14569/head:pull/14569 PR: https://git.openjdk.org/jdk/pull/14569
Re: RFR: 8310829: guarantee(!HAS_PENDING_EXCEPTION) failed in ExceptionTranslation::doit [v3]
On Mon, 26 Jun 2023 13:17:25 GMT, Doug Simon wrote: >> The VMSupport class is required for translating an exception between the >> HotSpot and libgraal heaps. >> Loading it lazily can result in a loading exception, obscuring the exception >> being translated. >> To avoid this, VMSupport is loaded eagerly along with the other vmClasses. > > Doug Simon has updated the pull request incrementally with one additional > commit since the last revision: > > each exception translation failure should trigger a JVMCI event looks ok to me. - Marked as reviewed by never (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14641#pullrequestreview-1498967746
Re: RFR: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x [v2]
> This is a minor fix to core file reading on macos x. I can confirm that > after this fix I can run the problem listed SA core file tests on Ventura. Tom Rodriguez has updated the pull request incrementally with three additional commits since the last revision: - Flatten nested ifs - Adjust ordering of message - Adjust printing - Changes: - all: https://git.openjdk.org/jdk/pull/14569/files - new: https://git.openjdk.org/jdk/pull/14569/files/9852ed16..2ebe4d09 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14569=01 - incr: https://webrevs.openjdk.org/?repo=jdk=14569=00-01 Stats: 12 lines in 1 file changed: 4 ins; 5 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14569.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14569/head:pull/14569 PR: https://git.openjdk.org/jdk/pull/14569
Re: RFR: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x [v2]
On Wed, 21 Jun 2023 19:56:58 GMT, Chris Plummer wrote:
>> Tom Rodriguez has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Flatten nested ifs
>> - Adjust ordering of message
>> - Adjust printing
>
> src/jdk.hotspot.agent/macosx/native/libsaproc/ps_core.c line 309:
>
>> 307: print_debug("LC_SEGMENT_64 added: nsects=%d fileoff=0x%llx
>> vmaddr=0x%llx vmsize=0x%llx filesize=0x%llx %s\n",
>> 308: segcmd.nsects, segcmd.fileoff, segcmd.vmaddr,
>> segcmd.vmsize,
>> 309: segcmd.filesize, [0]);
>
> It would be nice to include this print_debug for the `filesize == 0` case.
> Maybe you can move it outside of the `if` and print `added` or `skipped`
> conditional on `filesize`.
Good idea. I've flattened the ifs a little and fixed the message.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14569#discussion_r1240227363
RFR: 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x
This is a minor fix to core file reading on macos x. I can confirm that after this fix I can run the problem listed SA core file tests on Ventura. - Commit messages: - 8294316: SA core file support is broken on macosx-x64 starting with macOS 12.x Changes: https://git.openjdk.org/jdk/pull/14569/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14569=00 Issue: https://bugs.openjdk.org/browse/JDK-8294316 Stats: 20 lines in 2 files changed: 7 ins; 3 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/14569.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14569/head:pull/14569 PR: https://git.openjdk.org/jdk/pull/14569
Integrated: 8218885: Restore pop_frame and force_early_return functionality for Graal
On Wed, 22 Sep 2021 05:40:40 GMT, Tom Rodriguez wrote: > This logic no longer seems to be necessary since the adjustCompilationLevel > callback has been removed. This pull request has now been integrated. Changeset: d61720a4 Author: Tom Rodriguez URL: https://git.openjdk.org/jdk/commit/d61720a4dc1b3a9c6f7c5e6a2b68fa2b7735d545 Stats: 8 lines in 1 file changed: 0 ins; 8 del; 0 mod 8218885: Restore pop_frame and force_early_return functionality for Graal Reviewed-by: kvn, dlong, sspitsyn, amenkov - PR: https://git.openjdk.org/jdk/pull/5625
Re: RFR: 8218885: Restore pop_frame and force_early_return functionality for Graal [v2]
> This logic no longer seems to be necessary since the adjustCompilationLevel > callback has been removed. Tom Rodriguez has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: 8218885: Restore pop_frame and force_early_return functionality for Graal - Changes: https://git.openjdk.org/jdk/pull/5625/files Webrev: https://webrevs.openjdk.org/?repo=jdk=5625=01 Stats: 8 lines in 1 file changed: 0 ins; 8 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/5625.diff Fetch: git fetch https://git.openjdk.org/jdk pull/5625/head:pull/5625 PR: https://git.openjdk.org/jdk/pull/5625
Re: RFR: 8218885: Restore pop_frame and force_early_return functionality for Graal
On Wed, 22 Sep 2021 05:40:40 GMT, Tom Rodriguez wrote: > This logic no longer seems to be necessary since the adjustCompilationLevel > callback has been removed. mach5 runs with the requested options on a GraalVM have passed so I'm going to merge this now. Sound good? - PR: https://git.openjdk.org/jdk/pull/5625
