date:20210126

On Wed, 27 Jan 2021 00:44:51 GMT, Doug Lea  wrote:

>> Martin Buchholz 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:
>> 
>>   JDK-8260461
>
> Marked as reviewed by dl (Reviewer).

jcheck bot caught trailing whitespace in commit

-

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

Re: RFR: 8260461: Modernize jsr166 tck tests [v3]

> 8260461: Modernize jsr166 tck tests

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

  JDK-8260461

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2245/files
  - new: https://git.openjdk.java.net/jdk/pull/2245/files/0cd05e2d..ed6b6f0f

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

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

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

Re: RFR: 8260461: Modernize jsr166 tck tests [v2]

> 8260461: Modernize jsr166 tck tests

Martin Buchholz has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains one commit:

  JDK-8260461

-

Changes: https://git.openjdk.java.net/jdk/pull/2245/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2245=01
  Stats: 7616 lines in 71 files changed: 318 ins; 187 del; 7111 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2245.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2245/head:pull/2245

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

RFR: 8259816: Typo in java.util.stream package description

2021-01-26 Thread Stuart Marks

Fix a typo, and change an example to use Stream.toList().

-

Commit messages:
 - 8259816: Typo in java.util.stream package description

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

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

Re: bug jpackage in JDK 15

2021-01-26 Thread Michael Hall



> 
> When I open the built dmg there is an application icon, a drag to indicating 
> arrow, but no application folder icon as the drag target.
> 
> Possibly related…
> 
> Running [osascript, 
> /var/folders/dh/91wmrk0n6lzfmr4tjhjmcfp4gn/T/jdk.incubator.jpackage15947685185521368596/config/BlackJack
>  Blastoff-dmg-setup.scpt]
> /var/folders/dh/91wmrk0n6lzfmr4tjhjmcfp4gn/T/jdk.incubator.jpackage15947685185521368596/config/BlackJack
>  Blastoff-dmg-setup.scpt:1112:1334: execution error: Finder got an error: 
> Can’t make class alias file. (-2710)
> java.io.IOException: Command [osascript, 
> /var/folders/dh/91wmrk0n6lzfmr4tjhjmcfp4gn/T/jdk.incubator.jpackage15947685185521368596/config/BlackJack
>  Blastoff-dmg-setup.scpt] exited with 1 code
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Executor.executeExpectSuccess(Executor.java:75)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.IOUtils.exec(IOUtils.java:167)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.IOUtils.exec(IOUtils.java:135)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.MacDmgBundler.buildDMG(MacDmgBundler.java:393)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.MacDmgBundler.bundle(MacDmgBundler.java:91)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.MacDmgBundler.execute(MacDmgBundler.java:535)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Arguments.generateBundle(Arguments.java:680)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.internal.Arguments.processArguments(Arguments.java:549)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.main.Main.execute(Main.java:98)
>   at 
> jdk.incubator.jpackage/jdk.incubator.jpackage.main.Main.main(Main.java:52)
> 
> Also it seems...
> --add-modules java.desktop,java.prefs,java.se \
> 
> Is no longer an option?
> —help doesn’t show it and I end up with…
> java-options=--module-path
> 
> In the .cfg

Fwiw I was curious if this was a known issue.
https://bugs.openjdk.java.net/browse/JDK-8250615 


It seems like this might still be an open issue?
I am guessing new Folder security on OS X Catalina. If so I would be interested 
in hearing of any resolution. I have had another problem with a java app on the 
Documents folder.

Re: RFR: 8260461: Modernize jsr166 tck tests

2021-01-26 Thread Doug Lea

On Tue, 26 Jan 2021 21:23:25 GMT, Martin Buchholz  wrote:

> 8260461: Modernize jsr166 tck tests

Marked as reviewed by dl (Reviewer).

-

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

Re: RFR: 8259925: [Vector API] Unreasonable IndexOutOfBoundsException message when length < vlen

2021-01-26 Thread Jie Fu

On Tue, 26 Jan 2021 18:20:00 GMT, Paul Sandoz  wrote:

> Hi Jie, Thanks for the detailed analysis. I suspect the difference outside of 
> loops is because of expression "length - (vlen - 1)”. We need to make 
> intrinsic Objects.checkFromIndexSize. Is that something you might be 
> interested in pursuing? 

