Re: [jdk16] RFR: 8258378: Final nroff manpage update for JDK 16

[Magnus Ihse Bursie]

On Mon, 1 Feb 2021 22:46:15 GMT, David Holmes  wrote:

>> Before RC phase we need to ensure we have the final set of manpage updates 
>> published in OpenJDK.
>
> Thanks for doing this Magnus! I had overlooked the fact we would need to 
> re-run this in 2021 regardless of whether there were any content changes to 
> propagate.
> 
> Cheers,
> David

@dholmes-ora I did not think of that myself either. But the make target did the 
Right Thing. :-)

-

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

[jdk16] Integrated: 8258378: Final nroff manpage update for JDK 16

[Magnus Ihse Bursie]

On Mon, 1 Feb 2021 11:09:25 GMT, Magnus Ihse Bursie  wrote:

> Before RC phase we need to ensure we have the final set of manpage updates 
> published in OpenJDK.

This pull request has now been integrated.

Changeset: ed1a7755
Author:Magnus Ihse Bursie 
URL:   https://git.openjdk.java.net/jdk16/commit/ed1a7755
Stats: 238 lines in 27 files changed: 98 ins; 111 del; 29 mod

8258378: Final nroff manpage update for JDK 16

Reviewed-by: erikj, dholmes

-

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

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

[Xue-Lei Andrew Fan]

On Mon, 1 Feb 2021 18:49:04 GMT, djelinski 
 wrote:

>> Under certain load, MemoryCache operations take a substantial fraction of 
>> the time needed to complete SSL handshakes. This series of patches improves 
>> performance characteristics of MemoryCache, at the cost of a functional 
>> change: expired entries are no longer guaranteed to be removed before live 
>> ones. Unused entries are still removed before used ones, and cache 
>> performance no longer depends on its capacity.
>> 
>> First patch in the series contains a benchmark that can be run with `make 
>> test TEST="micro:CacheBench"`.
>> Baseline results before any MemoryCache changes:
>> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
>> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
>> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
>> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
>> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
>> there's a nonlinear performance drop between 20480 and 204800 entries, 
>> probably attributable to CPU cache thrashing. Beyond 204800 entries the 
>> cache scales more linearly.
>> 
>> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
>> only copy one:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
>> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
>> The third patch improves worst-case times on a mostly idle cache by 
>> scattering removal of expired entries over multiple `put` calls. It does not 
>> affect performance of an overloaded cache.
>> 
>> The 4th patch removes all code that clears cached values before handing them 
>> over to the GC. [This 
>> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>>  stated that clearing values was supposed to be a GC performance 
>> optimization. It wasn't. Benchmark results after that commit:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
>> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
>> I wasn't expecting that much of an improvement, and don't know how to 
>> explain it.
>> 
>> The 40ns difference between cache with and without a timeout can be 
>> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
>> my VM.
>
> djelinski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Simplify makefile

If I get the patch right, the benchmark performance improvement is a trade-off 
between CPU and memory, by keeping expired entries while putting a new entry in 
the cache.  I'm not very sure of the performance impact on memory and GC 
collections.  Would you mind add two more types of benchmark: get the entries 
and remove the entries, for cases that there are 1/10, 1/4, 1/3 and 1/2 expired 
entries in the cache?  And increase the size to some big scales, like 2M and 
20M.

It looks like a spec update as it may change the behavior of a few JDK 
components (TLS session cache, OCSP stapling response cache, cert store cache, 
certificate factory, etc), because of "expired entries are no longer guaranteed 
to be removed before live ones".  I'm not very sure of the impact. I may 
suggest to file a CSR and have more eyes to check the compatibility impact 
before moving forward.

-

Changes requested by xuelei (Reviewer).

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

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

[Xue-Lei Andrew Fan]

On Mon, 1 Feb 2021 22:45:17 GMT, Claes Redestad  wrote:

>> Build change looks good, but I would like to hear from @cl4es too.
>
> Adding an `--add-exports` to `JAVAC_FLAGS` is a bit iffy, but should be OK. 
> Yes, all benchmarks will now be compiled with that package exported and 
> visible, but that should have no unintentional effect on other compilations.

The impact could beyond the JSSE implementation, andI will have a look as well.

-

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

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

[Phil Race]

On Mon, 1 Feb 2021 22:17:38 GMT, Sergey Bylokhov  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8260616: Removing remaining JNF dependencies in the java.desktop module
>
> 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.

> 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.

> src/java.desktop/macosx/native/libosxapp/JNIUtilities.m line 30:
> 
>> 28: NSString* JavaStringToNSString(JNIEnv *env, jstring jstr) {
>> 29: if (jstr == NULL) {
>> 30: return NULL;
> 
> In other methods, the nil is used

Not understanding what you mean ? This is the same behavior as the function 
being replaced.

> 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.

-

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 19:09:59 GMT, Phil Race  wrote:

>> 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 incrementally with one additional 
> commit since the last revision:
> 
>   8260616: Removing remaining JNF dependencies in the java.desktop module

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?

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?

src/java.desktop/macosx/native/libosxapp/JNIUtilities.m line 30:

> 28: NSString* JavaStringToNSString(JNIEnv *env, jstring jstr) {
> 29: if (jstr == NULL) {
> 30: return NULL;

In other methods, the nil is used

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.

-

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

Re: [jdk16] RFR: 8258378: Final nroff manpage update for JDK 16

[David Holmes]

On Mon, 1 Feb 2021 11:09:25 GMT, Magnus Ihse Bursie  wrote:

> Before RC phase we need to ensure we have the final set of manpage updates 
> published in OpenJDK.

Thanks for doing this Magnus! I had overlooked the fact we would need to re-run 
this in 2021 regardless of whether there were any content changes to propagate.

Cheers,
David

-

Marked as reviewed by dholmes (Reviewer).

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

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

[Claes Redestad]

On Mon, 1 Feb 2021 19:22:22 GMT, Erik Joelsson  wrote:

>> djelinski has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Simplify makefile
>
> Build change looks good, but I would like to hear from @cl4es too.

Adding an `--add-exports` to `JAVAC_FLAGS` is a bit iffy, but should be OK. 
Yes, all benchmarks will now be compiled with that package exported and 
visible, but that should have no unintentional effect on other compilations.

-

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

Re: AArch64 OpenJDK bootstrap failure on head

[Ioi Lam]

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.

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).


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]

[Ioi Lam]

On Mon, 1 Feb 2021 20:29:10 GMT, Gerard Ziemski  wrote:

>> Ioi Lam has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fixed macos build
>
> 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?

-

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: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

Changes requested by gziemski (Committer).

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) ?

-

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

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

[Ioi Lam]

> - 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2338/files
  - new: https://git.openjdk.java.net/jdk/pull/2338/files/c0307e7d..3a6415eb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2338=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2338=00-01

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

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

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

[Erik Joelsson]

On Mon, 1 Feb 2021 18:49:04 GMT, djelinski 
 wrote:

>> Under certain load, MemoryCache operations take a substantial fraction of 
>> the time needed to complete SSL handshakes. This series of patches improves 
>> performance characteristics of MemoryCache, at the cost of a functional 
>> change: expired entries are no longer guaranteed to be removed before live 
>> ones. Unused entries are still removed before used ones, and cache 
>> performance no longer depends on its capacity.
>> 
>> First patch in the series contains a benchmark that can be run with `make 
>> test TEST="micro:CacheBench"`.
>> Baseline results before any MemoryCache changes:
>> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
>> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
>> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
>> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
>> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
>> there's a nonlinear performance drop between 20480 and 204800 entries, 
>> probably attributable to CPU cache thrashing. Beyond 204800 entries the 
>> cache scales more linearly.
>> 
>> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
>> only copy one:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
>> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
>> The third patch improves worst-case times on a mostly idle cache by 
>> scattering removal of expired entries over multiple `put` calls. It does not 
>> affect performance of an overloaded cache.
>> 
>> The 4th patch removes all code that clears cached values before handing them 
>> over to the GC. [This 
>> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>>  stated that clearing values was supposed to be a GC performance 
>> optimization. It wasn't. Benchmark results after that commit:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
>> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
>> I wasn't expecting that much of an improvement, and don't know how to 
>> explain it.
>> 
>> The 40ns difference between cache with and without a timeout can be 
>> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
>> my VM.
>
> djelinski has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Simplify makefile

Build change looks good, but I would like to hear from @cl4es too.

-

Marked as reviewed by erikj (Reviewer).

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

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]

[erik . joelsson]

On 2021-02-01 10:38, Alexey Semenyuk wrote:

On Mon, 1 Feb 2021 18:24:23 GMT, Erik Joelsson  wrote:


"common" was perfectly enough until this change. Unfortunately we cant just drop new C 
sources in "common" dir because we don't want them to be compiled with g++. That is why 
need new common directory (applauncherlibcommon) for C sources.

I'll rename applauncherlibcommon in libapplaunchercommon and applauncherlib in 
libapplauncher in the next commit.
"common" was perfectly enough until this change. Unfortunately we cant just drop new C 
sources in "common" dir because we don't want them to be compiled with g++. That is why 
need new common directory (applauncherlibcommon) for C sources.

We pick compiler based on file suffix, not directory, so it shouldn't matter 
where you put a .c file, it should always be compiled with gcc and .cpp files 
with g++. Which compiler is used to launch the linker can however differ. 
That's configured for each SetupNativeCompilation call with the different 
TOOLCHAIN settings.

Erik, thank you for explanation.

The launcher on Linux should not be linked with c++ runtime, that is why 
TOOLCHAIN_DEFAULT is used at a value for TOOLCHAIN property in 
BUILD_JPACKAGE_APPLAUNCHEREXE target on Linux.

Will SetupNativeCompilation work if `TOOLCHAIN=TOOLCHAIN_DEFAULT` and there are 
.cpp sources are in directories passed in `SRC` property of 
SetupNativeCompilation? Will it try to compile these sources? If it will ignore 
them and pick only .c files, that would be perfect.


SetupNativeCompilation will by default include all src files found in 
any directory given to SRC, recursively. You can use EXCLUDES, 
EXCLUDE_FILES and EXCLUDE_PATTERN to exclude files or directories from 
SRC. You can also use EXTRA_FILES to pick specific files outside of any 
directory in SRC. Sorting files in separate directories or using 
EXCLUDE*/EXTRA_FILES are both possible and picking the right solution is 
mostly down to taste.


/Erik


-

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

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

[Philip Race]

I can confirm that the autoconf warning disabling is currently still 
needed when building with our standard devkit.


It'll have to be removed at the very end.

In file included from 
./open/src/java.security.jgss/macosx/native/libosxkrb5/SCDynamicStoreConfig.m:27:

.

/System/Library/Frameworks/JavaVM.framework/Frameworks/JavaNativeFoundation.framework/Headers/JNFJNI.h:30:
/System/Volumes/Data/mesos/work_dir/jib-master/install/jpg/infra/builddeps/devkit-macosx_x64/System/Library/Frameworks/JavaVM.framework/Frameworks/JavaNativeFoundation.framework/Headers/JNFException.h:49:1: 
error: method has no return type specified; defaults to 'id' 
[-Werror,-Wmissing-method-return-type]

 - init:(JNIEnv *)env throwable:(jthrowable)throwable;
 ^
  (id)
/System/Library/Frameworks/JavaVM.framework/Frameworks/JavaNativeFoundation.framework/Headers/JNFException.h:50:1: 
error: method has no return type specified; defaults to 'id' 
[-Werror,-Wmissing-method-return-type]
 - init:(JNIEnv *)env as:(const char *)javaExceptionType reason:(const 
char *)reasonMsg;

 ^

-phil.

On 2/1/21 8:02 AM, Philip Race wrote:
Per my comment in the PR I am currently working on updating it to 
handle the test update needed.


Once the other in-flight PRs are integrated, my grepping says that the 
only remaining build change


needed after that is to remove one disabled warning in 
make/autoconf/flags-cflags.m4


I am going to try to find out if we can remove that now or have to 
wait until all JNF is removed.


FWIW it looks like you added it here : 
http://hg.openjdk.java.net/jdk/jdk/rev/ec62d6cab037


If we can roll this into an existing PR that might be easier. If it 
needs to wait for all of them, then we'll play it by ear as to whether 
to add it fo the last man standing or file a new bug.


-phil

On 2/1/21 3:33 AM, Magnus Ihse Bursie wrote:

On 2021-01-29 17:41, Phil Race wrote:
But we can just remove it and prove it - but probably a separate PR 
since it is nothing to do with the desktop module and the autoconf 
code needs to be updated once everything else is in.
Fair enough. Do you have a JBS issue tracking the remaining build 
system fixes for the JNF removal? I looked around but could not find 
one.


/Magnus

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

[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 incrementally with one additional commit 
since the last revision:

  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/efab1de5..7ea57c85

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2305=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2305=01-02

  Stats: 46 lines in 3 files changed: 34 ins; 1 del; 11 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 [v8]

[Andrew Haley]

On Sun, 31 Jan 2021 20:14:01 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 with a new target base due to a 
> merge or a rebase. The pull request now contains 62 commits:
> 
>  - Merge branch 'master' into jdk-macos
>  - Update copyright year for BsdAARCH64ThreadContext.java
>  - Fix inclusing of StubRoutines header
>  - Redo buildsys fix
>  - Revert harfbuzz changes, disable warnings for it
>  - Little adjustement of SlowSignatureHandler
>  - Partially bring previous commit
>  - Revert "Address feedback for signature generators"
>
>This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d.
>  - Refactor CDS disabling
>  - Redo builsys support for aarch64-darwin
>  - ... and 52 more: 
> https://git.openjdk.java.net/jdk/compare/8a9004da...b421e0b4

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

> 82: // on stack. Natural alignment for types are still in place,
> 83: // for example double/long should be 8 bytes alligned
> 84: 

This comment is a bit confusing because it's no longer #ifdef APPLE. Better 
move it up to Line 41.

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

> 350: 
> 351: #ifdef __APPLE__
> 352:   virtual void pass_byte()

Please remove ```#ifdef __APPLE__``` around this region.

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

> 837:   // The code unable to handle this, bailout.
> 838:   return -1;
> 839: #endif

This looks like a bug to me. The caller doesn't necessarily check the return 
value. See CallRuntimeNode::calling_convention.

-

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

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

[djelinski]

On Mon, 1 Feb 2021 18:35:56 GMT, djelinski 
 wrote:

>> Hm, maybe you just misunderstand how this makefile construct works. If you 
>> just want to add "--add-exports java.base/sun.security.util=ALL-UNNAMED", 
>> then that's all you should put in this assignment.
>
> yeah, I'm new to makefiles. Let me try that...

Removed. I could have sworn I tried this... but apparently I didn't. Thanks for 
the suggestion!

-

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

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

[djelinski]

> Under certain load, MemoryCache operations take a substantial fraction of the 
> time needed to complete SSL handshakes. This series of patches improves 
> performance characteristics of MemoryCache, at the cost of a functional 
> change: expired entries are no longer guaranteed to be removed before live 
> ones. Unused entries are still removed before used ones, and cache 
> performance no longer depends on its capacity.
> 
> First patch in the series contains a benchmark that can be run with `make 
> test TEST="micro:CacheBench"`.
> Baseline results before any MemoryCache changes:
> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
> there's a nonlinear performance drop between 20480 and 204800 entries, 
> probably attributable to CPU cache thrashing. Beyond 204800 entries the cache 
> scales more linearly.
> 
> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
> only copy one:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
> The third patch improves worst-case times on a mostly idle cache by 
> scattering removal of expired entries over multiple `put` calls. It does not 
> affect performance of an overloaded cache.
> 
> The 4th patch removes all code that clears cached values before handing them 
> over to the GC. [This 
> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>  stated that clearing values was supposed to be a GC performance 
> optimization. It wasn't. Benchmark results after that commit:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
> I wasn't expecting that much of an improvement, and don't know how to explain 
> it.
> 
> The 40ns difference between cache with and without a timeout can be 
> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
> my VM.

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

  Simplify makefile

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2255/files
  - new: https://git.openjdk.java.net/jdk/pull/2255/files/5859a032..34949970

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2255=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2255=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2255.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2255/head:pull/2255

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

Re: RFR: 8259886 : Improve SSL session cache performance and scalability

[djelinski]

On Mon, 1 Feb 2021 18:31:39 GMT, Erik Joelsson  wrote:

>> Adding "--add-exports java.base/sun.security.util=ALL-UNNAMED" is fine, I'm 
>> asking about "$(JAVAC_FLAGS)".
>
> Hm, maybe you just misunderstand how this makefile construct works. If you 
> just want to add "--add-exports java.base/sun.security.util=ALL-UNNAMED", 
> then that's all you should put in this assignment.

yeah, I'm new to makefiles. Let me try that...

-

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

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]

[Alexey Semenyuk]

On Mon, 1 Feb 2021 18:24:23 GMT, Erik Joelsson  wrote:

>> "common" was perfectly enough until this change. Unfortunately we cant just 
>> drop new C sources in "common" dir because we don't want them to be compiled 
>> with g++. That is why need new common directory (applauncherlibcommon) for C 
>> sources.
>> 
>> I'll rename applauncherlibcommon in libapplaunchercommon and applauncherlib 
>> in libapplauncher in the next commit.
>
>> "common" was perfectly enough until this change. Unfortunately we cant just 
>> drop new C sources in "common" dir because we don't want them to be compiled 
>> with g++. That is why need new common directory (applauncherlibcommon) for C 
>> sources.
> 
> We pick compiler based on file suffix, not directory, so it shouldn't matter 
> where you put a .c file, it should always be compiled with gcc and .cpp files 
> with g++. Which compiler is used to launch the linker can however differ. 
> That's configured for each SetupNativeCompilation call with the different 
> TOOLCHAIN settings.

Erik, thank you for explanation.

The launcher on Linux should not be linked with c++ runtime, that is why 
TOOLCHAIN_DEFAULT is used at a value for TOOLCHAIN property in 
BUILD_JPACKAGE_APPLAUNCHEREXE target on Linux.

Will SetupNativeCompilation work if `TOOLCHAIN=TOOLCHAIN_DEFAULT` and there are 
.cpp sources are in directories passed in `SRC` property of 
SetupNativeCompilation? Will it try to compile these sources? If it will ignore 
them and pick only .c files, that would be perfect.

-

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

Re: RFR: 8259886 : Improve SSL session cache performance and scalability

[Erik Joelsson]

On Mon, 1 Feb 2021 18:30:14 GMT, Erik Joelsson  wrote:

>> I'm trying to benchmark a class that is in a non-exported package 
>> `sun.security.util`. Without this line the benchmark doesn't compile. I 
>> couldn't find any other benchmarks that access non-exported classes, so I 
>> came up with my own implementation. 
>> 
>> Is there a better way to get the benchmark to compile?
>
> Adding "--add-exports java.base/sun.security.util=ALL-UNNAMED" is fine, I'm 
> asking about "$(JAVAC_FLAGS)".

Hm, maybe you just misunderstand how this makefile construct works. If you just 
want to add "--add-exports java.base/sun.security.util=ALL-UNNAMED", then 
that's all you should put in this assignment.

-

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

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

[Gerard Ziemski]

On Mon, 1 Feb 2021 18:31:23 GMT, Gerard Ziemski  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8260616: Removing remaining JNF dependencies in the java.desktop modul
>
> Marked as reviewed by gziemski (Committer).

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

-

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

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

[Gerard Ziemski]

On Fri, 29 Jan 2021 19:53:32 GMT, Phil Race  wrote:

>> src/java.desktop/macosx/native/libosxapp/JNIUtilities.m line 83:
>> 
>>> 81:   stringWithFileSystemRepresentation:chs length:len];
>>> 82: return result;
>>> 83: }
>> 
>> Why are we doing:
>> 
>> `java_string -> chars -> NSString -> chars -> [NSFileManager 
>> stringWithFileSystemRepresentation]`
>> 
>> ?
>> 
>> Why not just get the raw characters form JNI and feed them directly into 
>> `[NSFileManager  stringWithFileSystemRepresentation]`, ie:
>> 
>> `java_string -> chars -> [NSFileManager stringWithFileSystemRepresentation]`
>> 
>> and skip the `JavaStringToNSString` step altogether?
>
> I tried to explain the odd-ness of this in the comments preceding the 
> function definition.
> Near as I could figure out that intermediate call is needed to do the 
> decomposition since the NSFileManager won't do that.
> But this is not exactly my area of expertise and there may be a better way. 
> Who is an expert on the intricacies of the macOS file system naming ?

How about a comment like:

/*
 Returns an NSString in decomposed UTF16 format that is compatible with HFS's
 expectation of the UTF16 format for file system paths.
 
 Example string: "/Users/Amélie/"
 
 Java's UTF16 string is "/ U s e r s / A m \351 l i e /"
 macOS UTF16 string suitable for HFS is "/ U s e r s / A m e \314 \201 l i e /"
 
 There is no direct API that takes in NSString UTF16 encoded by Java
 and produces NSString UTF16 for HFS, so we first need to decompose it
 into chars (suitable for low level C file APIs), and only then
 create NSString representation of this decomposition back into UTF16 string.
 */

?

I borrowed the API description from Apple's original 
`JNFNormalizedNSStringForPath` API, but added an example and different 
description.

-

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

Re: RFR: 8259886 : Improve SSL session cache performance and scalability

[Erik Joelsson]

On Mon, 1 Feb 2021 18:24:46 GMT, djelinski 
 wrote:

>> make/test/BuildMicrobenchmark.gmk line 97:
>> 
>>> 95: SRC := $(MICROBENCHMARK_SRC), \
>>> 96: BIN := $(MICROBENCHMARK_CLASSES), \
>>> 97: JAVAC_FLAGS := $(JAVAC_FLAGS) --add-exports 
>>> java.base/sun.security.util=ALL-UNNAMED, \
>> 
>> Why do you need to add $(JAVAC_FLAGS) here?
>
> I'm trying to benchmark a class that is in a non-exported package 
> `sun.security.util`. Without this line the benchmark doesn't compile. I 
> couldn't find any other benchmarks that access non-exported classes, so I 
> came up with my own implementation. 
> 
> Is there a better way to get the benchmark to compile?

Adding "--add-exports java.base/sun.security.util=ALL-UNNAMED" is fine, I'm 
asking about "$(JAVAC_FLAGS)".

-

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

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

[Gerard Ziemski]

On Fri, 29 Jan 2021 17:24:05 GMT, Phil Race  wrote:

>> 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 incrementally with one additional 
> commit since the last revision:
> 
>   8260616: Removing remaining JNF dependencies in the java.desktop modul

Marked as reviewed by gziemski (Committer).

-

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

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

[Erik Joelsson]

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

Build changes look good. Thanks for fixing the .m support in 
TestFilesCompilation.gmk!

-

Marked as reviewed by erikj (Reviewer).

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

Re: RFR: 8259886 : Improve SSL session cache performance and scalability

[djelinski]

On Mon, 1 Feb 2021 18:18:51 GMT, Erik Joelsson  wrote:

>> Under certain load, MemoryCache operations take a substantial fraction of 
>> the time needed to complete SSL handshakes. This series of patches improves 
>> performance characteristics of MemoryCache, at the cost of a functional 
>> change: expired entries are no longer guaranteed to be removed before live 
>> ones. Unused entries are still removed before used ones, and cache 
>> performance no longer depends on its capacity.
>> 
>> First patch in the series contains a benchmark that can be run with `make 
>> test TEST="micro:CacheBench"`.
>> Baseline results before any MemoryCache changes:
>> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
>> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
>> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
>> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
>> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
>> there's a nonlinear performance drop between 20480 and 204800 entries, 
>> probably attributable to CPU cache thrashing. Beyond 204800 entries the 
>> cache scales more linearly.
>> 
>> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
>> only copy one:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
>> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
>> The third patch improves worst-case times on a mostly idle cache by 
>> scattering removal of expired entries over multiple `put` calls. It does not 
>> affect performance of an overloaded cache.
>> 
>> The 4th patch removes all code that clears cached values before handing them 
>> over to the GC. [This 
>> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>>  stated that clearing values was supposed to be a GC performance 
>> optimization. It wasn't. Benchmark results after that commit:
>> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
>> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
>> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
>> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
>> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
>> I wasn't expecting that much of an improvement, and don't know how to 
>> explain it.
>> 
>> The 40ns difference between cache with and without a timeout can be 
>> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
>> my VM.
>
> make/test/BuildMicrobenchmark.gmk line 97:
> 
>> 95: SRC := $(MICROBENCHMARK_SRC), \
>> 96: BIN := $(MICROBENCHMARK_CLASSES), \
>> 97: JAVAC_FLAGS := $(JAVAC_FLAGS) --add-exports 
>> java.base/sun.security.util=ALL-UNNAMED, \
> 
> Why do you need to add $(JAVAC_FLAGS) here?

I'm trying to benchmark a class that is in a non-exported package 
`sun.security.util`. Without this line the benchmark doesn't compile. I 
couldn't find any other benchmarks that access non-exported classes, so I came 
up with my own implementation. 

Is there a better way to get the benchmark to compile?

-

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

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]

[Erik Joelsson]

On Mon, 1 Feb 2021 16:17:35 GMT, Alexey Semenyuk  wrote:

> "common" was perfectly enough until this change. Unfortunately we cant just 
> drop new C sources in "common" dir because we don't want them to be compiled 
> with g++. That is why need new common directory (applauncherlibcommon) for C 
> sources.

We pick compiler based on file suffix, not directory, so it shouldn't matter 
where you put a .c file, it should always be compiled with gcc and .cpp files 
with g++. Which compiler is used to launch the linker can however differ. 
That's configured for each SetupNativeCompilation call with the different 
TOOLCHAIN settings.

-

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

Re: RFR: 8259886 : Improve SSL session cache performance and scalability

[Erik Joelsson]

On Wed, 27 Jan 2021 11:32:08 GMT, djelinski 
 wrote:

> Under certain load, MemoryCache operations take a substantial fraction of the 
> time needed to complete SSL handshakes. This series of patches improves 
> performance characteristics of MemoryCache, at the cost of a functional 
> change: expired entries are no longer guaranteed to be removed before live 
> ones. Unused entries are still removed before used ones, and cache 
> performance no longer depends on its capacity.
> 
> First patch in the series contains a benchmark that can be run with `make 
> test TEST="micro:CacheBench"`.
> Baseline results before any MemoryCache changes:
> Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
> CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
> CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
> CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
> CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
> there's a nonlinear performance drop between 20480 and 204800 entries, 
> probably attributable to CPU cache thrashing. Beyond 204800 entries the cache 
> scales more linearly.
> 
> Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
> only copy one:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
> CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
> The third patch improves worst-case times on a mostly idle cache by 
> scattering removal of expired entries over multiple `put` calls. It does not 
> affect performance of an overloaded cache.
> 
> The 4th patch removes all code that clears cached values before handing them 
> over to the GC. [This 
> comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
>  stated that clearing values was supposed to be a GC performance 
> optimization. It wasn't. Benchmark results after that commit:
> Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
> CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
> CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
> CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
> CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
> I wasn't expecting that much of an improvement, and don't know how to explain 
> it.
> 
> The 40ns difference between cache with and without a timeout can be 
> attributed to 2 `System.currentTimeMillis()` calls; they were pretty slow on 
> my VM.

make/test/BuildMicrobenchmark.gmk line 97:

> 95: SRC := $(MICROBENCHMARK_SRC), \
> 96: BIN := $(MICROBENCHMARK_CLASSES), \
> 97: JAVAC_FLAGS := $(JAVAC_FLAGS) --add-exports 
> java.base/sun.security.util=ALL-UNNAMED, \

Why do you need to add $(JAVAC_FLAGS) here?

-

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

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v3]

[Alexey Semenyuk]

> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702
> 
> The fix splits Linux app launcher in app launcher and launcher shared lib. 
> App launcher is pure C and doesn't have C++ code. App launcher lib 
> incorporates bulk of C++ code from app launcher. 
> At startup app launcher loads launcher shared lib and calls functions it 
> provides to get data for launching JVM (path to jli lib and arguments for 
> JLI_Launch function call).
> App launcher unloads launcher shared lib before launching JVM to remove C++ 
> runtime from the process memory.
> 
> Getting rid of C++ code from app launcher required to rewrite app 
> installation location lookup code from C++ to C. LinuxPackage.c source is C 
> alternative for 
> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp
>  and 
> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp.
> 
> Layout of jpackage's native code changed:
> - `common`: code shared between all jpackage binaries.
> - `applauncher`: launcher only code.
> - `applauncherlib`: launcher lib code on Linux and launcher code on other 
> platforms.
> - `applaunchercommon`: code shared between launcher and launcher lib on Linux 
> and launcher code on other platforms.

Alexey Semenyuk has updated the pull request incrementally with one additional 
commit since the last revision:

  8254702: jpackage app launcher crashes on CentOS

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2320/files
  - new: https://git.openjdk.java.net/jdk/pull/2320/files/b493bcfd..91744255

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2320=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2320=01-02

  Stats: 9 lines in 13 files changed: 0 ins; 9 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2320.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2320/head:pull/2320

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

Re: AArch64 OpenJDK bootstrap failure on head

[Thomas Stüfe]

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?

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: AArch64 OpenJDK bootstrap failure on head

[Andrew Haley]

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: AArch64 OpenJDK bootstrap failure on head

[Aleksey Shipilev]

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?


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

--
Thanks,
-Aleksey

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

[Weijun Wang]

On Mon, 1 Feb 2021 11:41:02 GMT, Magnus Ihse Bursie  wrote:

>> Weijun Wang 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 15 additional 
>> commits since the last revision:
>> 
>>  - move test
>>  - Merge branch 'master' into 8257858
>>  - a test
>>
>>only in patch2:
>>unchanged:
>>  - end values should be vectors
>>  - phil comment
>>  - same behavior as before -- empty realm map
>>  - error check, new JavaStringToNSString
>>  - do not find class and method in loop
>>  - no more header file
>>
>>reverted:
>>  - better macro, no more JNI_COCOA_ENTER
>>  - ... and 5 more: 
>> https://git.openjdk.java.net/jdk/compare/81433b38...ef337f12
>
> make/test/JtregNativeJdk.gmk line 84:
> 
>> 82:   -framework Cocoa -framework JavaNativeFoundation
>> 83:   BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeJniInvocationTest := -ljli
>> 84:   BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libTestDynamicStore := -ObjC
> 
> Instead of "tricking" the build system of compiling an Obj-C file by 
> masquerading it as a C file and passing compiler options, you should expose 
> the test file for what it is, and add support in the build system to handle 
> this. I can help you with that part.

Fixed.

-

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

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

[Weijun Wang]

> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1845/files
  - new: https://git.openjdk.java.net/jdk/pull/1845/files/ef337f12..b5f1e15d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1845=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1845=05-06

  Stats: 6 lines in 4 files changed: 0 ins; 1 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1845.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1845/head:pull/1845

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

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]

