date:20210126

On Tue, 26 Jan 2021 09:23:18 GMT, Magnus Ihse Bursie  wrote:

>> Anton Kozlov has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Refactor CDS disabling
>>  - Redo builsys support for aarch64-darwin
>
> make/autoconf/build-aux/autoconf-config.guess line 1275:
> 
>> 1273:  UNAME_PROCESSOR="aarch64"
>> 1274:  fi
>> 1275:fi ;;
> 
> Almost, but not quite, correct. We cannot change the autoconf-config.guess 
> file due to license restrictions (the license allows redistribution, not 
> modifications). Instead we have the config.guess file which "wraps" 
> autoconf-config.guess and makes pre-/post-call modifications to work around 
> limitations in the autoconf original file. So you need to check there if you 
> are getting incorrect results back and adjust it in that case. See the 
> already existing clauses in that file.

Hello
I have updated PR and moved this logic to make/autoconf/build-aux/config.guess
It's pretty similar to i386 -> x86_64 fix-up on macos_intel

-

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

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

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

  Redo buildsys fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2200/files
  - new: https://git.openjdk.java.net/jdk/pull/2200/files/b2b396fc..f1ef6240

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

  Stats: 18 lines in 2 files changed: 8 ins; 10 del; 0 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 [v5]

On Tue, 26 Jan 2021 19:33:28 GMT, Weijun Wang  wrote:

>> Anton Kozlov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Revert harfbuzz changes, disable warnings for it
>
> src/java.security.jgss/share/native/libj2gss/gssapi.h line 48:
> 
>> 46: // Condition was copied from
>> 47: // 
>> Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h
>> 48: #if TARGET_OS_MAC && (defined(__ppc__) || defined(__ppc64__) || 
>> defined(__i386__) || defined(__x86_64__))
> 
> So for macOS/AArch64, this `if` is no longer true?

That is according to Xcode sdk.

-

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

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

2021-01-26 Thread Bernhard Urban-Forster

On Mon, 25 Jan 2021 17:43:35 GMT, Phil Race  wrote:

>> Anton Kozlov has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Address feedback for signature generators
>>  - Enable -Wformat-nonliteral back
>
> src/java.desktop/share/native/libharfbuzz/hb-coretext.cc line 193:
> 
>> 191:* crbug.com/549610 */
>> 192:   // 0x0007 stands for "kCTVersionNumber10_10", see CoreText.h
>> 193: #if TARGET_OS_OSX && MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_10
> 
> I need a robust explanation of these changes.
> They are not mentioned, nor are they in upstream harfbuzz.
> Likely these should be pushed to upstream first if there is a reason for them.

I'm afraid it's one of those dirty changes that we did to make progress, but 
forgot to revisit later. Without the guard you would get:
* For target support_native_java.desktop_libharfbuzz_hb-coretext.o:

  if ( != nullptr && CTGetCoreTextVersion() < 0x0007) {
   ^

uint32_t CTGetCoreTextVersion( void ) CT_DEPRECATED("Use -[NSProcessInfo 
operatingSystemVersion]", macos(10.5, 11.0), ios(3.2, 14.0), watchos(2.0, 7.0), 
tvos(9.0, 14.0));
 ^

  if ( != nullptr && CTGetCoreTextVersion() < 0x0007) {
  ^

uint32_t CTGetCoreTextVersion( void ) CT_DEPRECATED("Use -[NSProcessInfo 
operatingSystemVersion]", macos(10.5, 11.0), ios(3.2, 14.0), watchos(2.0, 7.0), 
tvos(9.0, 14.0));
 ^
2 errors generated.
For now, it's probably better to go with what Anton did in the latest commit 
https://github.com/openjdk/jdk/pull/2200/commits/b2b396fca679fbc7ee78fb5bc80191bc79e1c490
Eventually upstream will do something about it I assume? How often does it get 
synced with upstream? Maybe worth to file an issue.

-

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

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

2021-01-26 Thread Weijun Wang

On Tue, 26 Jan 2021 18:42:02 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:
> 
>   Revert harfbuzz changes, disable warnings for it

src/java.security.jgss/share/native/libj2gss/gssapi.h line 48:

> 46: // Condition was copied from
> 47: // 
> Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h
> 48: #if TARGET_OS_MAC && (defined(__ppc__) || defined(__ppc64__) || 
> defined(__i386__) || defined(__x86_64__))

So for macOS/AArch64, this `if` is no longer true?

-

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

RFR: 8260460: GitHub actions still fail on Linux x86_32 with "Could not configure libc6:i386"

See for example:
 https://github.com/shipilev/jdk/runs/1771453139?check_suite_focus=true

