Re: RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups [v2]

2021-03-16 Thread Naoto Sato 
On Tue, 16 Mar 2021 21:02:28 GMT, Claes Redestad  wrote:

>> This patch changes the otherLowercase / otherUppercase bits to be set if 
>> either the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or 
>> the Unicode Other_Lowercase / Other_Uppercase property is set. This 
>> simplifies the lookup in Character.isLowerCase/isUpperCase to a single table 
>> lookup, which appears to be healthy for performance.
>> 
>> I also took the opportunity to clean up the somewhat dated GenerateCharacter 
>> utility class.
>> 
>> Testing: tier1-3
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Roger review + additional cleanups

Marked as reviewed by naoto (Reviewer).

-

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

Re: RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups [v2]

2021-03-16 Thread Magnus Ihse Bursie 
On Tue, 16 Mar 2021 21:02:28 GMT, Claes Redestad  wrote:

>> This patch changes the otherLowercase / otherUppercase bits to be set if 
>> either the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or 
>> the Unicode Other_Lowercase / Other_Uppercase property is set. This 
>> simplifies the lookup in Character.isLowerCase/isUpperCase to a single table 
>> lookup, which appears to be healthy for performance.
>> 
>> I also took the opportunity to clean up the somewhat dated GenerateCharacter 
>> utility class.
>> 
>> Testing: tier1-3
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Roger review + additional cleanups

Marked as reviewed by ihse (Reviewer).

-

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

Re: RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups [v2]

2021-03-16 Thread Claes Redestad 
> This patch changes the otherLowercase / otherUppercase bits to be set if 
> either the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or the 
> Unicode Other_Lowercase / Other_Uppercase property is set. This simplifies 
> the lookup in Character.isLowerCase/isUpperCase to a single table lookup, 
> which appears to be healthy for performance.
> 
> I also took the opportunity to clean up the somewhat dated GenerateCharacter 
> utility class.
> 
> Testing: tier1-3

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

  Roger review + additional cleanups

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3028/files
  - new: https://git.openjdk.java.net/jdk/pull/3028/files/5dc70a95..5698b88c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3028&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3028&range=00-01

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

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

Re: RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups

2021-03-16 Thread Roger Riggs 
On Tue, 16 Mar 2021 12:51:02 GMT, Claes Redestad  wrote:

> This patch changes the otherLowercase / otherUppercase bits to be set if 
> either the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or the 
> Unicode Other_Lowercase / Other_Uppercase property is set. This simplifies 
> the lookup in Character.isLowerCase/isUpperCase to a single table lookup, 
> which appears to be healthy for performance.
> 
> I also took the opportunity to clean up the somewhat dated GenerateCharacter 
> utility class.
> 
> Testing: tier1-3

make/jdk/src/classes/build/tools/generatecharacter/GenerateCharacter.java line 
310:

> 308: {
> 309: long[] result;
> 310: if (bLatin1) {

perhaps shorten to:
final long[] result = new long[bLatin1 ? 256 : 1 << 16];

make/jdk/src/classes/build/tools/generatecharacter/GenerateCharacter.java line 
652:

> 650: // There is no block just like it already, so add it to
> 651: // the buffer and put its index into the new map.
> 652: if (m >= 0) System.arraycopy(map, i, buffer, ptr, m);

If m == 0, you could skip the arraycopy.

make/jdk/src/classes/build/tools/generatecharacter/GenerateCharacter.java line 
659:

> 657: // so create a new array and copy data from the temporary buffer.
> 658: long[] newdata = new long[ptr];
> 659: if (ptr >= 0) System.arraycopy(buffer, 0, newdata, 0, ptr);

ditto, if ptr == 0, skip the arraycopy

make/jdk/src/classes/build/tools/generatecharacter/GenerateCharacter.java line 
1588:

> 1586: StringBuffer desc = new StringBuffer("java GenerateCharacter");
> 1587: for (String arg : args) {
> 1588: desc.append(" " + arg);

Avoid string concat:  
   desc.append(' ').append(arg);

-

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

Re: RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups

2021-03-16 Thread Erik Joelsson 
On Tue, 16 Mar 2021 12:51:02 GMT, Claes Redestad  wrote:

> This patch changes the otherLowercase / otherUppercase bits to be set if 
> either the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or the 
> Unicode Other_Lowercase / Other_Uppercase property is set. This simplifies 
> the lookup in Character.isLowerCase/isUpperCase to a single table lookup, 
> which appears to be healthy for performance.
> 
> I also took the opportunity to clean up the somewhat dated GenerateCharacter 
> utility class.
> 
> Testing: tier1-3

Looks good from build point of view. I like the code cleanups.

-

Marked as reviewed by erikj (Reviewer).

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

RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups

2021-03-16 Thread Claes Redestad 
This patch changes the otherLowercase / otherUppercase bits to be set if either 
the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or the Unicode 
Other_Lowercase / Other_Uppercase property is set. This simplifies the lookup 
in Character.isLowerCase/isUpperCase to a single table lookup, which appears to 
be healthy for performance.

I also took the opportunity to clean up the somewhat dated GenerateCharacter 
utility class.

Testing: tier1-3

-

Commit messages:
 - Merge branch 'master' into character_case
 - Cleanups and modernizations
 - Fix lookup in 00, 01, 0E planes
 - Widen the range of codepoints tested by Characters micro
 - Improve Character.isLowerCase/isUpperCase lookups

Changes: https://git.openjdk.java.net/jdk/pull/3028/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3028&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263677
  Stats: 261 lines in 8 files changed: 13 ins; 129 del; 119 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3028.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3028/head:pull/3028

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

Re: RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups

2021-03-16 Thread Claes Redestad 
On Tue, 16 Mar 2021 12:51:02 GMT, Claes Redestad  wrote:

> This patch changes the otherLowercase / otherUppercase bits to be set if 
> either the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or the 
> Unicode Other_Lowercase / Other_Uppercase property is set. This simplifies 
> the lookup in Character.isLowerCase/isUpperCase to a single table lookup, 
> which appears to be healthy for performance.
> 
> I also took the opportunity to clean up the somewhat dated GenerateCharacter 
> utility class.
> 
> Testing: tier1-3

Baseline:
Benchmark   (codePoint)  Mode  Cnt   Score   Error  Units
Characters.isLowerCase9  avgt5  13.809 ± 0.032  ns/op
Characters.isLowerCase   65  avgt5  13.811 ± 0.052  ns/op
Characters.isLowerCase   97  avgt5  12.552 ± 0.057  ns/op
Characters.isLowerCase  128  avgt5  13.823 ± 0.076  ns/op
Characters.isLowerCase  170  avgt5  13.811 ± 0.066  ns/op
Characters.isLowerCase  223  avgt5  12.556 ± 0.058  ns/op
Characters.isLowerCase  410  avgt5  19.466 ± 0.104  ns/op
Characters.isLowerCase  430  avgt5  20.718 ± 0.100  ns/op
Characters.isUpperCase9  avgt5  12.556 ± 0.056  ns/op
Characters.isUpperCase   65  avgt5  12.559 ± 0.067  ns/op
Characters.isUpperCase   97  avgt5  12.555 ± 0.055  ns/op
Characters.isUpperCase  128  avgt5  12.559 ± 0.060  ns/op
Characters.isUpperCase  170  avgt5  12.556 ± 0.036  ns/op
Characters.isUpperCase  223  avgt5  12.554 ± 0.055  ns/op
Characters.isUpperCase  410  avgt5  20.722 ± 0.129  ns/op
Characters.isUpperCase  430  avgt5  19.459 ± 0.091  ns/op

Patch:
Benchmark   (codePoint)  Mode  Cnt   Score   Error  Units
Characters.isLowerCase9  avgt5  12.556 ± 0.035  ns/op
Characters.isLowerCase   65  avgt5  12.562 ± 0.073  ns/op
Characters.isLowerCase   97  avgt5  12.551 ± 0.062  ns/op
Characters.isLowerCase  128  avgt5  12.553 ± 0.039  ns/op
Characters.isLowerCase  170  avgt5  12.554 ± 0.051  ns/op
Characters.isLowerCase  223  avgt5  12.552 ± 0.035  ns/op
Characters.isLowerCase  410  avgt5  18.833 ± 0.068  ns/op
Characters.isLowerCase  430  avgt5  18.832 ± 0.074  ns/op
Characters.isUpperCase9  avgt5  12.555 ± 0.050  ns/op
Characters.isUpperCase   65  avgt5  12.557 ± 0.041  ns/op
Characters.isUpperCase   97  avgt5  12.554 ± 0.056  ns/op
Characters.isUpperCase  128  avgt5  12.554 ± 0.055  ns/op
Characters.isUpperCase  170  avgt5  12.555 ± 0.054  ns/op
Characters.isUpperCase  223  avgt5  12.553 ± 0.036  ns/op
Characters.isUpperCase  410  avgt5  18.831 ± 0.099  ns/op
Characters.isUpperCase  430  avgt5  18.826 ± 0.047  ns/op

-

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

Integrated: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

2021-03-16 Thread Phil Race 
On Sat, 13 Mar 2021 00:15:16 GMT, Phil Race  wrote:

> From a build perspective this partially reverts 
> https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps 
> the harfbuzz sources separate and still supports building and running against 
> a system harfbuzz which is only of interest or relevance on Linux.
> 
> I ended up having to go this way because its is the least unsatisfactory 
> solution.
> I did not want us to build a devkit to link against a system linux only to 
> find we couldn't use it at runtime
> because too many systems have to old a version of harfbuzz.
> 
> This solves the Manjaro Linux problem and I've manually verified building 
> against a system hardbuxz on Ubuntu 20.10
> 
> There are couple of incidental fixes in here too
> - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building 
> against a system version
> - harfbuzz/hb-ucdn is gone and should not be listed as a header directory 
> needed to build the bundled copy
> - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502

This pull request has now been integrated.

Changeset: 05fe06a6
Author:Phil Race 
URL:   https://git.openjdk.java.net/jdk/commit/05fe06a6
Stats: 96 lines in 2 files changed: 7 ins; 51 del; 38 mod

8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

Reviewed-by: serb, ihse, azvegint

-

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

Re: RFR: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux [v3]

2021-03-16 Thread Alexander Zvegintsev 
On Tue, 16 Mar 2021 16:56:22 GMT, Phil Race  wrote:

>> From a build perspective this partially reverts 
>> https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps 
>> the harfbuzz sources separate and still supports building and running 
>> against a system harfbuzz which is only of interest or relevance on Linux.
>> 
>> I ended up having to go this way because its is the least unsatisfactory 
>> solution.
>> I did not want us to build a devkit to link against a system linux only to 
>> find we couldn't use it at runtime
>> because too many systems have to old a version of harfbuzz.
>> 
>> This solves the Manjaro Linux problem and I've manually verified building 
>> against a system hardbuxz on Ubuntu 20.10
>> 
>> There are couple of incidental fixes in here too
>> - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building 
>> against a system version
>> - harfbuzz/hb-ucdn is gone and should not be listed as a header directory 
>> needed to build the bundled copy
>> - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

Marked as reviewed by azvegint (Reviewer).

-

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

Re: RFR: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux [v3]

2021-03-16 Thread Magnus Ihse Bursie 
On Tue, 16 Mar 2021 16:56:22 GMT, Phil Race  wrote:

>> From a build perspective this partially reverts 
>> https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps 
>> the harfbuzz sources separate and still supports building and running 
>> against a system harfbuzz which is only of interest or relevance on Linux.
>> 
>> I ended up having to go this way because its is the least unsatisfactory 
>> solution.
>> I did not want us to build a devkit to link against a system linux only to 
>> find we couldn't use it at runtime
>> because too many systems have to old a version of harfbuzz.
>> 
>> This solves the Manjaro Linux problem and I've manually verified building 
>> against a system hardbuxz on Ubuntu 20.10
>> 
>> There are couple of incidental fixes in here too
>> - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building 
>> against a system version
>> - harfbuzz/hb-ucdn is gone and should not be listed as a header directory 
>> needed to build the bundled copy
>> - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

Marked as reviewed by ihse (Reviewer).

-

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

Re: RFR: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux [v2]

2021-03-16 Thread Phil Race 
On Tue, 16 Mar 2021 10:38:19 GMT, Alexander Zvegintsev  
wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux
>
> make/modules/java.desktop/lib/Awt2dLibraries.gmk line 456:
> 
>> 454:# when building libharfbuzz
>> 455:ifeq ($(call isTargetOs, aix), true)
>> 456:  LIBHARFBUZZ_CFLAGS += -qdebug=necan
> 
> Should it be `HARFBUZZ_CFLAGS` instead of `LIBHARFBUZZ_CFLAGS`? I don't see 
> any usage of `LIBHARFBUZZ_CFLAGS` in this makefile.

D'oh. It was LIB* in the old code and I copy / pasted and of course can't test 
that.
I've uploaded a fix.

-

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

Re: RFR: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux [v3]

2021-03-16 Thread Phil Race 
> From a build perspective this partially reverts 
> https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps 
> the harfbuzz sources separate and still supports building and running against 
> a system harfbuzz which is only of interest or relevance on Linux.
> 
> I ended up having to go this way because its is the least unsatisfactory 
> solution.
> I did not want us to build a devkit to link against a system linux only to 
> find we couldn't use it at runtime
> because too many systems have to old a version of harfbuzz.
> 
> This solves the Manjaro Linux problem and I've manually verified building 
> against a system hardbuxz on Ubuntu 20.10
> 
> There are couple of incidental fixes in here too
> - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building 
> against a system version
> - harfbuzz/hb-ucdn is gone and should not be listed as a header directory 
> needed to build the bundled copy
> - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502

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

  8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2982/files
  - new: https://git.openjdk.java.net/jdk/pull/2982/files/f668b327..a92a146c

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

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

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

Re: RFR: 8263667: Avoid running GitHub actions on branches named pr/*

2021-03-16 Thread Magnus Ihse Bursie 
On Tue, 16 Mar 2021 10:53:06 GMT, Robin Westberg  wrote:

> When the Skara feature "dependent pull requests" is activated for the JDK 
> repository, branches with the name "pr/" will start to appear. These 
> will not be synced into personal forks by the Skara sync command, but if they 
> are synced manually, we should avoid running GitHub actions workflows on them.

Marked as reviewed by ihse (Reviewer).

-

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

Re: RFR: 8263667: Avoid running GitHub actions on branches named pr/*

2021-03-16 Thread Erik Joelsson 
On Tue, 16 Mar 2021 10:53:06 GMT, Robin Westberg  wrote:

> When the Skara feature "dependent pull requests" is activated for the JDK 
> repository, branches with the name "pr/" will start to appear. These 
> will not be synced into personal forks by the Skara sync command, but if they 
> are synced manually, we should avoid running GitHub actions workflows on them.

Marked as reviewed by erikj (Reviewer).

-

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

Re: RFR: 8263667: Avoid running GitHub actions on branches named pr/*

2021-03-16 Thread Erik Helin 
On Tue, 16 Mar 2021 10:53:06 GMT, Robin Westberg  wrote:

> When the Skara feature "dependent pull requests" is activated for the JDK 
> repository, branches with the name "pr/" will start to appear. These 
> will not be synced into personal forks by the Skara sync command, but if they 
> are synced manually, we should avoid running GitHub actions workflows on them.

Looks good!

-

Marked as reviewed by ehelin (Reviewer).

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

RFR: 8263667: Avoid running GitHub actions on branches named pr/*

2021-03-16 Thread Robin Westberg 
When the Skara feature "dependent pull requests" is activated for the JDK 
repository, branches with the name "pr/" will start to appear. These 
will not be synced into personal forks by the Skara sync command, but if they 
are synced manually, we should avoid running GitHub actions workflows on them.

-

Commit messages:
 - Filter out branches named pr/ for GitHub actions

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

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

Re: RFR: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux [v2]

2021-03-16 Thread Alexander Zvegintsev 
On Mon, 15 Mar 2021 18:57:28 GMT, Phil Race  wrote:

>> From a build perspective this partially reverts 
>> https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps 
>> the harfbuzz sources separate and still supports building and running 
>> against a system harfbuzz which is only of interest or relevance on Linux.
>> 
>> I ended up having to go this way because its is the least unsatisfactory 
>> solution.
>> I did not want us to build a devkit to link against a system linux only to 
>> find we couldn't use it at runtime
>> because too many systems have to old a version of harfbuzz.
>> 
>> This solves the Manjaro Linux problem and I've manually verified building 
>> against a system hardbuxz on Ubuntu 20.10
>> 
>> There are couple of incidental fixes in here too
>> - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building 
>> against a system version
>> - harfbuzz/hb-ucdn is gone and should not be listed as a header directory 
>> needed to build the bundled copy
>> - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

make/modules/java.desktop/lib/Awt2dLibraries.gmk line 456:

> 454:# when building libharfbuzz
> 455:ifeq ($(call isTargetOs, aix), true)
> 456:  LIBHARFBUZZ_CFLAGS += -qdebug=necan

Should it be `HARFBUZZ_CFLAGS` instead of `LIBHARFBUZZ_CFLAGS`? I don't see any 
usage of `LIBHARFBUZZ_CFLAGS` in this makefile.

-

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

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

2021-03-16 Thread Andrew Haley 
On 3/15/21 6:56 PM, Anton Kozlov wrote:
> On Wed, 10 Mar 2021 11:21:44 GMT, Andrew Haley  wrote:
> 
>>> We always check for `R18_RESERVED` with `#if(n)def`, is there any reason to 
>>> define the value for the macro?
>>
>> Robustness, clarity, maintainability, convention. Why not?
> 
> I've tried to implement the suggestion, but it pulled more unnecessary 
> changes. It makes the intended way to check the condition less clear 
> (`#ifdef` and not `#if`).

No, no, no! I am not suggesting you change anything else, just that
you do not define contentless macros. You might as well define it
to be something, and true is a reasonable default, that's all. It's
not terribly important, it's just good practice.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671