[Alexey Semenyuk]

On Mon, 1 Feb 2021 12:19:56 GMT, Magnus Ihse Bursie  wrote:

>> Alexey Semenyuk has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR. The pull request 
>> contains one new commit since the last revision:
>> 
>>   8254702: jpackage app launcher crashes on CentOS
>
> Changes requested by ihse (Reviewer).

"common" was perfectly enough until this change. Unfortunately we cant just 
drop new C sources in "common" dir because we don't want them to be compiled 
with g++. That is why need new common directory (applauncherlibcommon) for C 
sources.

I'll rename applauncherlibcommon in libapplaunchercommon and applauncherlib in 
libapplauncher in the next commit.

-

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

RFR: 8259886 : Improve SSL session cache performance and scalability

[djelinski]

Under certain load, MemoryCache operations take a substantial fraction of the 
time needed to complete SSL handshakes. This series of patches improves 
performance characteristics of MemoryCache, at the cost of a functional change: 
expired entries are no longer guaranteed to be removed before live ones. Unused 
entries are still removed before used ones, and cache performance no longer 
depends on its capacity.

First patch in the series contains a benchmark that can be run with `make test 
TEST="micro:CacheBench"`.
Baseline results before any MemoryCache changes:
Benchmark   (size)  (timeout)  Mode  Cnt ScoreError  Units
CacheBench.put   20480  86400  avgt   2583.653 ?  6.269  us/op
CacheBench.put   20480  0  avgt   25 0.107 ?  0.001  us/op
CacheBench.put  204800  86400  avgt   25  2057.781 ? 35.942  us/op
CacheBench.put  204800  0  avgt   25 0.108 ?  0.001  us/op
there's a nonlinear performance drop between 20480 and 204800 entries, probably 
attributable to CPU cache thrashing. Beyond 204800 entries the cache scales 
more linearly.