E: Could not configure 'libc6:i386'.
E: Could not perform immediate configuration on 'libgcc-s1:i386'. Please see 
man 5 apt.conf under APT::Immediate-Configure for details. (2)

It seems JDK-8259924 helped only for a while.

There is a 
[bugfix](https://salsa.debian.org/apt-team/apt/-/commit/998a17d7e6f834c341f198ca5b6df2f27e18df38)
 in Apt 2.0.4 that prints these warnings and then allows the whole thing to 
continue. It was 
[released](https://bugs.launchpad.net/ubuntu-cdimage/+bug/1871268) in Ubuntu a 
week ago, but it is not in base image yet. So, if we upgrade apt before 
installing other packages, apt updates from 2.0.2 to 2.0.4 and the whole thing 
starts to work.

This also reverts changes added by previous attempt in 
[JDK-8259924](https://github.com/openjdk/jdk/commit/a9519c83).

-

Commit messages:
 - 8260460: GitHub actions still fail on Linux x86_32 with "Could not configure 
libc6:i386"

Changes: https://git.openjdk.java.net/jdk/pull/2243/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2243=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260460
  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2243.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2243/head:pull/2243

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

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

On Tue, 26 Jan 2021 16:35:54 GMT, Bernhard Urban-Forster  
wrote:

>> @lewurm This and other harfbuzz changes came from MS, could you please 
>> comment here ?
>
> This looks like a merge fix mistake: 
> https://github.com/openjdk/jdk/commit/051357ef977ecab77fa9b2b1e61f94f288e716f9#diff-e3815f37244d076e27feef0c778fb78a4e5691b47db9c92abcb28bb2a9c2865e

I've reverted this change and disabled some warnings for libharfbuzz instead. I 
should be blamed, yes. Now this is consistent with other changes covered by 
JDK-8260402

-

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

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

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

  Revert harfbuzz changes, disable warnings for it

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2200/files
  - new: https://git.openjdk.java.net/jdk/pull/2200/files/fef36580..b2b396fc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=03-04

  Stats: 5 lines in 3 files changed: 1 ins; 2 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 [v3]

2021-01-26 Thread erik . joelsson

On 2021-01-26 04:44, Magnus Ihse Bursie wrote:

On 2021-01-26 13:09, Vladimir Kempik wrote:
On Tue, 26 Jan 2021 12:02:02 GMT, Alan Hayward
wrote:

AIUI, the configure line needs passing a prebuilt
JavaNativeFoundation framework

ie:
`--with-extra-ldflags='-F
/Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'`

Otherwise there will be missing _JNFNative* functions.

Is this the long term plan? Or will eventually the required code be
moved into JDK and/or the xcode one automatically get picked up by
the configure scripts?

There is ongoing work by P. Race to eliminate dependence on JNF at all
How far has that work come? Otherwise the logic should be added to
configure to look for this framework automatically, and provide a way
to override it/set it if not found.

I don't think it's OK to publish a new port that cannot build
out-of-the-box without hacks like this.

My understanding is that Apple chose to not provide JNF for aarch64, so
if you want to build OpenJDK, you first need to build JNF yourself (it's
available in github). Phil is working on removing this dependency
completely, which will solve this issue [1].

In the meantime, I don't think we should rely on finding JNF in
unsupported locations inside Xcode. Could we wait with integrating this
port until it can be built without such hacks? If not, then adding
something in the documentation on how to get a working JNF would at
least be needed.

/Erik

[1] https://bugs.openjdk.java.net/browse/JDK-8257852

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

2021-01-26 Thread Bernhard Urban-Forster

On Tue, 26 Jan 2021 16:07:19 GMT, Vladimir Kempik  wrote:

>> src/java.desktop/share/native/libharfbuzz/hb-common.h line 113:
>> 
>>> 111: 
>>> 112: #define HB_TAG(c1,c2,c3,c4) 
>>> ((hb_tag_t)uint32_t)(c1)&0xFF)<<24)|(((uint32_t)(c2)&0xFF)<<16)|(((uint32_t)(c3)&0xFF)<<8)|((uint32_t)(c4)&0xFF)))
>>> 113: #define HB_UNTAG(tag)   (char)(((tag)>>24)&0xFF), 
>>> (char)(((tag)>>16)&0xFF), (char)(((tag)>>8)&0xFF), (char)((tag)&0xFF)
>> 
>> I need a robust explanation of these changes.
>> They are not mentioned, nor are they in upstream harfbuzz.
>> Likely these should be pushed to upstream first if there is a reason for 
>> them.
>
> @lewurm This and other harfbuzz changes came from MS, could you please 
> comment here ?

