Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

[Ioi Lam]

On Tue, 2 Feb 2021 16:00:43 GMT, Gerard Ziemski  wrote:

>> I am not sure if jni_utils.c is the right file (it defines the `JNU_XXX` 
>> functions that are used by other shared libraries).
>> 
>> There are other .c files that have trivial `DEF_JNI_OnLoad` functions (e.g., 
>> java.base/share/native/libnio/nio_util.c).
>> 
>> @AlanBateman do you have any suggestions?
>
> I'm fine with the way it is, just thought we might want to consider cleaning 
> up a bit more, since it's a cleanup task itself.

Thanks Gerard. The main purpose of this PR is to clean up the JVM side. I'll 
leave the refactoring of check_version.c to the core-lib team.

-

PR: https://git.openjdk.java.net/jdk/pull/2338

Integrated: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX

[Ioi Lam]

On Mon, 1 Feb 2021 18:40:54 GMT, Ioi Lam  wrote:

> - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which allowed 
> the same JDK library to use different version of HotSpot. However, HSX is no 
> longer supported so this API should be removed.
> - Implementations of APIs such as JVM_DTraceActivate, were removed in 
> [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their 
> declarations should be removed from jvm.h

This pull request has now been integrated.

Changeset: b9d4211b
Author:Ioi Lam 
URL:   https://git.openjdk.java.net/jdk/commit/b9d4211b
Stats: 112 lines in 4 files changed: 0 ins; 110 del; 2 mod

8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX

Reviewed-by: alanb, lfoltan, gziemski, ihse

-

PR: https://git.openjdk.java.net/jdk/pull/2338

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

[Ioi Lam]

On Tue, 2 Feb 2021 15:59:47 GMT, Gerard Ziemski  wrote:

>> Ioi Lam has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fixed macos build
>
> Marked as reviewed by gziemski (Committer).

Thanks @gerard-ziemski @magicus @AlanBateman @lfoltan  for the review.

-

PR: https://git.openjdk.java.net/jdk/pull/2338

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v2]

[Phil Race]

On Tue, 2 Feb 2021 21:26:28 GMT, Kevin Rushforth  wrote:

>> The changes look good to me, but please see my comment from my review about 
>> documenting `NormalizedPathNSStringFromJavaString()` API.
>
> The following commit was integrated to jdk master since you created this 
> branch:
> 
> acbcde8 : [JDK-8256111](https://bugs.openjdk.java.net/browse/JDK-8256111) : 
> Create implementation for NSAccessibilityStaticText protocol
> 
> with a new reference to JNF, so it fails to compile with this patch. You will 
> need to merge openjdk/jdk master into your PR branch and then fix the new JNF 
> reference.

Updated as follows
- merged to current master and fixed the new A11Y usage of JNF
- added comment provided by Gerard
- removed comments as suggested by Sergey
- made failure of GetStringChars in CTextPipe throw OOME

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v4]

[Phil Race]

> This completes the desktop module JNF removal
> 
> * remove  -framework JavaNativeFoundation from make files
> 
> * remove  #import  from all 
> source files. If needed add import of JNIUtilities.h to get jni.h definitions 
> - better anyway since then it gets the current JDK ones not the ones from the 
> O/S
> 
> * replace JNFNSToJavaString with NSStringToJavaString and  JNFJavaToNSString 
> with JavaStringToNSString
> 
> * replace JNFNormalizedNSStringForPath with 
> NormalizedPathNSStringFromJavaString and JNFNormalizedJavaStringForPath with 
> NormalizedPathJavaStringFromNSString
> 
> * replace JNFGet/ReleaseStringUTF16UniChars with direct calls to JNI
> 
> * Map all JNFRunLoop perform* calls to the ThreadUtilities versions (the vast 
> majority already did this)
> 
> * Redo the ThreadUtilities calls to JNFRunLoop to directly invoke NSObject 
> perform* methods.
> 
> * define new javaRunLoopMode in ThreadUtilities to replace the JNF one and 
> use where needed.
> 
> * Remove the single usage of JNFPerformEnvBlock
> 
> * replace JNFJavaToNSNumber in single A11Y file with local replacement
> 
> * replace single usage of JNFNSTimeIntervalToJavaMillis in ScreenMenu.m with 
> local replacement
> 
> * remove un-needed JNFRunLoopDidStartNotification from NSApplicationAWT.m
> 
> * misc. remaining cleanup (eg missed JNF_CHECK_AND_RETHROW_EXCEPTION)

Phil Race 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 six additional commits since the 
last revision:

 - 8260616: Removing remaining JNF dependencies in the java.desktop module
 - 8260616: Removing remaining JNF dependencies in the java.desktop module
 - Merge branch 'master' into jnf_string
 - 8260616: Removing remaining JNF dependencies in the java.desktop module
 - 8260616: Removing remaining JNF dependencies in the java.desktop modul
 - 8260616: Removing remaining JNF dependencies in the java.desktop module

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2305/files
  - new: https://git.openjdk.java.net/jdk/pull/2305/files/7ea57c85..8a014fa6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2305&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2305&range=02-03

  Stats: 34374 lines in 817 files changed: 13984 ins; 9392 del; 10998 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2305.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2305/head:pull/2305

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

[Daniel D . Daugherty]

On Tue, 2 Feb 2021 11:59:08 GMT, Anton Kozlov  wrote:

>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>> 
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
>> windows/aarch64. 
>> 
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with 
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
>> required on macOS/AArch64 platform. It's implemented with 
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
>> thread, so W^X mode change relates to the java thread state change (for java 
>> threads). In most cases, JVM executes in write-only mode, except when 
>> calling a generated stub like SafeFetch, which requires a temporary switch 
>> to execute-only mode. The same execute-only mode is enabled when a java 
>> thread executes in java or native states. This approach of managing W^X mode 
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   support macos_aarch64 in hsdis

For platform files that were copied from other ports to this port, if the file 
wasn't
changed I presume the copyright years are left alone. If the file required 
changes
for this port, I expect the year to be updated to 2021. How are you verifying 
that
these copyright years are being properly managed on the new files?

For the new W^X helpers, e.g., WXWriteFromExecSetter, a short comment
explaining why one was landed in a particular place would help reviewers.
Also see my comment about creating a new ThreadToNativeWithWXExecFromVM
helper.

I'm stopping my review with all the src/hotspot files done for now.

make/autoconf/flags.m4 line 140:

> 138: else
> 139:   MACOSX_VERSION_MIN=10.12.0
> 140: fi

Not something that needs to be addressed here, but these changes
illustrate that our collective use of macOSX/MACOSX/MacOSX names
are tied to the fact that the macOS major version number was at 10
for a very long time.