Benchmark results after the 2nd and 3rd patches are pretty similar, so I'll 
only copy one:
Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
CacheBench.put   20480  86400  avgt   25  0.146 ? 0.002  us/op
CacheBench.put   20480  0  avgt   25  0.108 ? 0.002  us/op
CacheBench.put  204800  86400  avgt   25  0.150 ? 0.001  us/op
CacheBench.put  204800  0  avgt   25  0.106 ? 0.001  us/op
The third patch improves worst-case times on a mostly idle cache by scattering 
removal of expired entries over multiple `put` calls. It does not affect 
performance of an overloaded cache.

The 4th patch removes all code that clears cached values before handing them 
over to the GC. [This 
comment](https://github.com/openjdk/jdk/commit/5859a0320334bfb6b46b62eb16b4c387641f4a2a#diff-c6bd583a97fbc4f471621fee7eab37c63718cdb6932ce357fa403cfda4b32b6fL346)
 stated that clearing values was supposed to be a GC performance optimization. 
It wasn't. Benchmark results after that commit:
Benchmark   (size)  (timeout)  Mode  Cnt  Score   Error  Units
CacheBench.put   20480  86400  avgt   25  0.113 ? 0.001  us/op
CacheBench.put   20480  0  avgt   25  0.075 ? 0.002  us/op
CacheBench.put  204800  86400  avgt   25  0.116 ? 0.001  us/op
CacheBench.put  204800  0  avgt   25  0.072 ? 0.001  us/op
I wasn't expecting that much of an improvement, and don't know how to explain 
it.

The 40ns difference between cache with and without a timeout can be attributed 
to 2 `System.currentTimeMillis()` calls; they were pretty slow on my VM.

-

Commit messages:
 - Do not invalidate objects before GC
 - Always expunge on put
 - Stop scanning expired entries after first non-expired one
 - Add cache microbenchmark

Changes: https://git.openjdk.java.net/jdk/pull/2255/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2255=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259886
  Stats: 138 lines in 3 files changed: 85 ins; 40 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2255.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2255/head:pull/2255

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

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]

