Re: RFR: 8311846: Resolve duplicate 'Thread' related symbols with JDK static linking
On Wed, 17 Jan 2024 00:14:58 GMT, Jiangli Zhou wrote: > Please review this PR with a simple solution for resolving duplicate `Thread` > symbol issue. In https://github.com/openjdk/jdk/pull/14808 comments, there > was an alternative suggestion to redefine the symbol at build time, such as > using`-DThread=HotSpotThread`. That would not address issues when symbol were > references as string literals. https://github.com/openjdk/jdk/pull/14808 also > discussed using namespace for hotspot code, which can have multiple > benefits/motivations. We could explore further using namespace with more > consensus on that approach. > > Contributed by Chuck Rasbold and @jianglizhou. Okay so now that I have context switched in the discussion from: https://github.com/openjdk/jdk/pull/14808 what happened to doing a JEP for namespaces? - PR Comment: https://git.openjdk.org/jdk/pull/17456#issuecomment-1897950301
Re: RFR: 8311846: Resolve duplicate 'Thread' related symbols with JDK static linking
On Wed, 17 Jan 2024 00:14:58 GMT, Jiangli Zhou wrote: > Please review this PR with a simple solution for resolving duplicate `Thread` > symbol issue. In https://github.com/openjdk/jdk/pull/14808 comments, there > was an alternative suggestion to redefine the symbol at build time, such as > using`-DThread=HotSpotThread`. That would not address issues when symbol were > references as string literals. https://github.com/openjdk/jdk/pull/14808 also > discussed using namespace for hotspot code, which can have multiple > benefits/motivations. We could explore further using namespace with more > consensus on that approach. > > Contributed by Chuck Rasbold and @jianglizhou. > Linking failures were observed when statically linking the launcher > executable with hotspot and user native code together: So the problem is that the user native code defines Thread as well - right? So this could keep happening for name after name depending on what native code is being linked. I second what @theRealAph said! This is really ugly. The way disparate libraries just get munged into a single namespace with static linking just seems wrong to me. At a minimum this hack should only be used when doing static linking as Coleen suggested. But I'd much prefer a solution that came from the tools doing the linking. - PR Comment: https://git.openjdk.org/jdk/pull/17456#issuecomment-1897940456
Re: RFR: 8324082: more monitoring test timeout adjustments
On Thu, 18 Jan 2024 01:28:09 GMT, Daniel D. Daugherty wrote: > Trivial fixes to adjust more monitoring test timeouts. > > See the bug report for the gory timeout details. Shouldn't we use a larger timeoutfactor for slowdebug builds? - PR Comment: https://git.openjdk.org/jdk/pull/17478#issuecomment-1897815509
RFR: 8324066: "clhsdb jstack" should not by default scan for j.u.c locks because it can be very slow
I noticed that "clhsdb jstack" seemed to hang when I attached to process with a somewhat large heap. It had taken over 10 minutes when I finally decided to have a look at the SA process (using bin/jstack), which came up with the following in the stack: at sun.jvm.hotspot.oops.ObjectHeap.iterateObjectsOfKlass([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/ObjectHeap.java:117) at sun.jvm.hotspot.runtime.ConcurrentLocksPrinter.fillLocks([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/ConcurrentLocksPrinter.java:70) at sun.jvm.hotspot.runtime.ConcurrentLocksPrinter.([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/ConcurrentLocksPrinter.java:36) at sun.jvm.hotspot.tools.StackTrace.run([jdk.hotspot.agent@23-internal](mailto:jdk.hotspot.agent@23-internal)/StackTrace.java:71) This code is meant to print information about j.u.c. locks. It it works by searching the entire java heap for instances of AbstractOwnableSynchronizer. This is a very expensive operation because it means not only does SA need to read in the entire java heap, but it needs to create a Klass mirror for every heap object so it can determine its type. Our testing doesn't seem to run into this slowness problem because "clhsdb jstack" only iterates over the heap if AbstractOwnableSynchronizer has been loaded, which it won't be if no j.u.c. locks have been created. This seems to be the case wherever we use "clhsdb jstack" in testing. We do have some tests that likely result in j.u.c locks, but they all use "jhsdb jstack", which doesn't have this issue (it requires use of the --locks argument to enable printing of j.u.c locks). This issue was already called out in [JDK-8262098](https://bugs.openjdk.org/browse/JDK-8262098). For this CR I am proposing that "clhsdb jstack" not include j.u.c lock scanning by default, and add the -l argument to enable it. This will make it similar to bin/jstack, which also has a -l argument, and to "clhsdb jstack", which has a --locks argument (which maps internally to the Jstack.java -l argument). The same changes are also being made to "clhsdb pstack". Tested with all tier1, tier2, and tier5 svc tests. - Commit messages: - fix jcheck errors - Minor cleanup - Don't search for l.u.c lock instances by default. Must pass -l option. Changes: https://git.openjdk.org/jdk/pull/17479/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17479=00 Issue: https://bugs.openjdk.org/browse/JDK-8324066 Stats: 226 lines in 9 files changed: 204 ins; 1 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/17479.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17479/head:pull/17479 PR: https://git.openjdk.org/jdk/pull/17479
RFR: 8324082: more monitoring test timeout adjustments
Trivial fixes to adjust more monitoring test timeouts. See the bug report for the gory timeout details. - Commit messages: - 8324082: more monitoring test timeout adjustments Changes: https://git.openjdk.org/jdk/pull/17478/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17478=00 Issue: https://bugs.openjdk.org/browse/JDK-8324082 Stats: 14 lines in 14 files changed: 0 ins; 0 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/17478.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17478/head:pull/17478 PR: https://git.openjdk.org/jdk/pull/17478
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: 8311846: Resolve duplicate 'Thread' related symbols with JDK static linking
On Wed, 17 Jan 2024 00:14:58 GMT, Jiangli Zhou wrote: > Please review this PR with a simple solution for resolving duplicate `Thread` > symbol issue. In https://github.com/openjdk/jdk/pull/14808 comments, there > was an alternative suggestion to redefine the symbol at build time, such as > using`-DThread=HotSpotThread`. That would not address issues when symbol were > references as string literals. https://github.com/openjdk/jdk/pull/14808 also > discussed using namespace for hotspot code, which can have multiple > benefits/motivations. We could explore further using namespace with more > consensus on that approach. > > Contributed by Chuck Rasbold and @jianglizhou. I was reading through the other PR for StringTable and was wonder how difficult it would be to wrap all of hotspot in namespace hotspot {}; using namespace hotspot; It would need a JEP as discussed in the other PR. Alternatively if there's a #ifdef you can use for renaming the Thread to HotspotThread for static linking only, it might make this change less worrysome. - PR Comment: https://git.openjdk.org/jdk/pull/17456#issuecomment-1897336087
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: 8311846: Resolve duplicate 'Thread' related symbols with JDK static linking
On Wed, 17 Jan 2024 10:07:15 GMT, Andrew Haley wrote: >> Please review this PR with a simple solution for resolving duplicate >> `Thread` symbol issue. In https://github.com/openjdk/jdk/pull/14808 >> comments, there was an alternative suggestion to redefine the symbol at >> build time, such as using`-DThread=HotSpotThread`. That would not address >> issues when symbol were references as string literals. >> https://github.com/openjdk/jdk/pull/14808 also discussed using namespace for >> hotspot code, which can have multiple benefits/motivations. We could explore >> further using namespace with more consensus on that approach. >> >> Contributed by Chuck Rasbold and @jianglizhou. > > Hooboy, this is an ugly solution, with some nasty side effects such as > confusing error mesasges for developers and a very confusing debugger > experience. Let's try to find a solution with a smaller blast radius. Hi @theRealAph Thanks for looking into this! https://github.com/openjdk/jdk/pull/14808 comments touched on several options: 1. Using namespace, in smaller scope for specific class such as `StringTable` or for all hotspot code in a global scope. Most seem to prefer using a specific namespace for all hotspot code, but there were still concerns. 2. Using #define to redefine the symbol (using in the current PR) This is a somewhat hacky solution. It requires small changes without touching many source code for renaming. 3. Redefine symbol at build/compile time. This is similar to the above. 4. Direct rename in the source Earlier discussions and feedback seem to prefer options requiring non-large scale change (except hotspot namespace solution). If acceptable by everyone, direct renaming would be the least confusion causing option. Any other suggestions and ideas for resolving the `Thread` issue? Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/17456#issuecomment-1896255274
Re: RFR: 8311846: Resolve duplicate 'Thread' related symbols with JDK static linking
On Wed, 17 Jan 2024 00:14:58 GMT, Jiangli Zhou wrote: > Please review this PR with a simple solution for resolving duplicate `Thread` > symbol issue. In https://github.com/openjdk/jdk/pull/14808 comments, there > was an alternative suggestion to redefine the symbol at build time, such as > using`-DThread=HotSpotThread`. That would not address issues when symbol were > references as string literals. https://github.com/openjdk/jdk/pull/14808 also > discussed using namespace for hotspot code, which can have multiple > benefits/motivations. We could explore further using namespace with more > consensus on that approach. > > Contributed by Chuck Rasbold and @jianglizhou. Hooboy, this is an ugly solution, with some nasty side effects such as confusing error mesasges for developers and a very confusing debugger experience. Let's try to find a solution with a smaller blast radius. - PR Comment: https://git.openjdk.org/jdk/pull/17456#issuecomment-1895486108
Integrated: JDK-8319382: com/sun/jdi/JdwpAllowTest.java shows failures on AIX if prefixLen of mask is larger than 32 in IPv6 case
On Thu, 11 Jan 2024 15:46:59 GMT, Joachim Kern wrote: > In parseAllowedMask in socketTransport.c, prefixLen of mask is compared with > a maxValue (32 for IPv4, 128 otherwise). This fails if it is larger than 32, > because getaddrinfo seems to detect IPv4 family, if IPv6 address has set only > some of the last 32 Bits. So we take the wrong maxValue. This pull request has now been integrated. Changeset: 22642ff0 Author:Joachim Kern Committer: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/22642ff0aac71eceb71f6a9eebb2988a9bd5f091 Stats: 37 lines in 1 file changed: 3 ins; 24 del; 10 mod 8319382: com/sun/jdi/JdwpAllowTest.java shows failures on AIX if prefixLen of mask is larger than 32 in IPv6 case Reviewed-by: mbaesken, amenkov - PR: https://git.openjdk.org/jdk/pull/17374
Re: RFR: 8294977: Convert test/jdk/java tests from ASM library to Classfile API [v11]
On Sun, 17 Dec 2023 23:11:10 GMT, Chen Liang wrote: >> Summaries: >> 1. A few recommendations about updating the constant API is made at >> https://mail.openjdk.org/pipermail/classfile-api-dev/2023-March/000233.html >> and I may update this patch shall the API changes be integrated before >> 2. One ASM library-specific test, `LambdaAsm` is removed. Others have their >> code generation infrastructure upgraded from ASM to Classfile API. >> 3. Most tests are included in tier1, but some are not: >> In `:jdk_io`: (tier2, part 2) >> >> test/jdk/java/io/Serializable/records/SerialPersistentFieldsTest.java >> test/jdk/java/io/Serializable/records/ProhibitedMethods.java >> test/jdk/java/io/Serializable/records/BadCanonicalCtrTest.java >> >> In `:jdk_instrument`: (tier 3) >> >> test/jdk/java/lang/instrument/RetransformAgent.java >> test/jdk/java/lang/instrument/NativeMethodPrefixAgent.java >> test/jdk/java/lang/instrument/asmlib/Instrumentor.java >> >> >> @asotona Would you mind reviewing? > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Fix the 2 failing tests and add notes Yes, CodeBuilder method renames will require some refactoring. This is good. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13009#pullrequestreview-1826743185