@magicus - Do we have an RFE to rename MACOSX or are we sticking
with it and evolving our interpretation of the 'X' from '10' to */splat/asterik?

make/common/NativeCompilation.gmk line 1178:

> 1176: endif
> 1177: # This only works if the openjdk_codesign identity is 
> present on the system. Let
> 1178: # silently fail otherwise.

Might want to add a comment here:
#  The '-f' option will replace an existing signature if one exists.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 41:

> 39: #define __ _masm->
> 40: 
> 41: //describe amount of space in bytes occupied by type on native stack

nit - needs a space after `//`

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 83:

> 81: // On macos/aarch64 native stack is packed, int/float are using only 4 
> bytes
> 82: // on stack. Natural alignment for types are still in place,
> 83: // for example double/long should be 8 bytes alligned

nit typo: s/alligned/aligned/
And sentence should end with a period.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 323:

> 321:   str(zr, Address(rthread, JavaThread::last_Java_pc_offset()));
> 322: 
> 323:   str(zr, Address(rthread, JavaFrameAnchor::saved_fp_address_offset()));

I don't think this switch from `JavaThread::saved_fp_address_offset()`
to `JavaFrameAnchor::saved_fp_address_offset()` is correct since
`rthread` is still used and is a JavaThread*. The new code will give you:

`rthread` + offset of the `saved_fp_address` field in a JavaFrameAnchor

The old code gave you:

`rthread` + offset of the `saved_fp_address` field in the JavaFrameAnchor 
field in the JavaThread

Those are not the same things.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp line 31:

> 29: #include "asm/assembler.inline.hpp"
> 30: #include "oops/compressedOops.hpp"
> 31: #include "runtime/vm_version.hpp"

It's not clear why this include needed to be added.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 810:

> 808: #ifdef __APPLE__
> 809:   // Less-than word types are stored one after another.
> 810:   // The code unable to handle this, bailout.

Perhaps:  // The code is unable to handle this so bailout.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 837:

> 835: #ifdef __APPLE__
> 836:   // Less-than word types are stored one after another.
> 837:   // The code unable to handle this, bailout.

Perhaps: // The code is unable to handle this so bailout.

src/hotspot/os/aix/os_aix.cpp line 69:

> 67: #include "runtime/sharedRuntime.hpp"
> 68: #include "runtime/statSampler.hpp"
> 69: #include "runtime/stubRoutines.inline.hpp"

It is not clear wh

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v3]

[Phil Race]

On Tue, 2 Feb 2021 22:33:09 GMT, Phil Race  wrote:

>> I read it and not sure that it is fine to ignore this error, why not throw 
>> an exception and signal the CTextPipe_doDrawString that an error occurred 
>> like InvalidPipeException or something(Sometimes we wrap other exception 
>> like OOM into the InvalidPipeException and this seems similar case)?
>
>> The changes look good to me, but please see my comment from my review about 
>> documenting `NormalizedPathNSStringFromJavaString()` API.
> 
> I see it. I will copy what you wrote in there.

> I read it and not sure that it is fine to ignore this error, why not throw an 
> exception and signal the CTextPipe_doDrawString that an error occurred like 
> InvalidPipeException or something(Sometimes we wrap other exception like OOM 
> into the InvalidPipeException and this seems similar case)?

OK. I will do something like throw OOME or InternalError. Since we already 
checked for NULL as an arg any failure here implies a deep VM problem rather 
than anytjhing like a 2D invalid pipe

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v3]

[Phil Race]

On Tue, 2 Feb 2021 21:48:36 GMT, Sergey Bylokhov  wrote:

>> Look a few lines further up at my reply 3 days ago Gerard about this.
>
> I read it and not sure that it is fine to ignore this error, why not throw an 
> exception and signal the CTextPipe_doDrawString that an error occurred like 
> InvalidPipeException or something(Sometimes we wrap other exception like OOM 
> into the InvalidPipeException and this seems similar case)?

> The changes look good to me, but please see my comment from my review about 
> documenting `NormalizedPathNSStringFromJavaString()` API.

I see it. I will copy what you wrote in there.

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v3]

[Phil Race]

On Tue, 2 Feb 2021 22:02:14 GMT, Sergey Bylokhov  wrote:

>> I ran some tests embedding JavaFX into Swing and vice versa both with and 
>> without `-Djavafx.embed.singleThread=true` and I don't see any regression in 
>> behavior.
>
> I am mostly worried about the usage of JNF by someone else's native code, as 
> far as I understand it could be "broken" now. But it is good that FX does not 
> use it.
> 
> BTW looks like all comments like  "// AWT_THREADING Safe (AWTRunLoop)" could 
> be removed now.

SWT does not use JNF at all. I've looked at their source code.
I can clean up those comments.

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v3]

[Sergey Bylokhov]

On Mon, 1 Feb 2021 23:47:06 GMT, Phil Race  wrote:

>> src/java.desktop/macosx/native/libawt_lwawt/awt/CTextPipe.m line 611:
>> 
>>> 609: const jchar *unichars = (*env)->GetStringChars(env, str, NULL);
>>> 610: if (unichars == NULL) {
>>> 611: return;
>> 
>> Do not we need to throw an exception here? Otherwise, GetStringChars error 
>> will be ignored?
>
> Look a few lines further up at my reply 3 days ago Gerard about this.

I read it and not sure that it is fine to ignore this error, why not throw an 
exception and signal the CTextPipe_doDrawString that an error occurred like 
InvalidPipeException or something(Sometimes we wrap other exception like OOM 
into the InvalidPipeException and this seems similar case)?

>> src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m 
>> line 967:
>> 
>>> 965: static NSNumber* JavaNumberToNSNumber(JNIEnv *env, jobject jnumber) {
>>> 966: if (jnumber == NULL) {
>>> 967: return nil;
>> 
>> Based on its usage it is probably should be zero on NULL number?
>
> Not an unreasonable idea and I considered it but :
> - It is never called with NULL. There is always a null check
> - The JNF equivalent returns nil on NULL
> 
> BTW two of the functions in which the code appears : 
> accessibilityMinValueAttribute and accessibilityMaxValueAttribute  (SFAIC) 
> aren't used anywhere.

@azuev-java Looks like a cleanup opportunity?

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v3]

[Sergey Bylokhov]

On Tue, 2 Feb 2021 21:18:42 GMT, Kevin Rushforth  wrote:

>> We are just specifying an additional run mode for JDK internal use.
>> It means that when we are saying to process only events for that mode, then 
>> only those will be processed.
>> And it is used only for nested event loops.
>> Nothing eternal should be assuming AWT uses JNF at all, never mind in a 
>> particular way.
>> 
>> There is a special case for FX but I don't see a problem.
>> 
>> If you look at Java_sun_lwawt_macosx_LWCToolkit_doAWTRunLoopImpl
>> there's a variable "inAWT".
>>  isRunning = [[NSRunLoop currentRunLoop] runMode:(inAWT ? 
>> [JNFRunLoop javaRunLoopMode] : NSDefaultRunLoopMode) ... ]];
>> 
>> This is specified as
>> inAWT = AccessController.doPrivileged(new 
>> PrivilegedAction() {
>> @Override
>> public Boolean run() {
>> return 
>> !Boolean.parseBoolean(System.getProperty("javafx.embed.singleThread", 
>> "false"));
>> }
>> });
>> }
>> 
>> So generally FX doesn't care, and is ignorant of this new mode.
>> Unless you set that property to true, in which case AWT use the  
>> NSDefaultRunLoopMode and so again FX is unaffected.
>> Nothing in the FX sources goes anywhere near JNF .. or has a reference to 
>> the special mode.
>> 
>> Bottom line I don't see it being affected. 
>> 
>> I'll check with Kevin but also Gerard had a lot to do with the creation of 
>> the current FX toolkit.
>
> I ran some tests embedding JavaFX into Swing and vice versa both with and 
> without `-Djavafx.embed.singleThread=true` and I don't see any regression in 
> behavior.