[Alexey Semenyuk]

On Mon, 1 Feb 2021 12:16:09 GMT, Magnus Ihse Bursie  wrote:

>> Alexey Semenyuk has refreshed the contents of this pull request, and 
>> previous commits have been removed. The incremental views will show 
>> differences compared to the previous content of the PR. The pull request 
>> contains one new commit since the last revision:
>> 
>>   8254702: jpackage app launcher crashes on CentOS
>
> make/modules/jdk.jpackage/Lib.gmk line 61:
> 
>> 59: JPACKAGE_OUTPUT_DIR := 
>> $(JDK_OUTPUTDIR)/modules/$(MODULE)/jdk/jpackage/internal/resources
>> 60: JPACKAGE_CXXFLAGS_windows := -EHsc -DUNICODE -D_UNICODE
>> 61: JPACKAGE_CFLAGS_windows := -DUNICODE -D_UNICODE
> 
> Why is this change modifying Windows? I thought it was supposed to be a 
> linux-only fix..?

There is new shared JvmlLauncherLib.c file. This new make variable is to setup 
complier for this file on Windows. 
The functional change is Linux-only, however code base code reshuffled  on all 
platforms.

> make/modules/jdk.jpackage/Lib.gmk line 65:
> 
>> 63: ))
>> 64: 
>> 65: $(BUILD_JPACKAGE_APPLAUNCHEREXE): $(call FindLib, java.base, java)
> 
> Why did you remove this dependency?