This looks like a merge fix mistake: 
https://github.com/openjdk/jdk/commit/051357ef977ecab77fa9b2b1e61f94f288e716f9#diff-e3815f37244d076e27feef0c778fb78a4e5691b47db9c92abcb28bb2a9c2865e

-

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

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

On Mon, 25 Jan 2021 17:43:22 GMT, Phil Race  wrote:

>> Anton Kozlov has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Address feedback for signature generators
>>  - Enable -Wformat-nonliteral back
>
> src/java.desktop/share/native/libharfbuzz/hb-common.h line 113:
> 
>> 111: 
>> 112: #define HB_TAG(c1,c2,c3,c4) 
>> ((hb_tag_t)uint32_t)(c1)&0xFF)<<24)|(((uint32_t)(c2)&0xFF)<<16)|(((uint32_t)(c3)&0xFF)<<8)|((uint32_t)(c4)&0xFF)))
>> 113: #define HB_UNTAG(tag)   (char)(((tag)>>24)&0xFF), 
>> (char)(((tag)>>16)&0xFF), (char)(((tag)>>8)&0xFF), (char)((tag)&0xFF)
> 
> I need a robust explanation of these changes.
> They are not mentioned, nor are they in upstream harfbuzz.
> Likely these should be pushed to upstream first if there is a reason for them.

@lewurm This and other harfbuzz changes came from MS, could you please comment 
here ?

-

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

building OpenJDK 11 with link time optimizations?

2021-01-26 Thread Matthias Klose

Several Linux distros now build with link time optimizations (-flto=auto) by
default.  It looks like 15 and newer versions can be built with -flto. I haven't
yet checked test results for LTO/non-LTO builds. I also only tried building with
GCC 10.

11 still fails with
/usr/bin/ld: /tmp/libjvm.so.xGmnnW.ltrans45.ltrans.o: in function
`G1CMOopClosure::do_oop(oopDesc**)':
/<>/openjdk-lts-11.0.9.1+1/make/hotspot/./src/hotspot/share/gc/g1/g1OopClosures.hpp:176:
undefined reference to `void G1CMOopClosure::do_oop_work(oopDesc**)'
[...]

However this might be just the first build failure.

Matthias

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

> 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 three additional 
commits since the last revision:

 - Little adjustement of SlowSignatureHandler
 - Partially bring previous commit
 - Revert "Address feedback for signature generators"
   
   This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2200/files
  - new: https://git.openjdk.java.net/jdk/pull/2200/files/0c2cb0a3..fef36580

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

  Stats: 98 lines in 1 file changed: 74 ins; 13 del; 11 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 [v4]

On Mon, 25 Jan 2021 10:00:20 GMT, Andrew Haley  wrote:

>> I like the suggestion. For the defense, new functions were made this way 
>> intentionally, to match existing `pass_int`, `pass_long`,.. I take your 
>> comment as a blessing to fix all of them. But I feel that refactoring of 
>> existing code should go in a separate commit. Especially after `pass_int` 
>> used `ldr/str` instead of `ldrw/strw`, which looks wrong. 
>> https://github.com/openjdk/jdk/pull/2200/files#diff-1ff58ce70aeea7e9842d34e8d8fd9c94dd91182999d455618b2a171efd8f742cL87-R122
>
> This is new code, and it should not repeat the mistakes of the past. There's 
> no need to refactor the similar existing code as part of this patch.

Makes sense, I've reverted changes in the old code but will propose them in the 
separate PR shortly.

-

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

Integrated: 8260406: Do not copy pure java source code to gensrc

On Tue, 26 Jan 2021 10:35:03 GMT, Magnus Ihse Bursie  wrote:

> For java.base gensrc we do the following:
> 
> # Copy two Java files that need no preprocessing.
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/%.java: 
> $(CHARACTERDATA_TEMPLATES)/%.java.template
> $(call LogInfo, Generating $(@F))
> $(call install-file)
> 
> GENSRC_CHARACTERDATA += 
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataUndefined.java \
> 
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataPrivateUse.java
> 
> What this means is that we have two "template" files that are just compiled 
> as java files, but only after being copied to gensrc. :-)
> 
> We should just rename these files to java and move them to the normal source 
> directory.

This pull request has now been integrated.

Changeset: 8d2f77fd
Author:Magnus Ihse Bursie 
URL:   https://git.openjdk.java.net/jdk/commit/8d2f77fd
Stats: 9 lines in 3 files changed: 0 ins; 8 del; 1 mod

8260406: Do not copy pure java source code to gensrc

Reviewed-by: alanb, erikj

-

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

Re: RFR: 8260406: Do not copy pure java source code to gensrc