I am mostly worried about the usage of JNF by someone else's native code, as 
far as I understand it could be "broken" now. But it is good that FX does not 
use it.

BTW looks like all comments like  "// AWT_THREADING Safe (AWTRunLoop)" could be 
removed now.

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v2]

[Kevin Rushforth]

On Mon, 1 Feb 2021 18:33:32 GMT, Gerard Ziemski  wrote:

>> Marked as reviewed by gziemski (Committer).
>
> The changes look good to me, but please see my comment from my review about 
> documenting `NormalizedPathNSStringFromJavaString()` API.

The following commit was integrated to jdk master since you created this branch:

acbcde8 : [JDK-8256111](https://bugs.openjdk.java.net/browse/JDK-8256111) : 
Create implementation for NSAccessibilityStaticText protocol

with a new reference to JNF, so it fails to compile with this patch. You will 
need to merge openjdk/jdk master into your PR branch and then fix the new JNF 
reference.

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v3]

[Kevin Rushforth]

On Tue, 2 Feb 2021 00:30:07 GMT, Phil Race  wrote:

>> src/java.desktop/macosx/native/libosxapp/ThreadUtilities.m line 53:
>> 
>>> 51: @implementation ThreadUtilities
>>> 52: 
>>> 53: + (void)initialize {
>> 
>> I think we need to check how this new modes will work when the AWT is 
>> embedded inside SWT and FX.
>
> We are just specifying an additional run mode for JDK internal use.
> It means that when we are saying to process only events for that mode, then 
> only those will be processed.
> And it is used only for nested event loops.
> Nothing eternal should be assuming AWT uses JNF at all, never mind in a 
> particular way.
> 
> There is a special case for FX but I don't see a problem.
> 
> If you look at Java_sun_lwawt_macosx_LWCToolkit_doAWTRunLoopImpl
> there's a variable "inAWT".
>  isRunning = [[NSRunLoop currentRunLoop] runMode:(inAWT ? [JNFRunLoop 
> javaRunLoopMode] : NSDefaultRunLoopMode) ... ]];
> 
> This is specified as
> inAWT = AccessController.doPrivileged(new PrivilegedAction() 
> {
> @Override
> public Boolean run() {
> return 
> !Boolean.parseBoolean(System.getProperty("javafx.embed.singleThread", 
> "false"));
> }
> });
> }
> 
> So generally FX doesn't care, and is ignorant of this new mode.
> Unless you set that property to true, in which case AWT use the  
> NSDefaultRunLoopMode and so again FX is unaffected.
> Nothing in the FX sources goes anywhere near JNF .. or has a reference to the 
> special mode.
> 
> Bottom line I don't see it being affected. 
> 
> I'll check with Kevin but also Gerard had a lot to do with the creation of 
> the current FX toolkit.

I ran some tests embedding JavaFX into Swing and vice versa both with and 
without `-Djavafx.embed.singleThread=true` and I don't see any regression in 
behavior.

-

PR: https://git.openjdk.java.net/jdk/pull/2305

Re: RFR: 8257988: Remove JNF dependency from libsaproc/MacosxDebuggerLocal.m

[Chris Plummer]

On Thu, 28 Jan 2021 22:40:57 GMT, Phil Race  wrote:

> This removes the JNF dependency from the jdk.hotspot.agent module.
> The macro expansions are the same as already used in the Java desktop module 
> - which actually uses a macro
> still but there there are hundreds of uses.
> The function of this macro code is to prevent NSExceptions escaping to Java 
> and also to drain the auto-release pool.
> What test group would be good for verifying this change ?

src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m line 294:

> 292: 
> 293:   NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
> 294:   @try {

Although there are only 3 places where the `JNF_COCOA_ENTER/EXIT` macros are 
used, it seems it would still be worth taking the same approach you did for 
`java.desktop` and add the replacement macros instead of inlining them. So just 
copy what you added to 
`src/java.desktop/macosx/native/libosxapp/JNIUtilities.h`, and replace 
`JNF_COCOA_ENTER` with `JNI_COCOA_ENTER/EXIT`. Otherwise the 
`JNF_COCOA_ENTER/EXIT` changes look fine to me, but I'm just basing this on a 
comparison with what you've done with `java.desktop`. I'm no expert in this 
area.

src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m line 296:

> 294:   @try {
> 295: 
> 296:   NSString *symbolNameString = JavaStringToNSString(env, symbolName);

Is there a reason why `java.desktop` continues to use `JNFJavaToNSString`? I 
was looking to see how this was handled in other places, but I couldn't find an 
example where `JNFJavaToNSString` was converted to call a newly implemented 
`JavaStringToNSString`.

-

PR: https://git.openjdk.java.net/jdk/pull/2304

Re: [jdk16] RFR: 8259794: Remove EA from JDK 16 version string starting with Initial RC promotion on Feb 04, 2021(B35)

[Mikael Vidstedt]

On Tue, 2 Feb 2021 18:17:55 GMT, Jesper Wilhelmsson  
wrote:

> We have our (first) RC candidate build for JDK 16 on Feb 04, 2021. We need to 
> remove the EA from version string for this build (b35) and going forward.
> 
> Pushing this for @pashh

Marked as reviewed by mikael (Reviewer).

-

PR: https://git.openjdk.java.net/jdk16/pull/146

[jdk16] Integrated: 8259794: Remove EA from JDK 16 version string starting with Initial RC promotion on Feb 04, 2021(B35)

[Jesper Wilhelmsson]

On Tue, 2 Feb 2021 18:17:55 GMT, Jesper Wilhelmsson  
wrote:

> We have our (first) RC candidate build for JDK 16 on Feb 04, 2021. We need to 
> remove the EA from version string for this build (b35) and going forward.
> 
> Pushing this for @pashh

This pull request has now been integrated.

Changeset: 1a7040e5
Author:Jesper Wilhelmsson 
URL:   https://git.openjdk.java.net/jdk16/commit/1a7040e5
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8259794: Remove EA from JDK 16 version string starting with Initial RC 
promotion on Feb 04, 2021(B35)

Reviewed-by: iignatyev, mikael

-

PR: https://git.openjdk.java.net/jdk16/pull/146

Re: [jdk16] RFR: 8259794: Remove EA from JDK 16 version string starting with Initial RC promotion on Feb 04, 2021(B35)

[Igor Ignatyev]

On Tue, 2 Feb 2021 18:17:55 GMT, Jesper Wilhelmsson  
wrote:

> We have our (first) RC candidate build for JDK 16 on Feb 04, 2021. We need to 
> remove the EA from version string for this build (b35) and going forward.
> 
> Pushing this for @pashh

Marked as reviewed by iignatyev (Reviewer).

-

PR: https://git.openjdk.java.net/jdk16/pull/146

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

[Bernhard Urban-Forster]

On Tue, 2 Feb 2021 18:23:04 GMT, Gerard Ziemski  wrote:

>> Anton Kozlov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   support macos_aarch64 in hsdis
>
> src/hotspot/os/posix/signals_posix.cpp line 1297:
> 
>> 1295:   kern_return_t kr;
>> 1296:   kr = task_set_exception_ports(mach_task_self(),
>> 1297: EXC_MASK_BAD_ACCESS | 
>> EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC,
> 
> Could someone elaborate on why we need to add `EXC_MASK_BAD_INSTRUCTION` to 
> the mask here?

See comment above about `gdb`, the same applies to `lldb` today. The AArch64 
backend uses `SIGILL` (~= `EXC_MASK_BAD_INSTRUCTION`) to initiate a 
deoptimization. Without this change you cannot continue debugging once you the 
debuggee receives `SIGILL`. This wasn't needed before as x86 doesn't use 
`SIGILL`.

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

[Gerard Ziemski]

On Tue, 2 Feb 2021 18:52:29 GMT, Gerard Ziemski  wrote:

>> Anton Kozlov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   support macos_aarch64 in hsdis
>
> Changes requested by gziemski (Committer).

There were bunch of assembly code that I couldn't review. I also shallow 
reviewed the brand new files that were copies of the existing BSD x86_64 files 
- I hope I can get back to those when I figure out how to build/run these 
changes.

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

[Gerard Ziemski]

On Tue, 2 Feb 2021 11:59:08 GMT, Anton Kozlov  wrote:

>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>> 
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
>> windows/aarch64. 
>> 
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with 
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
>> required on macOS/AArch64 platform. It's implemented with 
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
>> thread, so W^X mode change relates to the java thread state change (for java 
>> threads). In most cases, JVM executes in write-only mode, except when 
>> calling a generated stub like SafeFetch, which requires a temporary switch 
>> to execute-only mode. The same execute-only mode is enabled when a java 
>> thread executes in java or native states. This approach of managing W^X mode 
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   support macos_aarch64 in hsdis

Changes requested by gziemski (Committer).

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 390:

> 388:   store_and_inc(_to, from_obj, NativeStack::intSpace);
> 389: 
> 390:   _num_int_args++;

`pass_byte()` and  `pass_short()` use only one `_num_int_args++;` after the `if 
else` but other methods use 2 of them inside `if else` branches.

We should be consistent.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 404:

> 402: } else {
> 403:   store_and_inc(_to, from_obj, NativeStack::longSpace);
> 404:   _num_int_args++;

`pass_byte()` and  `pass_short()` use only one `_num_int_args++;` after the `if 
else` but other methods use it 2 of them inside `if else`

We should be consistent.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 418:

> 416: } else {
> 417:   store_and_inc(_to, (*from_addr == 0) ? (intptr_t)NULL : (intptr_t) 
> from_addr, wordSize);
> 418:   _num_int_args++;

`pass_byte()` and  `pass_short()` use only one `_num_int_args++;` after the `if 
else` but other methods use it 2 of them inside `if else`

We should be consistent.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 433:

> 431:   store_and_inc(_to, from_obj, NativeStack::floatSpace);
> 432: 
> 433:   _num_fp_args++;

`pass_byte()` and  `pass_short()` use only one `_num_int_args++;` after the `if 
else` but other methods use it 2 of them inside `if else`

We should be consistent.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 448:

> 446: } else {
> 447:   store_and_inc(_to, from_obj, NativeStack::doubleSpace);
> 448:   _num_fp_args++;

`pass_byte()` and  `pass_short()` use only one `_num_int_args++;` after the `if 
else` but other methods use it 2 of them inside `if else`

We should be consistent.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5271:

> 5269: //
> 5270: void MacroAssembler::get_thread(Register dst) {
> 5271:   RegSet saved_regs = RegSet::range(r0, r1) + 
> BSD_ONLY(RegSet::range(r2, r17)) + lr - dst;

The comment needs to be updated, since on BSD we also seem to clobber r2,r17 ?

src/hotspot/os/posix/signals_posix.cpp line 1297:

> 1295:   kern_return_t kr;
> 1296:   kr = task_set_exception_ports(mach_task_self(),
> 1297: EXC_MASK_BAD_ACCESS | 
> EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC,

Could someone elaborate on why we need to add `EXC_MASK_BAD_INSTRUCTION` to the 
mask here?

src/hotspot/cpu/aarch64/vm_version_aarch64.hpp line 93:

> 91: CPU_MARVELL   = 'V',
> 92: CPU_INTEL = 'i',
> 93: CPU_APPLE = 'a',

The `ARM Architecture Reference Manual ARMv8, for ARMv8-A architecture profile` 
has 8538 pages, can we be more specific and point to the particular section of 
the document where the CPU codes are defined?

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: [jdk16] RFR: 8259794: Remove EA from JDK 16 version string starting with Initial RC promotion on Feb 04, 2021(B35)

[pashh]

On Tue, 2 Feb 2021 18:17:55 GMT, Jesper Wilhelmsson  
wrote:

> We have our (first) RC candidate build for JDK 16 on Feb 04, 2021. We need to 
> remove the EA from version string for this build (b35) and going forward.
> 
> Pushing this for @pashh

Marked as reviewed by pa...@github.com (no known OpenJDK username).

-

PR: https://git.openjdk.java.net/jdk16/pull/146

[jdk16] RFR: 8259794: Remove EA from JDK 16 version string starting with Initial RC promotion on Feb 04, 2021(B35)

[Jesper Wilhelmsson]

We have our (first) RC candidate build for JDK 16 on Feb 04, 2021. We need to 
remove the EA from version string for this build (b35) and going forward.

Pushing this for @pashh

-

Commit messages:
 - Remove ea

Changes: https://git.openjdk.java.net/jdk16/pull/146/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk16&pr=146&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259794
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk16/pull/146.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/146/head:pull/146

PR: https://git.openjdk.java.net/jdk16/pull/146

Re: [External] : Re: AArch64 OpenJDK bootstrap failure on head

[Thomas Stüfe]

(dropping Andrew and Aleksey)

On Tue, Feb 2, 2021 at 6:13 PM Ioi Lam  wrote:

>
>
> On 2/2/21 1:32 AM, Thomas Stüfe wrote:
>
>
>
> On Mon, Feb 1, 2021 at 10:11 PM Ioi Lam  wrote:
>
>> On 2/1/21 9:36 AM, Thomas Stüfe wrote:
>>
>> This does not solve the alignment problem, but I don't like that we
>> unconditionally print a message here since this is a non-fatal error. Also,
>> CDS mapping may fail for other reasons, for which we do not print
>> unconditionally. I think we should make this info log level:
>>
>> --- a/src/hotspot/share/memory/metaspaceShared.cpp
>> +++ b/src/hotspot/share/memory/metaspaceShared.cpp
>> @@ -1725,7 +1725,7 @@ MapArchiveResult
>> MetaspaceShared::map_archive(FileMapInfo* mapinfo, char* mapped
>>mapinfo->set_is_mapped(false);
>>
>>if (mapinfo->alignment() != (size_t)os::vm_allocation_granularity()) {
>> -log_error(cds)("Unable to map CDS archive --
>> os::vm_allocation_granularity() expected: " SIZE_FORMAT
>> +log_info(cds)("Unable to map CDS archive --
>> os::vm_allocation_granularity() expected: " SIZE_FORMAT
>> " actual: %d", mapinfo->alignment(),
>> os::vm_allocation_granularity());
>>  return MAP_ARCHIVE_OTHER_FAILURE;
>>}
>>
>> @ Ioi, does that make sense?
>>
>>
>> Yes, your fix makes sense.
>>
>>
> Thanks. https://github.com/openjdk/jdk/pull/2348
> 
>
>
>> This issue is being address in
>> https://bugs.openjdk.java.net/browse/JDK-8236847. We will probably
>> unconditionally change the alignment to 64KB for AARCH64, as well as MacOS
>> (so that you can run a X64 JDK on M1 using Rosetta).
>>
>>
> I thought so too. I also see it being used for region-to-region alignment,
> where I believe page size would be sufficient?
>
>
> The problem in JDK-8236847
>  happens when you
> create a CDS archive on one machine, and use it on another, and the two
> machines have different page sizes.
>
>
Oh, I understand. Sorry, I was not clear enough.

What I meant was: os::vm_allocation_granularity() is the OS-dictated
alignment for the start address for memory mappings. We need to know it
wherever we want to predict the mapping address of a future mapping.
Outside of that, there is very little reason to use it instead of using
page size. WIthin the hotspot, it is used all over the place, and a number
of those usages are bewildering and possibly wrong. Hence my original
question, why use it for CDS alignment. But you answered the question, as
long as we mmap the cds regions individually via mmap, we need to do that
with allocation granularity.

Setting it hard wired to 64k sounds pragmatic.

Note that I created https://bugs.openjdk.java.net/browse/JDK-8253683 a
while ago to cleanup usages of os::vm_allocation_granularity(), but never
got along to do this.


> (a) linux/aarch64 can be configured to have either 4KB or 64KB page sizes
> (b) macOS/x64 uses 4KB, but macOS/aarch64 uses 64KB
>
> So if you create the archive on a machine with 4KB page size, your RW
> region may start at (64KB * N + 4KB), and this region cannot be mapped
> directly on a machine with 64KB sizes.
>
> My proposal is to always align the CDS regions by 64KB on these platforms,
> so they can always be mapped under all circumstances.
>
>
Makes perfect sense.


> Alternatives are: use read() instead of mmap; or, instead of mmaping the
> individual regions, mmap all of them at once (assuming that the first
> region, MC, is 64KB aligned). Either solution will reduce the possibility
> of sharing, and make the code more complicated.
>
>
I still think, as in earlier discussions, that mapping it in one go would
be the right way to do.


> Since the CDS archive is at least 10MB in size, adding 3 extra padding
> areas of up to 64KB each doesn't seem that outrageous in file size. There's
> no change in physical memory usage since we never touch the padding area.
>
>
I agree.


> Thanks
> - Ioi
>
>
>
Cheers, Thomas


> .:Thomas
>
> Thanks
>> - Ioi
>>
>> Cheers, Thomas
>>
>>
>> On Mon, Feb 1, 2021 at 6:20 PM Andrew Haley  wrote:
>>
>>> On 2/1/21 5:14 PM, Aleksey Shipilev wrote:
>>> > On 2/1/21 4:38 PM, Andrew Haley wrote:
>>> >> but that doesn't work either. Any ideas? I'm really stuck.
>>> >
>>> > Did you "make clean" after changing any of the configure files and/or
>>> configure arguments? I.e. did
>>> > AWTIcon32_java_icon16_png actually regenerate?
>>>
>>> Many times.
>>>
>>> > Prepending the build with "LOG=debug" would tell what cmdlines are
>>> used at every step of build process.
>>>
>>> Sure, I can see what it's doing. I think this is actually a regression
>>> caused by the Windows-AArch64 port. I'll do some bisecting.
>>>
>>> --
>>> Andrew Haley  (he/him)
>>> Java Platform Lead Engineer
>>> Red Hat UK Ltd. >> 

Re: [External] : Re: AArch64 OpenJDK bootstrap failure on head

[Ioi Lam]

On 2/2/21 1:32 AM, Thomas Stüfe wrote:



On Mon, Feb 1, 2021 at 10:11 PM Ioi Lam > wrote:


On 2/1/21 9:36 AM, Thomas Stüfe wrote:

This does not solve the alignment problem, but I don't like
that we unconditionally print a message here since this is a
non-fatal error. Also, CDS mapping may fail for other reasons,
for which we do not print unconditionally. I think we should make
this info log level:

--- a/src/hotspot/share/memory/metaspaceShared.cpp
+++ b/src/hotspot/share/memory/metaspaceShared.cpp
@@ -1725,7 +1725,7 @@ MapArchiveResult
MetaspaceShared::map_archive(FileMapInfo* mapinfo, char* mapped
   mapinfo->set_is_mapped(false);

   if (mapinfo->alignment() !=
(size_t)os::vm_allocation_granularity()) {
-    log_error(cds)("Unable to map CDS archive --
os::vm_allocation_granularity() expected: " SIZE_FORMAT
+    log_info(cds)("Unable to map CDS archive --
os::vm_allocation_granularity() expected: " SIZE_FORMAT
                    " actual: %d", mapinfo->alignment(),
os::vm_allocation_granularity());
     return MAP_ARCHIVE_OTHER_FAILURE;
   }

@ Ioi, does that make sense?



Yes, your fix makes sense.


Thanks. https://github.com/openjdk/jdk/pull/2348 



This issue is being address in
https://bugs.openjdk.java.net/browse/JDK-8236847
. We will
probably unconditionally change the alignment to 64KB for AARCH64,
as well as MacOS (so that you can run a X64 JDK on M1 using Rosetta).


I thought so too. I also see it being used for region-to-region 
alignment, where I believe page size would be sufficient?




The problem in JDK-8236847 
 happens when you 
create a CDS archive on one machine, and use it on another, and the two 
machines have different page sizes.


(a) linux/aarch64 can be configured to have either 4KB or 64KB page sizes
(b) macOS/x64 uses 4KB, but macOS/aarch64 uses 64KB

So if you create the archive on a machine with 4KB page size, your RW 
region may start at (64KB * N + 4KB), and this region cannot be mapped 
directly on a machine with 64KB sizes.


My proposal is to always align the CDS regions by 64KB on these 
platforms, so they can always be mapped under all circumstances.


Alternatives are: use read() instead of mmap; or, instead of mmaping the 
individual regions, mmap all of them at once (assuming that the first 
region, MC, is 64KB aligned). Either solution will reduce the 
possibility of sharing, and make the code more complicated.


Since the CDS archive is at least 10MB in size, adding 3 extra padding 
areas of up to 64KB each doesn't seem that outrageous in file size. 
There's no change in physical memory usage since we never touch the 
padding area.


Thanks
- Ioi



.:Thomas

Thanks
- Ioi


Cheers, Thomas


On Mon, Feb 1, 2021 at 6:20 PM Andrew Haley mailto:a...@redhat.com>> wrote:

On 2/1/21 5:14 PM, Aleksey Shipilev wrote:
> On 2/1/21 4:38 PM, Andrew Haley wrote:
>> but that doesn't work either. Any ideas? I'm really stuck.
>
> Did you "make clean" after changing any of the configure
files and/or configure arguments? I.e. did
> AWTIcon32_java_icon16_png actually regenerate?

Many times.

> Prepending the build with "LOG=debug" would tell what
cmdlines are used at every step of build process.

Sure, I can see what it's doing. I think this is actually a
regression
caused by the Windows-AArch64 port. I'll do some bisecting.

-- 
Andrew Haley  (he/him)

Java Platform Lead Engineer
Red Hat UK Ltd. >
https://keybase.io/andrewhaley


EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

[Gerard Ziemski]

On Mon, 1 Feb 2021 20:10:58 GMT, Ioi Lam  wrote:

>> - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which 
>> allowed the same JDK library to use different version of HotSpot. However, 
>> HSX is no longer supported so this API should be removed.
>> - Implementations of APIs such as JVM_DTraceActivate, were removed in 
>> [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their 
>> declarations should be removed from jvm.h
>
> Ioi Lam has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fixed macos build

Marked as reviewed by gziemski (Committer).

-

PR: https://git.openjdk.java.net/jdk/pull/2338

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

[Gerard Ziemski]

On Mon, 1 Feb 2021 20:54:42 GMT, Ioi Lam  wrote:

>> src/java.base/share/native/libjava/check_version.c line 33:
>> 
>>> 31: DEF_JNI_OnLoad(JavaVM *vm, void *reserved)
>>> 32: {
>>> 33: return JNI_VERSION_1_2;
>> 
>> This leaves an entire file with one trivial function implementation. Can we 
>> remove the file and implement  `DEF_JNI_OnLoad()` in `jni_util.h` (or some 
>> other existing suitable file) ?
>
> I am not sure if jni_utils.c is the right file (it defines the `JNU_XXX` 
> functions that are used by other shared libraries).
> 
> There are other .c files that have trivial `DEF_JNI_OnLoad` functions (e.g., 
> java.base/share/native/libnio/nio_util.c).
> 
> @AlanBateman do you have any suggestions?

I'm fine with the way it is, just thought we might want to consider cleaning up 
a bit more, since it's a cleanup task itself.

-

PR: https://git.openjdk.java.net/jdk/pull/2338

Re: RFR: 8259886 : Improve SSL session cache performance and scalability [v2]

[djelinski]

On Tue, 2 Feb 2021 02:37:39 GMT, Xue-Lei Andrew Fan  wrote:

> the benchmark performance improvement is a trade-off between CPU and memory, 
> by keeping expired entries while putting a new entry in the cache

Not exactly. The memory use is capped by cache size. The patch is a trade off 
between the cache's hit/miss ratio and CPU; we will get faster cache access at 
the cost of more frequent cache misses.

All calls to `put()` remove expired items from the front of the queue, and 
never perform a full scan. `get()` calls shuffle the queue, moving the accessed 
item to the back. Compare this to original code where `put()` only removed 
expired items when the cache overflowed, and scanned the entire cache.
Let me give some examples. 
**Example 1**: insertions at a fast pace leading to cache overflows and no 
expirations. Here the new implementation improves performance. Consider a cache 
with size=4, timeout=10, and the following sequence of events:
T=1, put(1);
T=2, put(2);
T=3, put(3);
T=4, put(4);
Cache contents after these calls (same in old and new scenario). Queue order: 
least recently accessed items on the left, most recently accessed on the right. 
_K_ denotes cache key, _exp_ denotes entry expiration time and is equal to 
insertion time _T_ plus timeout:

|K=1, exp=11|K=2, exp=12|K=3, exp=13|K=4, exp=14|

If we now add another item to the queue, it will overflow. Here's where the 
implementations behave differently, but the outcome is identical: old one scans 
the entire list for expired entries; new one improves performance by ending the 
search for expired entries after encountering the first non-expired entry 
(which is the first entry in the above example). The end result is the same in 
both cases - oldest (least recently accessed) item is dropped:
T=5, put(5)

|K=2, exp=12|K=3, exp=13|K=4, exp=14|K=5, exp=15|

**Example 2**: insertions at a moderate pace, with interleaved reads. Here the 
new implementation improves performance, but at a possible cost of wasting 
cache capacity on expired entries. Consider a cache with size=4, timeout=7, and 
the following sequence of events:
T=1, put(1);
T=3, put(3);
T=5, put(5);
T=7, put(7);
T=7, get(1);
Cache contents after these calls:

|K=3, exp=10|K=5, exp=12|K=7, exp=14|K=1, exp=8|

`get(1)` operation moved item with K=1 to the back of the queue.

If we wait for item with K=1 to expire and then add another item to the queue, 
it will overflow. Here's where the implementations behave differently, and the 
outcome is different: old one scans the entire list for expired entries, finds 
entry with K=1 and drops it; new one gives up after first non-expired entry 
(which is the first entry), and drops the first entry.

So, when we perform:
T=9, put(9);

Old implementation will get:
|K=3, exp=10|K=5, exp=12|K=7, exp=14|K=9, exp=16|

New implementation will get:
|K=5, exp=12|K=7, exp=14|K=1, exp=8(expired)|K=9, exp=16|

Note that:
- an attempt to retrieve expired item (i.e. `get(1)`) will immediately remove 
that item from cache, making room for other items
- retrieving a non-expired item will move it to the back of the queue, behind 
all expired items

**Example 3**: insertions at a slow pace, where most items expire before queue 
overflows. Here the new implementation improves memory consumption. Consider a 
cache with size=4, timeout=1, and the following sequence of events:
T=1, put(1);
T=3, put(3);
T=5, put(5);
T=7, put(7);
Every cache item is expired at then point when a new one is added. Old 
implementation only removes expired entries when cache overflows, so all 
entries will still be there:

|K=1, exp=2(expired)|K=3, exp=4(expired)|K=5, exp=6(expired)|K=7, exp=8|

New implementation removes expired entries on every put, so after the last put 
only one entry is in the cache:

|K=7, exp=8|

After another put the old implementation will encounter a cache overflow and 
remove all expired items.

Let me know if that helped.

> add two more types of benchmark: get the entries and remove the entries

Both these operations are constant-time, both before and after my changes. Do 
you expect to see some oddities here, or do we just want a benchmark that could 
be used to compare other implementations?

> increase the size to some big scales, like 2M and 20M

Can do. Do you think it makes sense to also benchmark the scenario where GC 
kicks in and collects soft references?

> it may change the behavior of a few JDK components

Of all uses of Cache, only `SSLSessionContextImpl` (TLS session cache), 
`StatusResponseManager` (OCSP stapling) and `LDAPCertStoreImpl` (I'm not 
familiar with that one) set expiration timeout; when the timeout is not set, 
the behavior is exactly the same as before.
`StatusResponseManager` is constantly querying the same keys, and is liberally 
sized, so I don't expect much of an impact.
TLS session cache changes may result in fewer session resumptions and more full 
handshakes; I expect the cache performance improvement to more than off

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

[Alan Hayward]

On Tue, 2 Feb 2021 11:56:12 GMT, Vladimir Kempik  wrote:

> > > > Hello, hsdis is a separate out-of-tree project and is not part of this 
> > > > jep.
> > > 
> > > 
> > > Unless there's something I'm missing it only requires a few lines of 
> > > change to src/utils/hsdis/makefile (it already has support for macos 
> > > x86_64)
> > 
> > 
> > I agree with Alan that it makes sense to add this trivial change as part of 
> > this PR, if it allows the current hsdis solution to continue working on 
> > mac/aarch64.
> > > > support for looking for proper hsdis dylib library was added as part of 
> > > > this jep.
> > > 
> > > 
> > > I'm a little confused. Are you planning on adding a new disassembler?
> > 
> > 
> > @a74nh I think Vladimir is referring to #392. The hsdis "component" has 
> > been left behind for a long time, and there are several requests to add new 
> > backends, which require a modernized build of hsdis. This is undfortunately 
> > not a high-priority project, and has been postponed several times already. 
> > :(
> 
> Sorry I was under impression hsdis is not part of openjdk tree.
> 
> Alan, could you please share with us the version of binutils you were using 
> in your test ?
> 

I was just using the latest HEAD:
git clone git://sourceware.org/git/binutils-gdb.git 
src/utils/hsdis/build/binutils

A slightly safer approach would be to grab the latest release:
https://ftp.gnu.org/gnu/binutils/binutils-2.36.tar.gz


Once hsdis-demo was working for me, for only other oddity I had was that the 
library needed renaming when copying/linking into the build dir:
jdk/lib/server/libhsdis-aarch64.dylib

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

[Vladimir Kempik]

On Tue, 2 Feb 2021 11:14:12 GMT, Vladimir Kempik  wrote:

> > > > Hello, hsdis is a separate out-of-tree project and is not part of this 
> > > > jep.
> > > 
> > > 
> > > Unless there's something I'm missing it only requires a few lines of 
> > > change to src/utils/hsdis/makefile (it already has support for macos 
> > > x86_64)
> > 
> > 
> > I agree with Alan that it makes sense to add this trivial change as part of 
> > this PR, if it allows the current hsdis solution to continue working on 
> > mac/aarch64.
> > > > support for looking for proper hsdis dylib library was added as part of 
> > > > this jep.
> > > 
> > > 
> > > I'm a little confused. Are you planning on adding a new disassembler?
> > 
> > 
> > @a74nh I think Vladimir is referring to #392. The hsdis "component" has 
> > been left behind for a long time, and there are several requests to add new 
> > backends, which require a modernized build of hsdis. This is undfortunately 
> > not a high-priority project, and has been postponed several times already. 
> > :(
> 
> Sorry I was under impression hsdis is not part of openjdk tree.
> 
> Alan, could you please share with us the version of binutils you were using 
> in your test ?
> 
> Regards, Vladimir

I have updated PR with patch for hsdis, one thing to notice, LP64 is not 
defined for arm64 in Makefile

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

[Anton Kozlov]

> Please review the implementation of JEP 391: macOS/AArch64 Port.
> 
> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
> windows/aarch64. 
> 
> Major changes are in:
> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
> JDK-8253817, JDK-8253818)
> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary 
> adjustments (JDK-8253819)
> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
> required on macOS/AArch64 platform. It's implemented with 
> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
> thread, so W^X mode change relates to the java thread state change (for java 
> threads). In most cases, JVM executes in write-only mode, except when calling 
> a generated stub like SafeFetch, which requires a temporary switch to 
> execute-only mode. The same execute-only mode is enabled when a java thread 
> executes in java or native states. This approach of managing W^X mode turned 
> out to be simple and efficient enough.
> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)

Anton Kozlov has updated the pull request incrementally with one additional 
commit since the last revision:

  support macos_aarch64 in hsdis

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2200/files
  - new: https://git.openjdk.java.net/jdk/pull/2200/files/b421e0b4..3c705ae5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2200&range=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2200&range=07-08

  Stats: 8 lines in 1 file changed: 6 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2200.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

[Vladimir Kempik]

On Mon, 1 Feb 2021 14:06:32 GMT, Magnus Ihse Bursie  wrote:

>>> Hello, hsdis is a separate out-of-tree project and is not part of this jep.
>> 
>> Unless there's something I'm missing it only requires a few lines of change 
>> to src/utils/hsdis/makefile (it already has support for macos x86_64)
>> 
>>>support for looking for proper hsdis dylib library was added as part of this 
>>>jep.
>> 
>> I'm a little confused. Are you planning on adding a new disassembler?
>
>> > Hello, hsdis is a separate out-of-tree project and is not part of this jep.
>> 
>> Unless there's something I'm missing it only requires a few lines of change 
>> to src/utils/hsdis/makefile (it already has support for macos x86_64)
> 
> I agree with Alan that it makes sense to add this trivial change as part of 
> this PR, if it allows the current hsdis solution to continue working on 
> mac/aarch64.
> 
>> 
>> > support for looking for proper hsdis dylib library was added as part of 
>> > this jep.
>> 
>> I'm a little confused. Are you planning on adding a new disassembler?
> 
> @a74nh I think Vladimir is referring to 
> https://github.com/openjdk/jdk/pull/392. The hsdis "component" has been left 
> behind for a long time, and there are several requests to add new backends, 
> which require a modernized build of hsdis. This is undfortunately not a 
> high-priority project, and has been postponed several times already. :(

> > > Hello, hsdis is a separate out-of-tree project and is not part of this 
> > > jep.
> > 
> > 
> > Unless there's something I'm missing it only requires a few lines of change 
> > to src/utils/hsdis/makefile (it already has support for macos x86_64)
> 
> I agree with Alan that it makes sense to add this trivial change as part of 
> this PR, if it allows the current hsdis solution to continue working on 
> mac/aarch64.
> 
> > > support for looking for proper hsdis dylib library was added as part of 
> > > this jep.
> > 
> > 
> > I'm a little confused. Are you planning on adding a new disassembler?
> 
> @a74nh I think Vladimir is referring to #392. The hsdis "component" has been 
> left behind for a long time, and there are several requests to add new 
> backends, which require a modernized build of hsdis. This is undfortunately 
> not a high-priority project, and has been postponed several times already. :(

Sorry I was under impression hsdis is not part of openjdk tree.

Alan, could you please share with us the version of binutils you were using in 
your test ?

Regards, Vladimir

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: AArch64 OpenJDK bootstrap failure on head

[Thomas Stüfe]

On Mon, Feb 1, 2021 at 10:11 PM Ioi Lam  wrote:

> On 2/1/21 9:36 AM, Thomas Stüfe wrote:
>
> This does not solve the alignment problem, but I don't like that we
> unconditionally print a message here since this is a non-fatal error. Also,
> CDS mapping may fail for other reasons, for which we do not print
> unconditionally. I think we should make this info log level:
>
> --- a/src/hotspot/share/memory/metaspaceShared.cpp
> +++ b/src/hotspot/share/memory/metaspaceShared.cpp
> @@ -1725,7 +1725,7 @@ MapArchiveResult
> MetaspaceShared::map_archive(FileMapInfo* mapinfo, char* mapped
>mapinfo->set_is_mapped(false);
>
>if (mapinfo->alignment() != (size_t)os::vm_allocation_granularity()) {
> -log_error(cds)("Unable to map CDS archive --
> os::vm_allocation_granularity() expected: " SIZE_FORMAT
> +log_info(cds)("Unable to map CDS archive --
> os::vm_allocation_granularity() expected: " SIZE_FORMAT
> " actual: %d", mapinfo->alignment(),
> os::vm_allocation_granularity());
>  return MAP_ARCHIVE_OTHER_FAILURE;
>}
>
> @ Ioi, does that make sense?
>
>
> Yes, your fix makes sense.
>
>
Thanks. https://github.com/openjdk/jdk/pull/2348


> This issue is being address in
> https://bugs.openjdk.java.net/browse/JDK-8236847. We will probably
> unconditionally change the alignment to 64KB for AARCH64, as well as MacOS
> (so that you can run a X64 JDK on M1 using Rosetta).
>
>
I thought so too. I also see it being used for region-to-region alignment,
where I believe page size would be sufficient?

.:Thomas

Thanks
> - Ioi
>
> Cheers, Thomas
>
>
> On Mon, Feb 1, 2021 at 6:20 PM Andrew Haley  wrote:
>
>> On 2/1/21 5:14 PM, Aleksey Shipilev wrote:
>> > On 2/1/21 4:38 PM, Andrew Haley wrote:
>> >> but that doesn't work either. Any ideas? I'm really stuck.
>> >
>> > Did you "make clean" after changing any of the configure files and/or
>> configure arguments? I.e. did
>> > AWTIcon32_java_icon16_png actually regenerate?
>>
>> Many times.
>>
>> > Prepending the build with "LOG=debug" would tell what cmdlines are used
>> at every step of build process.
>>
>> Sure, I can see what it's doing. I think this is actually a regression
>> caused by the Windows-AArch64 port. I'll do some bisecting.
>>
>> --
>> Andrew Haley  (he/him)
>> Java Platform Lead Engineer
>> Red Hat UK Ltd. > 
>> >
>> https://keybase.io/andrewhaley
>> 
>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>>
>>
>

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

[Magnus Ihse Bursie]

On Mon, 1 Feb 2021 20:10:58 GMT, Ioi Lam  wrote:

>> - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which 
>> allowed the same JDK library to use different version of HotSpot. However, 
>> HSX is no longer supported so this API should be removed.
>> - Implementations of APIs such as JVM_DTraceActivate, were removed in 
>> [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their 
>> declarations should be removed from jvm.h
>
> Ioi Lam has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   fixed macos build

"Build" change looks good.

-

Marked as reviewed by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2338

Re: RFR: 8257858: [macOS]: Remove JNF dependency from libosxsecurity/KeystoreImpl.m [v7]

[Magnus Ihse Bursie]

On Mon, 1 Feb 2021 16:51:12 GMT, Weijun Wang  wrote:

>> This fix covers both
>> 
>> - [[macOS]: Remove JNF dependency from 
>> libosxsecurity/KeystoreImpl.m](https://bugs.openjdk.java.net/browse/JDK-8257858)
>> - [[macOS]: Remove JNF dependency from 
>> libosxkrb5/SCDynamicStoreConfig.m](https://bugs.openjdk.java.net/browse/JDK-8257860)
>
> Weijun Wang has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - file attr error
>  - objc use .m

Looks good now!

-

Marked as reviewed by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1845