I moved it to the bottom of the file making all artifacts produced by 
make/modules/jdk.jpackage/Lib.gmk depend on java.bas and java. There is 
`$(JPACKAGE_TARGETS): $(call FindLib, java.base, java)` at the bottom of the 
file.

> make/modules/jdk.jpackage/Lib.gmk line 106:
> 
>> 104:   CFLAGS_linux := -Wno-format-nonliteral, \
>> 105:   LDFLAGS := $(LDFLAGS_JDKLIB) \
>> 106:   
>> -Wl$(COMMA)--version-script=$(TOPDIR)/make/modules/$(MODULE)/applauncherlib.ld-version-script,
>>  \
> 
> We should really not be using linker scripts. I did not understand your 
> comment in the linker script -- was it only needed to handle your personal 
> build environment? If so, you need to fix your build environment instead.

Yeh, I found that linker script is needed only in my local build env. I'll 
remove it then.

-

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

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

[Philip Race]

Per my comment in the PR I am currently working on updating it to handle 
the test update needed.


Once the other in-flight PRs are integrated, my grepping says that the 
only remaining build change


needed after that is to remove one disabled warning in 
make/autoconf/flags-cflags.m4


I am going to try to find out if we can remove that now or have to wait 
until all JNF is removed.


FWIW it looks like you added it here : 
http://hg.openjdk.java.net/jdk/jdk/rev/ec62d6cab037


If we can roll this into an existing PR that might be easier. If it 
needs to wait for all of them, then we'll play it by ear as to whether 
to add it fo the last man standing or file a new bug.


-phil

On 2/1/21 3:33 AM, Magnus Ihse Bursie wrote:

On 2021-01-29 17:41, Phil Race wrote:
But we can just remove it and prove it - but probably a separate PR 
since it is nothing to do with the desktop module and the autoconf 
code needs to be updated once everything else is in.
Fair enough. Do you have a JBS issue tracking the remaining build 
system fixes for the JNF removal? I looked around but could not find one.


/Magnus

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

[Phil Race]

On Mon, 1 Feb 2021 11:35:22 GMT, Magnus Ihse Bursie  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8260616: Removing remaining JNF dependencies in the java.desktop modul
>
> Build changes look good for this PR.

> * There is also a test dependency that I have seen being addressed, in 
> `make/test/JtregNativeJdk.gmk` line 82, for `libTestMainKeyWindow`.

update on this. It Is used by precisely one jtreg AWT test. So I guess it is 
fair game to add resolving this to the desktop module update. I am currently 
testing it before committing - will take a couple of hours as the headful tests 
take a while to run.

-

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

AArch64 OpenJDK bootstrap failure on head

[Andrew Haley]

I've been unable to get OpenJDK to build on one box because of this message:

[0.013s][error][cds] Unable to map CDS archive -- 
os::vm_allocation_granularity() expected: 4096 actual: 65536

The problem is that this box is using 64kb pages, and the boostrap
Java was built on a machine with 4k pages. It shouldn't really
be a problem, but this happens:

build/linux-aarch64-server-slowdebug/support/gensrc/java.desktop/sun/awt/AWTIcon32_java_icon16_png.java:

package sun.awt;
public class AWTIcon32_java_icon16_png {
public static int[] java_icon16_png = {
[0.013s][error][cds] Unable to map CDS archive -- 
os::vm_allocation_granularity() expected: 4096 actual: 65536
16,16,
0x689fb5c9,
0xcc9fb5c8, 0xff9fb5c8, 0xff9fb5c8, 0xff9fb5c8, 0xff9fb5c8, 0xff9fb5c8, 
0xff9fb5c8, 0xff9fb5c8, 0xff9fb5c8, 0xff9fb5c8,
0xff9fb5c8, 0xff9fb5c8, 0xff9fb5c8, 0xcc9fb5c8, 0x689fb5c9, 0xcc9bb2c4, 
0xffd5dade, 0xfff5f5f5, 0xfff7f7f7, 0xfff7f7f7,

As you can see, the generated AWTIcon32_java_icon16_png.java has the error
message mixed in with the Java source code, so bootstrap fails.

I've tried a bunch of ways to get around this problem, but nothing
works.

I did this:

diff --git a/make/autoconf/boot-jdk.m4 b/make/autoconf/boot-jdk.m4
index 1d82f7c79b9..5c26be68fe4 100644
--- a/make/autoconf/boot-jdk.m4
+++ b/make/autoconf/boot-jdk.m4
@@ -413,7 +413,7 @@ AC_DEFUN_ONCE([BOOTJDK_SETUP_BOOT_JDK_ARGUMENTS],
 UTIL_ADD_JVM_ARG_IF_OK([$boot_jdk_cds_args 
-Xshare:auto],boot_jdk_jvmargs,[$JAVA])
   else
 # Otherwise optimistically use the system-wide one, if one is present
-UTIL_ADD_JVM_ARG_IF_OK([-Xshare:auto],boot_jdk_jvmargs,[$JAVA])
+UTIL_ADD_JVM_ARG_IF_OK([-Xshare:off],boot_jdk_jvmargs,[$JAVA])
   fi

   # Finally append user provided options to allow them to override.
@@ -427,6 +427,9 @@ AC_DEFUN_ONCE([BOOTJDK_SETUP_BOOT_JDK_ARGUMENTS],

   AC_MSG_CHECKING([flags for boot jdk java command for big workloads])

+  # Disable UseSharedSpaces in case the boot JVM was built on another system
+  UTIL_ADD_JVM_ARG_IF_OK([-XX:-UseSharedSpaces],boot_jdk_jvmargs,[$JAVA])
+
   # Starting amount of heap memory.
   UTIL_ADD_JVM_ARG_IF_OK([-Xms64M],boot_jdk_jvmargs_big,[$JAVA])
   BOOTCYCLE_JVM_ARGS_BIG=-Xms64M

I also tried  --with-boot-jdk-jvmargs='-Xshare:off -XX:-UseSharedSpaces'

but that doesn't work either. Any ideas? I'm really stuck.

-- 
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: [jdk16] RFR: 8258378: Final nroff manpage update for JDK 16

[Erik Joelsson]

On Mon, 1 Feb 2021 11:09:25 GMT, Magnus Ihse Bursie  wrote:

> Before RC phase we need to ensure we have the final set of manpage updates 
> published in OpenJDK.

Marked as reviewed by erikj (Reviewer).

-

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

Integrated: JDK-8260669: Missing quotes in fixpath.sh

[Erik Joelsson]

On Fri, 29 Jan 2021 19:54:12 GMT, Erik Joelsson  wrote:

> The build on Windows can fail if certain directory names are present in root 
> directory of the workspace. In particular I've observed that the single 
> letter 'p' is triggering this problem. This is caused by missing quotes 
> around [:upper:] in a 'tr' call in fixpath.sh.

This pull request has now been integrated.

Changeset: 80760a32
Author:Erik Joelsson 
URL:   https://git.openjdk.java.net/jdk/commit/80760a32
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8260669: Missing quotes in fixpath.sh

Reviewed-by: tbell, iris, mikael, ihse

-

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

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

[Magnus Ihse Bursie]

On Mon, 1 Feb 2021 12:35:05 GMT, Alan Hayward 
 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 
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. :(

-

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

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

[Alan Hayward]

On Mon, 1 Feb 2021 11:19:34 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)

>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?

-

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

Re: RFR: 8254702: jpackage app launcher crashes on CentOS [v2]

[Magnus Ihse Bursie]

On Fri, 29 Jan 2021 23:06:20 GMT, Alexey Semenyuk  wrote:

>> Fix for https://bugs.openjdk.java.net/browse/JDK-8254702
>> 
>> The fix splits Linux app launcher in app launcher and launcher shared lib. 
>> App launcher is pure C and doesn't have C++ code. App launcher lib 
>> incorporates bulk of C++ code from app launcher. 
>> At startup app launcher loads launcher shared lib and calls functions it 
>> provides to get data for launching JVM (path to jli lib and arguments for 
>> JLI_Launch function call).
>> App launcher unloads launcher shared lib before launching JVM to remove C++ 
>> runtime from the process memory.
>> 
>> Getting rid of C++ code from app launcher required to rewrite app 
>> installation location lookup code from C++ to C. LinuxPackage.c source is C 
>> alternative for 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Package.cpp
>>  and 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.jpackage/linux/native/applauncher/Executor.cpp.
>> 
>> Layout of jpackage's native code changed:
>> - `common`: code shared between all jpackage binaries.
>> - `applauncher`: launcher only code.
>> - `applauncherlib`: launcher lib code on Linux and launcher code on other 
>> platforms.
>> - `applaunchercommon`: code shared between launcher and launcher lib on 
>> Linux and launcher code on other platforms.
>
> Alexey Semenyuk has refreshed the contents of this pull request, and previous 
> commits have been removed. The incremental views will show differences 
> compared to the previous content of the PR.

This whole change seems very messy to me. :-(

I'm having a hard time even untangling the PR to understand what's going on. 
Are you creating two new directories, "applauncherlib" and 
"applauncherlibcommon"? First of all, for shared libraries, the norm is to have 
a "lib-" prefix, not a "-lib" suffix. Secondly, there is already a "common" 
directory, is that not enough?

Changes requested by ihse (Reviewer).

src/jdk.jpackage/share/native/common/app.cpp line 26:

> 24:  */
> 25: 
> 26: #include "kludge_c++11.h"

The name arose my curiosity, so I had to check out the file. Now that we indeed 
do have C++11 in the JDK (indeed, C++14), this should perhaps be revisited? 
(Not as part of this PR, of course)

make/modules/jdk.jpackage/Lib.gmk line 61:

> 59: JPACKAGE_OUTPUT_DIR := 
> $(JDK_OUTPUTDIR)/modules/$(MODULE)/jdk/jpackage/internal/resources
> 60: JPACKAGE_CXXFLAGS_windows := -EHsc -DUNICODE -D_UNICODE
> 61: JPACKAGE_CFLAGS_windows := -DUNICODE -D_UNICODE

Why is this change modifying Windows? I thought it was supposed to be a 
linux-only fix..?

make/modules/jdk.jpackage/Lib.gmk line 65:

> 63: ))
> 64: 
> 65: $(BUILD_JPACKAGE_APPLAUNCHEREXE): $(call FindLib, java.base, java)

Why did you remove this dependency?

make/modules/jdk.jpackage/Lib.gmk line 106:

> 104:   CFLAGS_linux := -Wno-format-nonliteral, \
> 105:   LDFLAGS := $(LDFLAGS_JDKLIB) \
> 106:   
> -Wl$(COMMA)--version-script=$(TOPDIR)/make/modules/$(MODULE)/applauncherlib.ld-version-script,
>  \

We should really not be using linker scripts. I did not understand your comment 
in the linker script -- was it only needed to handle your personal build 
environment? If so, you need to fix your build environment instead.

-

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

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

[Magnus Ihse Bursie]

On Sun, 31 Jan 2021 18:45:19 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 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 15 additional 
> commits since the last revision:
> 
>  - move test
>  - Merge branch 'master' into 8257858
>  - a test
>
>only in patch2:
>unchanged:
>  - end values should be vectors
>  - phil comment
>  - same behavior as before -- empty realm map
>  - error check, new JavaStringToNSString
>  - do not find class and method in loop
>  - no more header file
>
>reverted:
>  - better macro, no more JNI_COCOA_ENTER
>  - ... and 5 more: 
> https://git.openjdk.java.net/jdk/compare/fdd718db...ef337f12

Basically looks good, but the Obj-C test file needs proper handling.

make/test/JtregNativeJdk.gmk line 84:

> 82:   -framework Cocoa -framework JavaNativeFoundation
> 83:   BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeJniInvocationTest := -ljli
> 84:   BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libTestDynamicStore := -ObjC

Instead of "tricking" the build system of compiling an Obj-C file by 
masquerading it as a C file and passing compiler options, you should expose the 
test file for what it is, and add support in the build system to handle this. I 
can help you with that part.

-

Changes requested by ihse (Reviewer).

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

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

[Magnus Ihse Bursie]

On Fri, 29 Jan 2021 17:24:05 GMT, Phil Race  wrote:

>> 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 incrementally with one additional 
> commit since the last revision:
> 
>   8260616: Removing remaining JNF dependencies in the java.desktop modul

Build changes look good for this PR.

-

Marked as reviewed by ihse (Reviewer).

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

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

[Magnus Ihse Bursie]

On 2021-01-29 17:41, Phil Race wrote:

But we can just remove it and prove it - but probably a separate PR since it is 
nothing to do with the desktop module and the autoconf code needs to be updated 
once everything else is in.
Fair enough. Do you have a JBS issue tracking the remaining build system 
fixes for the JNF removal? I looked around but could not find one.


/Magnus

Re: [jdk16] RFR: 8258378: Final nroff manpage update for JDK 16

[Magnus Ihse Bursie]

On Mon, 1 Feb 2021 11:09:25 GMT, Magnus Ihse Bursie  wrote:

> Before RC phase we need to ensure we have the final set of manpage updates 
> published in OpenJDK.

These updates have been made automatically from the markdown source files 
(which unfortunately is still closed-only).

-

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

Re: RFR: JDK-8260669: Missing quotes in fixpath.sh

[Magnus Ihse Bursie]

On Fri, 29 Jan 2021 19:54:12 GMT, Erik Joelsson  wrote:

> The build on Windows can fail if certain directory names are present in root 
> directory of the workspace. In particular I've observed that the single 
> letter 'p' is triggering this problem. This is caused by missing quotes 
> around [:upper:] in a 'tr' call in fixpath.sh.

Looks good. I also verified that there are no other calls to tr in the build 
system with missing quotes.

-

Marked as reviewed by ihse (Reviewer).

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

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

[Vladimir Kempik]

On Mon, 1 Feb 2021 09:31:31 GMT, Alan Hayward 
 wrote:

> You need add macos arm64 to hsdis. Having it working is fairly essential for 
> debugging.
> 
> Inside src/utils/hsdis, After cloning binutils
> 
> ```
> make; make demo; ./build/macosx-arm64/hsdis-demo
> ```
> 
> Results in:
> 
> ```
> Hello, world!
> ...And now for something completely different:
> 
> Decoding from 0x1046e31a4 to 0x1046e3664...with decode_instructions_virtual
> hsdis: bad native mach=architecture not set in Makefile!; please port hsdis 
> to this platform
> hsdis output options:
> ```
> 
> I fixed it by changing the makefile to force the build flags:
> 
> ```
> ARCH=aarch64
> CFLAGS/aarch64+= -m64
> ```
> 
> Resulting in:
> 
> ```
> Hello, world!
> ...And now for something completely different:
> 
> Decoding from 0x10012719c to 0x10012765c...with decode_instructions_virtual
> Decoding for CPU 'aarch64'
> main:
>  0x10012719c  sub sp, sp, #0x60
>  0x1001271a0  stp x29, x30, [sp, #80]
> ...etc
> ```
> 
> Putting the library in the right place then made disassembly in java work for 
> me.

Hello, hsdis is a separate out-of-tree project and is not part of this jep.
support for looking for proper hsdis dylib library was added as part of this 
jep.

-

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

[jdk16] RFR: 8258378: Final nroff manpage update for JDK 16

[Magnus Ihse Bursie]

Before RC phase we need to ensure we have the final set of manpage updates 
published in OpenJDK.

-

Commit messages:
 - 8258378: Final nroff manpage update for JDK 16

Changes: https://git.openjdk.java.net/jdk16/pull/142/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=142=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8258378
  Stats: 238 lines in 27 files changed: 98 ins; 111 del; 29 mod
  Patch: https://git.openjdk.java.net/jdk16/pull/142.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/142/head:pull/142

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

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

[Alan Hayward]

On Wed, 27 Jan 2021 14:58:27 GMT, Vladimir Kempik  wrote:

>> Build changes per se now looks okay. However, I agree with Erik that unless 
>> this PR can wait for the JNF removal, at the very least the build docs needs 
>> to be updated to explain how to successfully build for this platform. (I can 
>> live with the configure command line hack, since it's temporary -- otherwise 
>> I'd have requested a new configure argument.) This can be done in this PR or 
>> a follow-up PR.
>
>> Build changes per se now looks okay. However, I agree with Erik that unless 
>> this PR can wait for the JNF removal, at the very least the build docs needs 
>> to be updated to explain how to successfully build for this platform. (I can 
>> live with the configure command line hack, since it's temporary -- otherwise 
>> I'd have requested a new configure argument.) This can be done in this PR or 
>> a follow-up PR.
> 
> I believe it's better be done under separate PR/bugfix, so it can be 
> completely reverted once JNF removed.

You need add macos arm64 to hsdis. Having it working is fairly essential for 
debugging.

Inside src/utils/hsdis, After cloning binutils
make; make demo; ./build/macosx-arm64/hsdis-demo
Results in:
Hello, world!
...And now for something completely different:

Decoding from 0x1046e31a4 to 0x1046e3664...with decode_instructions_virtual
hsdis: bad native mach=architecture not set in Makefile!; please port hsdis to 
this platform
hsdis output options:

I fixed it by changing the makefile to force the build flags:
ARCH=aarch64
CFLAGS/aarch64+= -m64
Resulting in:
Hello, world!
...And now for something completely different:

Decoding from 0x10012719c to 0x10012765c...with decode_instructions_virtual
Decoding for CPU 'aarch64'
main:
 0x10012719csub sp, sp, #0x60
 0x1001271a0stp x29, x30, [sp, #80]
...etc

Putting the library in the right place then made disassembly in java work for 
me.

-

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