Yes, I'd like to do that in the future.
Thanks.

-

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

Re: RFR: JDK-8183372 : Refactor java/lang/Class shell tests to java [v3]

2021-01-26 Thread Florent Guillaume

On Mon, 25 Jan 2021 20:51:06 GMT, Mahendra Chhipa 
 wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8183372
>
> Mahendra Chhipa has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Implemented the review comments.

test/jdk/java/lang/Class/forName/NonJavaNames.java line 67:

> 65: @BeforeClass
> 66: public void createInvalidNameClasses() throws IOException {
> 67: Path hyphenPath = Paths.get(TEST_SRC + "hyphen.class");

Building a `Path` with string concatenation is an anti-pattern.
Also `Path.of` is probably preferred instead of `Paths.get`.

-

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

Re: RFR: JDK-8183372 : Refactor java/lang/Class shell tests to java [v3]

2021-01-26 Thread Brent Christian

On Mon, 25 Jan 2021 20:51:06 GMT, Mahendra Chhipa 
 wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8183372
>
> Mahendra Chhipa has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Implemented the review comments.

I like keeping the changes within the existing .java files, thanks.  I have a 
few more suggestions.

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 76:

> 74: * @modules jdk.compiler
> 75: * @build jdk.test.lib.process.* EnclosingClassTest
> 76: * @run testng/othervm EnclosingClassTest

The test should still be run with -esa -ea

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 88:

> 86: @BeforeClass
> 87: public void createEnclosingClasses() throws Throwable {
> 88: Path enclosingPath = Paths.get(ENCLOSING_CLASS_SRC);

The 'enclosingPath' Path could be the constant.  Then ENCLOSING_CLASS_SRC is 
not needed.

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 126:

> 124: Path pkg1Dir = Paths.get(SRC_DIR + "/pkg1");
> 125: FileUtils.deleteFileTreeWithRetry(pkg1Dir);
> 126: }

I'm not convinced the `@AfterClass` method is necessary.  Pre-cleanup is 
already done on LL94-95.  And occasionally, test-generated artifacts are 
helpful in post-mortem analysis.

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 158:

> 156: private void match(final String actual, final String expected) {
> 157: System.out.println("actual:" + actual + "expected:" + expected);
> 158: assert ((actual == null && expected == null) || 
> actual.trim().equals(expected.trim()));

Out of curiousity, why is the trim() now needed ?

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 180:

> 178: info(c, encClass, annotation.desc());
> 179: check(c, encClass, "" + encClass, annotation.encl(), 
> c.getSimpleName(), annotation.simple(),
> 180: c.getCanonicalName(), annotation.hasCanonical() ? 
> annotation.canonical() : null);

This looks like an unneeded formatting change

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 187:

> 185: check(array, array.getEnclosingClass(), "", "", 
> array.getSimpleName(),
> 186: annotation.simple() + "[]", array.getCanonicalName(), 
> annotation.hasCanonical()
> 187: ? annotation.canonical() + "[]" : null);

This looks like an unneeded formatting change

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 197:

> 195: testClass((Class) f.get(tests), annotation, f);
> 196: } catch (AssertionError ex) {
> 197: System.err.println("Error in " + 
> tests.getClass().getName() + "." + f.getName());

This looks like an unneeded formatting change

-

Changes requested by bchristi (Reviewer).

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

Re: RFR: JDK-8183372 : Refactor java/lang/Class shell tests to java [v2]

2021-01-26 Thread Mandy Chung

On Tue, 26 Jan 2021 22:30:03 GMT, Mandy Chung  wrote:

>> To avoid the Checkstyle warnings, I added them.
>
> Is it from your IDE configurations?  You can turn off Checkstyle warnings.  
> This just adds noise.

I also assume some of the formatting changes are changed by your IDE 
suggestion.  Can you please revert the formatting changes (there are quite a 
few of them).   This will help the reviewers.

-

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

Re: RFR: JDK-8183372 : Refactor java/lang/Class shell tests to java [v2]

2021-01-26 Thread Mandy Chung

On Mon, 25 Jan 2021 19:33:08 GMT, Mahendra Chhipa 
 wrote:

>> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 103:
>> 
>>> 101: createAndWriteEnclosingClasses(enclosingPath, pkg2File, 
>>> "pkg1.pkg2");
>>> 102: 
>>> 103: String javacPath = JDKToolFinder.getJDKTool("javac");
>> 
>> You can use `jdk.test.lib.compiler.CompilerUtils` test library to compile 
>> the file in process.
>
> I tried to use the CopmilerUtils to compile the file, But this utility is not 
> able to  the dependencies of the class. Example it is not able to find the 
> common.TestMe class while try to compile the EnclosingClass.java.

You need to add `--source-path` where it can find the dependent source files.

>> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 176:
>> 
>>> 174: }
>>> 175: 
>>> 176: private void check(final Class c, final Class enc,
>> 
>> I see that you make all parameters in all methods final.   It just adds 
>> noise.   Can you leave it out?
>
> To avoid the Checkstyle warnings, I added them.

Is it from your IDE configurations?  You can turn off Checkstyle warnings.  
This just adds noise.

-

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

Re: RFR: JDK-8183372 : Refactor java/lang/Class shell tests to java [v3]

2021-01-26 Thread Mandy Chung

On Mon, 25 Jan 2021 20:51:06 GMT, Mahendra Chhipa 
 wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8183372
>
> Mahendra Chhipa has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Implemented the review comments.

test/jdk/java/lang/Class/forName/NonJavaNames.java line 37:

> 35:  * @bug 4952558
> 36:  * @library /test/lib
> 37:  * @build NonJavaNames

This `@build` can be dropped as this is done implicitly.

test/jdk/java/lang/Class/forName/NonJavaNames.java line 63:

> 61: private static final String TEST_SRC = SRC_DIR + "/classes/";
> 62: private static final String TEST_CLASSES = 
> System.getProperty("test.classes", ".");
> 63: Path dhyphenPath, dcommaPath, dperiodPath, dleftsquarePath, 
> drightsquarePath, dplusPath, dsemicolonPath, dzeroPath, dthreePath, dzadePath;

These variables should belong to the `createInvalidNameClasses` method.  You 
can simply add `Path` type declaration in line 78-87.

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 90:

> 88: Path enclosingPath = Paths.get(ENCLOSING_CLASS_SRC);
> 89: Path pkg1Dir = Paths.get(SRC_DIR + "/pkg1");
> 90: Path pkg2Dir = Paths.get(SRC_DIR+"/pkg1/pkg2");

nit: space before and after `+` is missing

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 89:

> 87: public void createEnclosingClasses() throws Throwable {
> 88: Path enclosingPath = Paths.get(ENCLOSING_CLASS_SRC);
> 89: Path pkg1Dir = Paths.get(SRC_DIR + "/pkg1");

Alternatively:

Path pkg1Dir = Paths.get(SRC_DIR, pkg1);
Path pkg2Dir = Paths.get(SRC_DIR, pkg1, pkg2);
Path pkg1File = pkg1Dir.resolve("EnclosingClass.java");
Path pkg2File = pkg2Dir.resolve("EnclosingClass.java");

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 84:

> 82: public class EnclosingClassTest {
> 83: private static final String SRC_DIR = System.getProperty("test.src");
> 84: private static final String ENCLOSING_CLASS_SRC = SRC_DIR + 
> "/EnclosingClass.java";

Suggestion:

private static final Path ENCLOSING_CLASS_SRC = Paths.get(SRC_DIR, 
"EnclosingClass.java");

Use Path::toString to convert to a String where needed.

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 103:

> 101: String javacPath = JDKToolFinder.getJDKTool("javac");
> 102: OutputAnalyzer outputAnalyzer = 
> ProcessTools.executeCommand(javacPath, "-d", 
> System.getProperty("test.classes", "."),
> 103: SRC_DIR + "/EnclosingClass.java", SRC_DIR + 
> "/pkg1/EnclosingClass.java", SRC_DIR + "/pkg1/pkg2/EnclosingClass.java");

`File.separator` is different on Unix and Windows.   Either replace `/` with 
`File.separator`.   Or you can use `java.nio.file.Path` and call 
`Path::toString` to get the string representation.

See above suggestion of declaring `static final Path ENCLOSING_CLASS_SRC`.  So 
the above should use `ENCLOSING_CLASS_SRC` and `pkg2File` instead (calling 
toString)

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 108:

> 106: 
> 107: @Test
> 108: public void testEnclosingClasses() throws Throwable {

better to throw specific exceptions for example `ReflectiveOperationException`. 
 Same for the `@Test` below.

test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 124:

> 122: @AfterClass
> 123: public void deleteEnclosingClasses() throws IOException {
> 124: Path pkg1Dir = Paths.get(SRC_DIR + "/pkg1");

Suggestion:

Path pkg1Dir = Paths.get(SRC_DIR, "pkg1");

-

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

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

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

RFR: 8260461: Modernize jsr166 tck tests

8260461: Modernize jsr166 tck tests

-

Commit messages:
 - JDK-8260461

Changes: https://git.openjdk.java.net/jdk/pull/2245/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2245=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260461
  Stats: 7535 lines in 70 files changed: 314 ins; 183 del; 7038 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2245.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2245/head:pull/2245

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

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

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 [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 [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: 8259925: [Vector API] Unreasonable IndexOutOfBoundsException message when length < vlen

2021-01-26 Thread Paul Sandoz

On Tue, 26 Jan 2021 13:24:57 GMT, Jie Fu  wrote:

>> The intrinsic enables C2 to more reliably elide bounds checks. I don't know 
>> the exact details but at a high-level it transforms signed values into 
>> unsigned values (and therefore the signed comparisons become unsigned 
>> comparisons), which helps C2 remove checks when there is a dominating check 
>> of, for example, an upper loop bound.
>> 
>> You say "the intrinsified Objects.checkIndex seems to generate more code 
>> than inlined if-statements". Can you present some examples of Java code and 
>> the corresponding C2 generated assembler where this happens?
>
>> The intrinsic enables C2 to more reliably elide bounds checks. I don't know 
>> the exact details but at a high-level it transforms signed values into 
>> unsigned values (and therefore the signed comparisons become unsigned 
>> comparisons), which helps C2 remove checks when there is a dominating check 
>> of, for example, an upper loop bound.
>> 
>> You say "the intrinsified Objects.checkIndex seems to generate more code 
>> than inlined if-statements". Can you present some examples of Java code and 
>> the corresponding C2 generated assembler where this happens?
> 
> Hi @PaulSandoz ,
> 
> I agree with you that let's keep the code as it is for the sake of 
> performance.
> 
> I spent some time looking into the assembly code generated by 
> Objects.checkIndex and inlined if-statements.
> Here are the test program [1], running script [2] and diff [3].
> 
>  - For testSimple [4] that I checked last week, inlined if-statements [5] is 
> better than Objects.checkIndex [6].
>  - However, for testLoop [7], Objects.checkIndex [8] is better than inlined 
> if-statements [9].
>(I'm sorry I didn't check loop methods last week.)
>AFAICS, the inlined if-statements will generate two more instructions [10] 
> to check wether idx >= 0 for each loop iteration.
> 
> It seems that the intrinsified Objects.checkIndex will be able to optimize 
> out the lower bound check for loops.
> So I also agree with you that an intrinsified method seems the right choice.
> 
> Thanks.
> Best regards,
> Jie
> 
> [1] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/Bar.java
> [2] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/1.sh
> [3] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/1.diff
> [4] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/Bar.java#L10
> [5] 
> https://github.com/DamonFool/experiment/blob/main/JDK-8259925/3-testSimple.log
> [6] 
> https://github.com/DamonFool/experiment/blob/main/JDK-8259925/2-testSimple.log
> [7] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/Bar.java#L15
> [8] 
> https://github.com/DamonFool/experiment/blob/main/JDK-8259925/2-testLoop.log
> [9] 
> https://github.com/DamonFool/experiment/blob/main/JDK-8259925/3-testLoop.log
> [10] 
> https://github.com/DamonFool/experiment/blob/main/JDK-8259925/3-testLoop.log#L135

Hi Jie,

Thanks for the detailed analysis.

I suspect the difference outside of loops is because of expression "length - 
(vlen - 1)”.

We need to make intrinsic Objects.checkFromIndexSize. Is that something you 
might be interested in pursuing?

Paul.

On Jan 26, 2021, at 5:25 AM, Jie Fu 
mailto:notificati...@github.com>> wrote:

The intrinsic enables C2 to more reliably elide bounds checks. I don't know the 
exact details but at a high-level it transforms signed values into unsigned 
values (and therefore the signed comparisons become unsigned comparisons), 
which helps C2 remove checks when there is a dominating check of, for example, 
an upper loop bound.

You say "the intrinsified Objects.checkIndex seems to generate more code than 
inlined if-statements". Can you present some examples of Java code and the 
corresponding C2 generated assembler where this happens?

Hi 
@PaulSandoz
 ,

I agree with you that let's keep the code as it is for the sake of performance.

I spent some time looking into the assembly code generated by 
Objects.checkIndex and inlined if-statements.
Here are the test program [1], running script [2] and diff [3].

  *   For testSimple [4] that I checked last week, inlined if-statements [5] is 
better than Objects.checkIndex [6].
  *   However, for testLoop [7], Objects.checkIndex [8] is better than inlined 
if-statements [9].
(I'm sorry I didn't check loop methods last week.)
AFAICS, the inlined if-statements will generate two more instructions [10] to 
check wether idx >= 0 for each loop iteration.

It seems that the intrinsified Objects.checkIndex will be able to optimize out 
the lower bound check for loops.
So I also agree with you that an intrinsified method seems the right choice.

Thanks.
Best regards,
Jie

[1]

Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v5]

2021-01-26 Thread Philippe Marschall

> Implement three optimiztations for Reader.read(CharBuffer)
> 
> * Add a code path for heap buffers in Reader#read to use the backing array 
> instead of allocating a new one.
> * Change the code path for direct buffers in Reader#read to limit the 
> intermediate allocation to `TRANSFER_BUFFER_SIZE`.
> * Implement `InputStreamReader#read(CharBuffer)` and delegate to 
> `StreamDecoder`.
> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation.

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

  Limit amount read to avoid BufferOverflowException
  
  - limit the amount read
  - add tests

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1915/files
  - new: https://git.openjdk.java.net/jdk/pull/1915/files/d247b637..a8531c1b

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

  Stats: 79 lines in 2 files changed: 62 ins; 2 del; 15 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1915.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1915/head:pull/1915

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

Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v4]

2021-01-26 Thread Philippe Marschall

On Tue, 19 Jan 2021 07:22:49 GMT, Philippe Marschall 
 wrote:

>> src/java.base/share/classes/java/io/Reader.java line 207:
>> 
>>> 205: target.put(cbuf, 0, n);
>>> 206: nread += n;
>>> 207: remaining -= n;
>> 
>> Wouldn't there be a possibility for target.put(cbuf, 0, n) to throw 
>> BufferOverflowException ?
>> For example:
>> - there's room (remaining) for TRANSFER_BUFFER_SIZE + 1 characters in target
>> - cbuff is sized to TRANSFER_BUFFER_SIZE
>> - 1st iteration of do loop transfers TRANSFER_BUFFER_SIZE charasters 
>> (remaining == 1)
>> - 2nd iteration reads > 1 (up to TRANSFER_BUFFER_SIZE) characters
>> - target.put throws BufferOverflowException
>> 
>> You have to limit the amount read in each iteration to be 
>> Math.min(remaining, cbuf.length)
>
> You're correct. I need to expand the unit tests.

Fixed

-

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

Re: RFR: 8257074 Update the ByteBuffers micro benchmark [v6]

2021-01-26 Thread Chris Hegarty

> The ByteBuffers micro benchmark seems to be a little dated. 
> 
> It should be a useful resource to leverage when analysing the performance 
> impact of any potential implementation changes in the byte buffer classes. 
> More specifically, the impact of such changes on the performance of sharp 
> memory access operations. 
> 
> This issue proposes to update the benchmark in the following ways to meet the 
> aforementioned use-case: 
> 
> 1. Remove allocation from the individual benchmarks - it just creates noise. 
> 2. Consolidate per-thread shared heap and direct buffers. 
> 3. All scenarios now use absolute memory access operations - so no state of 
> the shared buffers is considered. 
> 4. Provide more reasonable default fork, warmup, etc, out-of-the-box. 
> 5. There seems to have been an existing bug in the test where the size 
> parameter was not always considered - this is now fixed.

Chris Hegarty 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 ten additional 
commits since the last revision:

 - Update copyright year range and tailing whitespace
 - Merge branch 'master' into bb-bench
 - Template generation of carrier specific micro-benchmarks.
   
   * Benchmarks are now split out into carrier specific source files
   * All variants and views are covered
   * Test scenario naming is idiomatic
   
   Even with the split by carrier, regex expressions can be used to easily run 
subsets the benchmarks. E.g. test all memory operations related to Short with 
read-only buffers:
   
   $ java -jar benchmarks.jar 
"org.openjdk.bench.java.nio..*Buffers.testDirect.*Short.*"  -l
   Benchmarks:
   org.openjdk.bench.java.nio.ByteBuffers.testDirectLoopGetShortRO
   org.openjdk.bench.java.nio.ByteBuffers.testDirectLoopGetShortSwapRO
   org.openjdk.bench.java.nio.ShortBuffers.testDirectBulkGetShortViewRO
   org.openjdk.bench.java.nio.ShortBuffers.testDirectBulkGetShortViewSwapRO
   org.openjdk.bench.java.nio.ShortBuffers.testDirectLoopGetShortViewRO
   org.openjdk.bench.java.nio.ShortBuffers.testDirectLoopGetShortViewSwapRO
 - Merge branch 'master' into bb-bench
 - Add explicitly allocated heap carrier buffer tests
 - Replace Single with Loop
 - whitespace
 - Add additional carrier views and endianness variants.
   
   A large number of variants are now covered. The individual benchmarks 
conform to the following convention:
 
test(Direct|Heap)(Bulk|Single)(Get|Put)(Byte|Char|Short|Int|Long|Float|Double)(View)?(Swap)?
   
   This allows to easily run a subset of particular interest. For example:
 Direct only :- "org.openjdk.bench.java.nio.ByteBuffers.testDirect.*"
 Char only   :- "org.openjdk.bench.java.nio.ByteBuffers.test.*Char.*"
 Bulk only   :- "org.openjdk.bench.java.nio.ByteBuffers.test.*Bulk.*"
 Put with Int or Long carrier :-
test(Direct|Heap)(Single)(Put)(Int|Long)(View)?(Swap)?"
   
   Running all variants together is likely not all that useful, since there 
will be a lot of data.
   
   The param sizes are changed so as to better allow for wider carrier views.
   
   There are a lot of individual benchmark methods, but their implementation is 
trivial, and largely mechanical.
   
   Question: where do folk stand on having a `main` method in a benchmark - as 
a standalone-run sanity? I found this useful to assert the validity of the 
benchmark code. It can be commented out if it could somehow affect the 
benchmark runs.
   
   ( I omitted read-only views, since they less interesting, and we already 
have a lot of variants )
 - Initial changes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1430/files
  - new: https://git.openjdk.java.net/jdk/pull/1430/files/a8e81a84..9e8014cf

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

  Stats: 123643 lines in 3239 files changed: 61246 ins; 39707 del; 22690 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1430.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1430/head:pull/1430

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

Re: RFR: 8257074 Update the ByteBuffers micro benchmark [v4]

2021-01-26 Thread Chris Hegarty

On Thu, 10 Dec 2020 14:01:56 GMT, Jorn Vernee  wrote:

>> While the updated set of scenarios covered by this benchmark is
>> reasonably (and vastly improves coverage), I find myself reluctant to
>> add the last remaining buffer property - read-only views. It's time to
>> template the generation of this code!
>
> If the cases get to be too many, you might also want to consider splitting 
> the benchmark class into several classes that cover different cases (we did 
> this for the memory access benchmarks as well). That would also make it 
> easier to run a subset of cases in isolation.

All comments to date have been addressed. Unless there are any further 
comments, I would like to integrate this change.

-

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

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

Integrated: 8260391: Remove StringCoding::err

On Tue, 26 Jan 2021 11:47:54 GMT, Claes Redestad  wrote:

> This patch removes the unused StringCoding::err method, and the associated 
> native code.

This pull request has now been integrated.

Changeset: b07797c2
Author:Claes Redestad 
URL:   https://git.openjdk.java.net/jdk/commit/b07797c2
Stats: 82 lines in 2 files changed: 0 ins; 82 del; 0 mod

8260391: Remove StringCoding::err

Reviewed-by: shade, rriggs

-

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

Re: RFR: 8260391: Remove StringCoding::err

On Tue, 26 Jan 2021 13:53:53 GMT, Aleksey Shipilev  wrote:

>> This patch removes the unused StringCoding::err method, and the associated 
>> native code.
>
> Looks fine.

@shipilev, @RogerRiggs, thanks for reviewing!

-

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

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

Re: RFR: 8260391: Remove StringCoding::err

2021-01-26 Thread Roger Riggs

On Tue, 26 Jan 2021 11:47:54 GMT, Claes Redestad  wrote:

> This patch removes the unused StringCoding::err method, and the associated 
> native code.

Marked as reviewed by rriggs (Reviewer).

-

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

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: 8260391: Remove StringCoding::err

2021-01-26 Thread Aleksey Shipilev

On Tue, 26 Jan 2021 11:47:54 GMT, Claes Redestad  wrote:

> This patch removes the unused StringCoding::err method, and the associated 
> native code.

Looks fine.

-

Marked as reviewed by shade (Reviewer).

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

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

Integrated: 8242456: PreviewFeature.Feature enum removal of TEXT_BLOCKS

2021-01-26 Thread Jan Lahoda

On Mon, 25 Jan 2021 11:55:42 GMT, Jan Lahoda  wrote:

> The enum constant is unused, and can be removed. The class is not an API.

This pull request has now been integrated.

Changeset: 5e8e0ada
Author:Jan Lahoda 
URL:   https://git.openjdk.java.net/jdk/commit/5e8e0ada
Stats: 7 lines in 1 file changed: 0 ins; 7 del; 0 mod

8242456: PreviewFeature.Feature enum removal of TEXT_BLOCKS

Reviewed-by: jlaskey

-

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

Re: RFR: 8259925: [Vector API] Unreasonable IndexOutOfBoundsException message when length < vlen

2021-01-26 Thread Jie Fu

On Thu, 21 Jan 2021 16:54:36 GMT, Paul Sandoz  wrote:

> The intrinsic enables C2 to more reliably elide bounds checks. I don't know 
> the exact details but at a high-level it transforms signed values into 
> unsigned values (and therefore the signed comparisons become unsigned 
> comparisons), which helps C2 remove checks when there is a dominating check 
> of, for example, an upper loop bound.
> 
> You say "the intrinsified Objects.checkIndex seems to generate more code than 
> inlined if-statements". Can you present some examples of Java code and the 
> corresponding C2 generated assembler where this happens?

Hi @PaulSandoz ,

I agree with you that let's keep the code as it is for the sake of performance.

I spent some time looking into the assembly code generated by 
Objects.checkIndex and inlined if-statements.
Here are the test program [1], running script [2] and diff [3].

 - For testSimple [4] that I checked last week, inlined if-statements [5] is 
better than Objects.checkIndex [6].
 - However, for testLoop [7], Objects.checkIndex [8] is better than inlined 
if-statements [9].
   (I'm sorry I didn't check loop methods last week.)
   AFAICS, the inlined if-statements will generate two more instructions [10] 
to check wether idx >= 0 for each loop iteration.

It seems that the intrinsified Objects.checkIndex will be able to optimize out 
the lower bound check for loops.
So I also agree with you that an intrinsified method seems the right choice.

Thanks.
Best regards,
Jie

[1] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/Bar.java
[2] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/1.sh
[3] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/1.diff
[4] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/Bar.java#L10
[5] 
https://github.com/DamonFool/experiment/blob/main/JDK-8259925/3-testSimple.log
[6] 
https://github.com/DamonFool/experiment/blob/main/JDK-8259925/2-testSimple.log
[7] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/Bar.java#L15
[8] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/2-testLoop.log
[9] https://github.com/DamonFool/experiment/blob/main/JDK-8259925/3-testLoop.log
[10] 
https://github.com/DamonFool/experiment/blob/main/JDK-8259925/3-testLoop.log#L135

-

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

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

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: 8260391: Remove StringCoding::err

On Tue, 26 Jan 2021 11:47:54 GMT, Claes Redestad  wrote:

> This patch removes the unused StringCoding::err method, and the associated 
> native code.

Testing: tier1+2

-

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

RFR: 8260391: Remove StringCoding::err

This patch removes the unused StringCoding::err method, and the associated 
native code.

-

Commit messages:
 - Remove StringCoding::err

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

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

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: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]