2021-01-26 Thread Erik Joelsson

On Tue, 26 Jan 2021 10:35:03 GMT, Magnus Ihse Bursie  wrote:

> For java.base gensrc we do the following:
> 
> # Copy two Java files that need no preprocessing.
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/%.java: 
> $(CHARACTERDATA_TEMPLATES)/%.java.template
> $(call LogInfo, Generating $(@F))
> $(call install-file)
> 
> GENSRC_CHARACTERDATA += 
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataUndefined.java \
> 
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataPrivateUse.java
> 
> What this means is that we have two "template" files that are just compiled 
> as java files, but only after being copied to gensrc. :-)
> 
> We should just rename these files to java and move them to the normal source 
> directory.

Marked as reviewed by erikj (Reviewer).

-

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

Re: RFR: 8260408: Shenandoah: adjust inline hints after JDK-8255019

On Tue, 26 Jan 2021 12:42:10 GMT, Magnus Ihse Bursie  wrote:

>> I was following up on performance testing results for some ongoing work, and 
>> realized that there are `ShenandoahMarkContext::mark_strong` calls from 
>> `mark_work_loop`. Those callees were supposed to be inlined. I believe it is 
>> a simple omission after JDK-8255019 moved `mark_work_loop` code to 
>> `shenandoahMark.cpp`.
>> 
>> On a typical workload:
>> 
>> # Before
>> [44.500s][info][gc,stats] Concurrent Marking  =0.395 s (a =11278 us) 
>>(n =35) (lvls, us = 6426, 9473,11133,12891,16476)
>> 
>> # After
>> [44.405s][info][gc,stats] Concurrent Marking  =0.337 s (a = 9636 us) 
>>(n =35) (lvls, us = 3770, 7383, 9785,11328,16776)
>> 
>> Additional testing:
>>  - [x] Eyeballing GC hot paths
>>  - [x] Ad-hoc performance tests
>
> Looks good from the build PoV.

Thanks, GHA reports all builds are green. I would not wait for tests.

-

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

Integrated: 8260408: Shenandoah: adjust inline hints after JDK-8255019

On Tue, 26 Jan 2021 11:41:52 GMT, Aleksey Shipilev  wrote:

> I was following up on performance testing results for some ongoing work, and 
> realized that there are `ShenandoahMarkContext::mark_strong` calls from 
> `mark_work_loop`. Those callees were supposed to be inlined. I believe it is 
> a simple omission after JDK-8255019 moved `mark_work_loop` code to 
> `shenandoahMark.cpp`.
> 
> On a typical workload:
> 
> # Before
> [44.500s][info][gc,stats] Concurrent Marking  =0.395 s (a =11278 us) 
>(n =35) (lvls, us = 6426, 9473,11133,12891,16476)
> 
> # After
> [44.405s][info][gc,stats] Concurrent Marking  =0.337 s (a = 9636 us) 
>(n =35) (lvls, us = 3770, 7383, 9785,11328,16776)
> 
> Additional testing:
>  - [x] Eyeballing GC hot paths
>  - [x] Ad-hoc performance tests

This pull request has now been integrated.

Changeset: edd27074
Author:Aleksey Shipilev 
URL:   https://git.openjdk.java.net/jdk/commit/edd27074
Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod

8260408: Shenandoah: adjust inline hints after JDK-8255019

Reviewed-by: rkennke, ihse

-

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

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

On Mon, 25 Jan 2021 19:12:17 GMT, Coleen Phillimore  wrote:

>> I've tried to do something like this initially. The idea was to use Write in 
>> VM state and Exec in Java and Native states. However, for example, JIT runs 
>> in the Native state and needs Write access. So instead, W^X is managed on 
>> entry and exit from VM code, in macros like JRT_ENTRY. Unfortunately, not 
>> every JVM entry function is defined with an appropriate macro (at least for 
>> now), so I had to add explicit W^X state management along with the explicit 
>> java thread state, like here.
>
> Yes, that's why I thought it should be added to the classes 
> ThreadInVMfromNative, etc like:
> class ThreadInVMfromNative : public ThreadStateTransition {
>   ResetNoHandleMark __rnhm;
> 
> We can look at it with cleaning up the thread transitions RFE or as a 
> follow-on.  If every line of ThreadInVMfromNative has to have one of these   
> Thread::WXWriteVerifier __wx_write; people are going to miss them when 
> adding the former.

Not every ThreadInVMfromNative needs this, for example JIT goes to Native state 
without changing of W^X state. But from some experience of maintaining this 
patch, I actually had a duty to add missing W^X transitions after assert 
failures. A possible solution is actually to make W^X transition a part of 
ThreadInVMfromNative (and similar), but controlled by an optional constructor 
parameter with possible values "do exec->write", "verify write"...;. So in a 
common case ThreadInVMfromNative would implicitly do the transition and still 
would allow something uncommon like verification of the state for the JIT. I 
have to think about this.

-

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

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


On 2021-01-26 13:09, Vladimir Kempik wrote:

On Tue, 26 Jan 2021 12:02:02 GMT, Alan Hayward 
 wrote:


AIUI, the configure line needs passing a prebuilt JavaNativeFoundation framework
ie:
`--with-extra-ldflags='-F 
/Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'`

Otherwise there will be missing _JNFNative* functions.

Is this the long term plan? Or will eventually the required code be moved into 
JDK and/or the xcode one automatically get picked up by the configure scripts?

There is ongoing work by P. Race to eliminate dependence on JNF at all
How far has that work come? Otherwise the logic should be added to 
configure to look for this framework automatically, and provide a way to 
override it/set it if not found.


I don't think it's OK to publish a new port that cannot build 
out-of-the-box without hacks like this.


/Magnus


-

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

Re: RFR: 8260408: Shenandoah: adjust inline hints after JDK-8255019

On Tue, 26 Jan 2021 11:41:52 GMT, Aleksey Shipilev  wrote:

> I was following up on performance testing results for some ongoing work, and 
> realized that there are `ShenandoahMarkContext::mark_strong` calls from 
> `mark_work_loop`. Those callees were supposed to be inlined. I believe it is 
> a simple omission after JDK-8255019 moved `mark_work_loop` code to 
> `shenandoahMark.cpp`.
> 
> On a typical workload:
> 
> # Before
> [44.500s][info][gc,stats] Concurrent Marking  =0.395 s (a =11278 us) 
>(n =35) (lvls, us = 6426, 9473,11133,12891,16476)
> 
> # After
> [44.405s][info][gc,stats] Concurrent Marking  =0.337 s (a = 9636 us) 
>(n =35) (lvls, us = 3770, 7383, 9785,11328,16776)
> 
> Additional testing:
>  - [x] Eyeballing GC hot paths
>  - [x] Ad-hoc performance tests

Looks good from the build PoV.

-

Marked as reviewed by ihse (Reviewer).

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

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

2021-01-26 Thread Alan Hayward

On Tue, 26 Jan 2021 12:06:28 GMT, Vladimir Kempik  wrote:

> > AIUI, the configure line needs passing a prebuilt JavaNativeFoundation 
> > framework
> > ie:
> > `--with-extra-ldflags='-F 
> > /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'`
> > Otherwise there will be missing _JNFNative* functions.
> > Is this the long term plan? Or will eventually the required code be moved 
> > into JDK and/or the xcode one automatically get picked up by the configure 
> > scripts?
> 
> There is ongoing work by P. Race to eliminate dependence on JNF at all

Ok, that's great.
In the meantime is it worth adding something to the MacOS section of 
doc/building.* ?
(I pieced together the required line from multiple posts in a mailing list)

-

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

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

On Tue, 26 Jan 2021 12:02:02 GMT, Alan Hayward 
 wrote:

> AIUI, the configure line needs passing a prebuilt JavaNativeFoundation 
> framework
> ie:
> `--with-extra-ldflags='-F 
> /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'`
> 
> Otherwise there will be missing _JNFNative* functions.
> 
> Is this the long term plan? Or will eventually the required code be moved 
> into JDK and/or the xcode one automatically get picked up by the configure 
> scripts?

There is ongoing work by P. Race to eliminate dependence on JNF at all

-

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

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

2021-01-26 Thread Alan Hayward

On Tue, 26 Jan 2021 09:23:23 GMT, Magnus Ihse Bursie  wrote:

>> Anton Kozlov has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Refactor CDS disabling
>>  - Redo builsys support for aarch64-darwin
>
> Changes requested by ihse (Reviewer).

AIUI, the configure line needs passing a prebuilt JavaNativeFoundation framework
ie:
`
--with-extra-ldflags='-F 
/Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'
`

Otherwise there will be missing _JNFNative* functions.

Is this the long term plan? Or will eventually the required code be moved into 
JDK and/or the xcode one automatically get picked up by the configure scripts?

-

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

Re: RFR: 8260408: Shenandoah: adjust inline hints after JDK-8255019

2021-01-26 Thread Roman Kennke

On Tue, 26 Jan 2021 11:41:52 GMT, Aleksey Shipilev  wrote:

> I was following up on performance testing results for some ongoing work, and 
> realized that there are `ShenandoahMarkContext::mark_strong` calls from 
> `mark_work_loop`. Those callees were supposed to be inlined. I believe it is 
> a simple omission after JDK-8255019 moved `mark_work_loop` code to 
> `shenandoahMark.cpp`.
> 
> On a typical workload:
> 
> # Before
> [44.500s][info][gc,stats] Concurrent Marking  =0.395 s (a =11278 us) 
>(n =35) (lvls, us = 6426, 9473,11133,12891,16476)
> 
> # After
> [44.405s][info][gc,stats] Concurrent Marking  =0.337 s (a = 9636 us) 
>(n =35) (lvls, us = 3770, 7383, 9785,11328,16776)
> 
> Additional testing:
>  - [x] Eyeballing GC hot paths
>  - [x] Ad-hoc performance tests

Looks good! Thanks!

-

Marked as reviewed by rkennke (Reviewer).

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

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

2021-01-26 Thread Coleen Phillimore

On Tue, 26 Jan 2021 11:31:18 GMT, Anton Kozlov  wrote:

>> This could be a follow-up RFE.
>
> I assume a WXVerifier class that tracks W^X mode in debug mode and does 
> nothing in release mode. I've considered to do this, it's relates to small 
> inefficiencies, while this patch brings zero overhead (in release) for a 
> platform that does not need W^X. 
> * We don't need thread instance in release to call 
> `os::current_thread_enable_wx`. Having WXVerifier a part of the Thread will 
> require calling `Thread::current()` first and we could only hope for compiler 
> to optimize this out, not sure if it will happen at all. In some contexts the 
> Thread instance is available, in some it's not. 
> * An instance of the empty class (as WXVerifier will be in the release) will 
> occupy non-zero space in the Thread instance.
> 
> If such costs are negligible, I can do as suggested.

I really just want the minimal number of lines of code and hooks in thread.hpp. 
 You can still access it through the thread, just like these lines currently.  
Look at HandshakeState as an example.

-

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

RFR: 8260408: Shenandoah: adjust inline hints after JDK-8255019

I was following up on performance testing results for some ongoing work, and 
realized that there are `ShenandoahMarkContext::mark_strong` calls from 
`mark_work_loop`. Those callees were supposed to be inlined. I believe it is a 
simple omission after JDK-8255019 moved `mark_work_loop` code to 
`shenandoahMark.cpp`.

On a typical workload:

# Before
[44.500s][info][gc,stats] Concurrent Marking  =0.395 s (a =11278 us) 
   (n =35) (lvls, us = 6426, 9473,11133,12891,16476)

# After
[44.405s][info][gc,stats] Concurrent Marking  =0.337 s (a = 9636 us) 
   (n =35) (lvls, us = 3770, 7383, 9785,11328,16776)

Additional testing:
 - [x] Eyeballing GC hot paths
 - [x] Ad-hoc performance tests

-

Commit messages:
 - 8260408: Shenandoah: adjust inline hints after JDK-8255019

Changes: https://git.openjdk.java.net/jdk/pull/2235/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2235=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260408
  Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2235.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2235/head:pull/2235

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

Re: RFR: 8260406: Do not copy pure java source code to gensrc

2021-01-26 Thread Alan Bateman

On Tue, 26 Jan 2021 10:35:03 GMT, Magnus Ihse Bursie  wrote:

> For java.base gensrc we do the following:
> 
> # Copy two Java files that need no preprocessing.
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/%.java: 
> $(CHARACTERDATA_TEMPLATES)/%.java.template
> $(call LogInfo, Generating $(@F))
> $(call install-file)
> 
> GENSRC_CHARACTERDATA += 
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataUndefined.java \
> 
> $(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataPrivateUse.java
> 
> What this means is that we have two "template" files that are just compiled 
> as java files, but only after being copied to gensrc. :-)
> 
> We should just rename these files to java and move them to the normal source 
> directory.

Good. I notice the comment in both source files says "Java.lang.Character" 
rather than "java.lang.Character", probably should fix that at some point.

-

Marked as reviewed by alanb (Reviewer).

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

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

On Mon, 25 Jan 2021 14:40:42 GMT, Coleen Phillimore  wrote:

>> src/hotspot/share/runtime/thread.hpp line 915:
>> 
>>> 913:   verify_wx_state(WXExec);
>>> 914: }
>>> 915:   };
>> 
>> Rather than add all this to thread.hpp, can you make a wxVerifier.hpp and 
>> just add the class instance as a field in thread.hpp?
>
> This could be a follow-up RFE.

