Re: RFR: 8311846: Resolve duplicate 'Thread' related symbols with JDK static linking

2024-01-17 Thread David Holmes
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

2024-01-17 Thread David Holmes
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

2024-01-17 Thread Chris Plummer
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

2024-01-17 Thread Chris Plummer
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

2024-01-17 Thread Daniel D . Daugherty
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]

2024-01-17 Thread Cesar Soares Lucas
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

2024-01-17 Thread Coleen Phillimore
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]

2024-01-17 Thread Chris Plummer
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]

2024-01-17 Thread Chris Plummer
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]

2024-01-17 Thread Tom Rodriguez
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]

2024-01-17 Thread Tom Rodriguez
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]

2024-01-17 Thread Chris Plummer
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]

2024-01-17 Thread Tom Rodriguez
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]

2024-01-17 Thread Tom Rodriguez
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]

2024-01-17 Thread Tom Rodriguez
> 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]

2024-01-17 Thread Tom Rodriguez
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]

2024-01-17 Thread Chris Plummer
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]

2024-01-17 Thread Chris Plummer
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]

2024-01-17 Thread Tom Rodriguez
> 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]

2024-01-17 Thread Tom Rodriguez
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

2024-01-17 Thread Jiangli Zhou
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

2024-01-17 Thread Andrew Haley
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

2024-01-17 Thread Joachim Kern
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]

2024-01-17 Thread Adam Sotona
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