I assume a WXVerifier class that tracks W^X mode in debug mode and does nothing 
in release mode. I've considered to do this, it's relates to small 
inefficiencies, while this patch brings zero overhead (in release) for a 
platform that does not need W^X. 
* We don't need thread instance in release to call 
`os::current_thread_enable_wx`. Having WXVerifier a part of the Thread will 
require calling `Thread::current()` first and we could only hope for compiler 
to optimize this out, not sure if it will happen at all. In some contexts the 
Thread instance is available, in some it's not. 
* An instance of the empty class (as WXVerifier will be in the release) will 
occupy non-zero space in the Thread instance.

If such costs are negligible, I can do as suggested.

-

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

RFR: 8260406: Do not copy pure java source code to gensrc

For java.base gensrc we do the following:

# Copy two Java files that need no preprocessing.
$(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/%.java: 
$(CHARACTERDATA_TEMPLATES)/%.java.template
$(call LogInfo, Generating $(@F))
$(call install-file)

GENSRC_CHARACTERDATA += 
$(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataUndefined.java \
$(SUPPORT_OUTPUTDIR)/gensrc/java.base/java/lang/CharacterDataPrivateUse.java

What this means is that we have two "template" files that are just compiled as 
java files, but only after being copied to gensrc. :-)

We should just rename these files to java and move them to the normal source 
directory.

-

Commit messages:
 - 8260406: Do not copy pure java source code to gensrc

Changes: https://git.openjdk.java.net/jdk/pull/2233/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2233=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260406
  Stats: 9 lines in 3 files changed: 0 ins; 8 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2233.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2233/head:pull/2233

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

Re: RFR: 8257733: Move module-specific data from make to respective module [v4]

On Sat, 23 Jan 2021 07:55:09 GMT, Alan Bateman  wrote:

> We should create a separate issue to rename them and get rid of the copying 
> in the make file.

I opened https://bugs.openjdk.java.net/browse/JDK-8260406.

-

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

Re: RFR: 8259679: GitHub actions should use MSVC 14.28


On 2021-01-20 06:48, David Holmes wrote:

On 19/01/2021 11:48 pm, Magnus Ihse Bursie wrote:


On 2021-01-19 00:13, David Holmes wrote:
Hard-wiring build numbers seems a maintenance PITA. :( Is there not 
a way to do this that is independent of the actual build installed?
I agree that hard-wiring numbers in the code is bad. However, having 
explicit version numbers for our dependencies is good. That is the 
philosophy behind jib, and it has served us well. We should look at 
finding a way to store the actual versioon numbers outside the 
submit.yml file, though.


I think this is a bit different to the way we use jib to manage our 
own build environment.



But what do you mean by "independent of the actual build installed"?


We have AFAIK no control over what toolkit versions are installed for 
Github actions, so they could change at any time. We update today to 
request 14.28 and tomorrow they replace 14.28 with 14.29 and our 
builds fail again even though they would likely be perfectly fine on 
14.29. To me if we just were able to say "Use latest VS 14.x" then 
that would suffice most of the time. Occasionally we could hit an 
issue where our sources are not yet compatible with the latest VS but 
hopefully that is less frequent than the frequency these VS builds 
change.


I think that depends on how we view the GHA testing, which still is not 
clear. Or rather, each OpenJDK developer seem to have their own opinion 
on this, and no official guidelines to help us.


I think we need to make the most effort we can to control the GHA 
environment. That means requesting a specific version of VS, and having 
a fail-fast if that is not available. That way, we get to know when GHA 
environment changes.


Which is how things work today.



Would it not also be good to add a check for the existence of the 
build that we do hard-wire? That way the problem will be very clear 
next time.
It would be fantastic! Please tell me how to to that in the limited, 
brain-dead Github Actions environment. :-(


If we put those hard-wired version/build numbers into a file readable 
by configure, then couldn't configure check it?


So just to be clear: configure *did* check for the existence of VS, and 
it did not find it, and thus it aborted and complained. So we did have 
this fail-fast functionality. But since the GHA environment contained a 
half-broken, half-working installation of the VS version in question, 
the error message from configure was not really clear.


This PR was about updating the version of VS required by the GHA build 
script. Unfortunately, I think it's not possible to either:


1) move the version number to a separate configuration file, instead of 
hard-coding it in the yaml source


2) have the submit.yml GHA script check for the availability of the 
specific version of VS before launching configure.


but it might be just my limited knowledge about the system. If it were 
possible, I think we should do both.


/Magnus



Cheers,
David


/Magnus


Cheers,
David


-

Commit messages:
  - 8259679: GitHub actions should use MSVC 14.28

Changes: https://git.openjdk.java.net/jdk/pull/2126/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2126=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8259679
   Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
   Patch: https://git.openjdk.java.net/jdk/pull/2126.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk 
pull/2126/head:pull/2126


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

Re: RFR: 8258477: Pre-submit testing using GitHub Actions should merge changes from target branch

On Thu, 21 Jan 2021 09:03:57 GMT, Robin Westberg  wrote:

>> Normally when running GitHub Actions on a pull request, what is checked out 
>> is the merge of the pull request with the latest changes on the target 
>> branch. This ensure that what is tested is as close as possible to what will 
>> actually be the result of integrating said pull request. 
>> 
>> In our use of GitHub Actions, we don't run directly on pull requests but 
>> instead allow contributors to run them in their own personal forks. In that 
>> context, there is no notion of a target branch. However, we can infer the 
>> target project and branch by encoding this in the workflow file. This allows 
>> us to perform the same merge manually, and achieve the same behaviour. 
>> 
>> The change also adds an option to disable this automated merge when 
>> launching the workflow manually, which could be useful when investigating 
>> unexpected test failures.
>> 
>> Note that downstream projects picking up this change will have to adapt the 
>> workflow file to keep using these actions for pre-submit testing. This has 
>> been a common request from downstream projects, but could also be done in a 
>> separate change (or not at all).
>
> So, is there anyone who would like to review this? I still think it will 
> improve the effectiveness of the GitHub Actions-based testing.

I might be missing something here, but it sounds like it muddies the water on 
exactly what is tested: if an automatic merge is possible, this will be done 
and tested. If the automatic merge fails, the original, un-merged, branch is 
tested. For instance, it will have the ironic effect of a small non-compatible 
change that passes the automatic merge will fail the test, but an even larger 
non-compatible change that do not pass the automatic merge will succeed!

I understand the intention, but I'm just worried it decreases the transparency 
of what GHA tests even more. 

How can you know what was tested?

-

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

Re: RFR: 8246112: Remove build-time and run-time checks for clock_gettime and CLOCK_MONOTONIC [v6]

2021-01-26 Thread mkraljt

On Mon, 25 Jan 2021 04:46:52 GMT, David Holmes  wrote:

>> We are now confident that we have build-time and runtime support for 
>> clock_gettime and CLOCK_MONOTONIC on all our OpenJDK supported POSIX 
>> platforms - see bug report for some more details on different OS. 
>> Consequently we can simplify a lot of the code in this area and move common 
>> code to os_posix.
>> 
>> As of glibc 2.17 the necessary functions are in glibc rather than librt, but 
>> we (Oracle at least) aren't yet in position to set our minimum Linux version 
>> to support that. We still have supported platforms at glibc 2.12. So to 
>> address that we link librt at build time on Linux. This seems to work find 
>> for older and more modern Linuxes and also works for the Apline Linux with 
>> Musl variant.
>> 
>> The changes are in layered commits:
>> 
>> Step 1: Remove build time checks. SUPPORTS_CLOCK_MONOTONIC is assumed true 
>> and removed.
>> Step 2: make supports_monotonic_clock always true and so remove checking in 
>> OS code
>> Step 3: Replace gettimeofday by clock_gettime(CLOCK_REALTIME)
>> Step 4: Move shared time functions to os_posix.cpp
>> Step 5: Alway link librt on Linux so we don't rely on glibc > 2.17
>> 
>> Testing: tiers 1-3 for functional testing
>>  built and checked (-Xlog:os) on Linux with glibc 2.12 and 2.17, 
>>  macOS 10.13.6 and 10.15.7
>
> David Holmes has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 12 commits:
> 
>  - 'Merge branch 'master' into 8246112-mono
>  - Update copyrights
>  - Remove the always true os::supports_monotonic_clock()
>  - Merge
>  - Merge branch 'master' into 8246112-mono
>  - Restrict librt linking to JVM - per Magnus's request
>  - Merge branch 'master' into 8246112-mono
>  - Alway link librt on Linux so we don't rely on glibc > 2.12
>  - Step 4: Move shared time functions to os_posix.cpp
>  - Step 3: Replace gettimeofday by clock_gettime(CLOCK_REALTIME)
>  - ... and 2 more: 
> https://git.openjdk.java.net/jdk/compare/764111ff...f4bf2b8a

src/hotspot/os/posix/os_posix.cpp line 1292:

> 1290:   return jlong(ts.tv_sec) * MILLIUNITS +
> 1291: jlong(ts.tv_nsec) / NANOUNITS_PER_MILLIUNIT;
> 1292: }

David,
Great to see simplifications like this happening in OpenJdk code.
- Posix letting you delete platform specific code.
- Always nice to see 'if' branching eliminated to simplify test coverage and 
runtime.

-

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

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