date:20210305

> `java.net.URLClassLoader.getResource` can throw an undocumented 
> `IllegalArgumentException`.
> 
> According to the javadoc for the `getResource` and `findResource` methods, 
> neither should be throwing `IllegalArgumentException` - they should return 
> null if the resource can't be resolved.
> 
> Quoting the javadoc for 
> [`URLClassLoader.html#findResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URLClassLoader.html#findResource(java.lang.String))
>> Returns:
>> a URL for the resource, or null if the resource could not be found, or 
>> if the loader is closed.
> 
> And quoting the javadoc for 
> [`ClassLoader.html#getResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String))
>> Returns:
>> URL object for reading the resource; null if the resource could not be 
>> found, a URL could not be constructed to locate the resource, the resource 
>> is in a package that is not opened unconditionally, or access to the 
>> resource is denied by the security manager.
> 
> Neither mentions throwing `IllegalArgumentException` and both are clear that 
> when URL can't be constructed, `null` should be returned.
> 
> Here's a stack trace:
> java.lang.IllegalArgumentException: name
> at 
> java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:600)
> at 
> java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:291)
> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:655)
> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:653)
> at java.base/java.security.AccessController.doPrivileged(Native 
> Method)
> at 
> java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:652)
> 
> Looking at 
> [`URLClassPath.findResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L603)
> URL findResource(final String name, boolean check) {
> URL url;
> try {
> url = new URL(base, ParseUtil.encodePath(name, false));
> } catch (MalformedURLException e) {
> throw new IllegalArgumentException("name");
> }
> 
> Instead of throwing `IllegalArgumentException`, that line should simply 
> return `null`.
> 
> A similar issue exists at 
> [`URLClassPath.getResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L639)
> URL findResource(final String name, boolean check) {
> URL url;
> try {
> url = new URL(base, ParseUtil.encodePath(name, false));
> } catch (MalformedURLException e) {
> throw new IllegalArgumentException("name");
> }
> 
> Instead of throwing `IllegalArgumentException`, that line should simply 
> return `null`.

Craig Andrews 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-8262277: java.net.URLClassLoader.getResource throws undocumented 
IllegalArgumentException

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2662/files
  - new: https://git.openjdk.java.net/jdk/pull/2662/files/79869e78..944956c9

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

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

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

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v4]

On Fri, 5 Mar 2021 23:35:46 GMT, Brent Christian  wrote:

>> Craig Andrews 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-8262277: java.net.URLClassLoader.getResource throws undocumented 
>> IllegalArgumentException
>
> test/jdk/java/net/URLClassLoader/FindResourceDoesNotThrowException.java line 
> 27:
> 
>> 25:  * @test
>> 26:  * @bug 8262277
>> 27:  * @summary Test to see if URLClassLoader.getResource and 
>> URLClassLoader.getResources
> 
> The summary mentions the get* methods, but the test calls the find* methods.  
> I think it would be good to test all four methods (getResource, getResources, 
> findResource, findResources), and update the summary e.g. "Test if 
> URLClassLoader throws IllegalArgumentException when getting or finding 
> resources."

Good point - thank you.

I've made the change and updated this PR accordingly.

-

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

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v4]

2021-03-05 Thread Brent Christian

On Fri, 5 Mar 2021 16:02:49 GMT, Craig Andrews 
 wrote:

>> `java.net.URLClassLoader.getResource` can throw an undocumented 
>> `IllegalArgumentException`.
>> 
>> According to the javadoc for the `getResource` and `findResource` methods, 
>> neither should be throwing `IllegalArgumentException` - they should return 
>> null if the resource can't be resolved.
>> 
>> Quoting the javadoc for 
>> [`URLClassLoader.html#findResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URLClassLoader.html#findResource(java.lang.String))
>>> Returns:
>>> a URL for the resource, or null if the resource could not be found, or 
>>> if the loader is closed.
>> 
>> And quoting the javadoc for 
>> [`ClassLoader.html#getResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String))
>>> Returns:
>>> URL object for reading the resource; null if the resource could not be 
>>> found, a URL could not be constructed to locate the resource, the resource 
>>> is in a package that is not opened unconditionally, or access to the 
>>> resource is denied by the security manager.
>> 
>> Neither mentions throwing `IllegalArgumentException` and both are clear that 
>> when URL can't be constructed, `null` should be returned.
>> 
>> Here's a stack trace:
>> java.lang.IllegalArgumentException: name
>> at 
>> java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:600)
>> at 
>> java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:291)
>> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:655)
>> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:653)
>> at java.base/java.security.AccessController.doPrivileged(Native 
>> Method)
>> at 
>> java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:652)
>> 
>> Looking at 
>> [`URLClassPath.findResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L603)
>> URL findResource(final String name, boolean check) {
>> URL url;
>> try {
>> url = new URL(base, ParseUtil.encodePath(name, false));
>> } catch (MalformedURLException e) {
>> throw new IllegalArgumentException("name");
>> }
>> 
>> Instead of throwing `IllegalArgumentException`, that line should simply 
>> return `null`.
>> 
>> A similar issue exists at 
>> [`URLClassPath.getResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L639)
>> URL findResource(final String name, boolean check) {
>> URL url;
>> try {
>> url = new URL(base, ParseUtil.encodePath(name, false));
>> } catch (MalformedURLException e) {
>> throw new IllegalArgumentException("name");
>> }
>> 
>> Instead of throwing `IllegalArgumentException`, that line should simply 
>> return `null`.
>
> Craig Andrews 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-8262277: java.net.URLClassLoader.getResource throws undocumented 
> IllegalArgumentException

Changes requested by bchristi (Reviewer).

test/jdk/java/net/URLClassLoader/FindResourceDoesNotThrowException.java line 27:

> 25:  * @test
> 26:  * @bug 8262277
> 27:  * @summary Test to see if URLClassLoader.getResource and 
> URLClassLoader.getResources

The summary mentions the get* methods, but the test calls the find* methods.  I 
think it would be good to test all four methods (getResource, getResources, 
findResource, findResources), and update the summary e.g. "Test if 
URLClassLoader throws IllegalArgumentException when getting or finding 
resources."

-

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

Re: RFR: 8263091: Remove CharacterData.isOtherUppercase/-Lowercase

2021-03-05 Thread Iris Clark

On Fri, 5 Mar 2021 14:24:34 GMT, Claes Redestad  wrote:

> This patch removes the CharacterData.isOtherUppercase and isOtherLowercase 
> methods. It also exploits the fact that isOtherUppercase is always false for 
> all codepoints in the CharacterDataLatin1 range for a small speed-up.
> 
> I have no means to test if this is correct on PPC, which has intrinsics for 
> isLowerCase/isUpperCase, but unless I'm reading the code wrong the intrinsic 
> for isLowerCase on PPC already appears to effectively do the fused logic of 
> isLowerCase(ch) || isOtherLowerCase(ch) since it handles the two values where 
> isLowerCase and isOtherLowercase disagrees (0xaa, 0xba), which means this 
> change should make the intrinsic and the java code be in better agreement.

Marked as reviewed by iris (Reviewer).

-

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

Re: [External] : Re: ThreadLocal lookup elimination

2021-03-05 Thread Eirik Bjørsnøs

>
> But before that. Alan Bateman said to me that Loom project has much
> simpler implementation to what you want.
>

Ahh! Another good reason why I'm seriously considering just going to the
beach and returning once Loom is shipped! :-)

Can't do that. JIT have to update underlying hash table to correctly
> execute application.
>

I don't think I suggested altering the write path, only the reads would be
optimized away. My understanding is those won't update the hash table or
have other side effects.

Cheers,
Eirik.

Re: [External] : Re: ThreadLocal lookup elimination

2021-03-05 Thread Vladimir Kozlov

Eirik

See my answers bellow.

But before that. Alan Bateman said to me that Loom project has much simpler
implementation to what you want.
Thread local scopedCache which is Object[] array referenced from current thread:

https://github.com/openjdk/loom/blob/fibers/src/java.base/share/classes/java/lang/Thread.java#L310
https://github.com/openjdk/loom/blob/fibers/src/hotspot/share/prims/jvm.cpp#L3130

It will be much easy to optimize.

On 3/5/21 11:13 AM, Eirik Bjørsnøs wrote:

On Fri, Mar 5, 2021 at 6:41 PM Vladimir Kozlov mailto:vladimir.koz...@oracle.com>> wrote:

Vladimir,

Thanks a lot for taking time to consider this! Some comments inline:

Currently it is "impossible" for JIT compiler to reliably know that value
stored by set() in hash map is the same as
read by get().

My layman (perhaps naive?) thinking was that a JIT could cheat and infer this by definition. Since a ThreadLocal holds
memory that is by definition local to the executing thread, a JIT should have exclusive access to the content of the
ThreadLocal. Any back-to-back set/get (or get/get) should therefore yield the same value (because no other thread can
update the contents of the ThreadLocal).

However, the JMM doesn't seem to mention ThreadLocals explicitly, so perhaps a JIT would need to look at the
implementation.. As a programmer I would certainly be somewhat surprised if a ThreadLocal get yielded another value than
my immediately preceding set, or if my two consecutive gets would yield different values.

What kind of access patterns could you see breaking this assumption? Reflective access into Thread.threadLocals
internals by clever frameworks, similar to how final fields aren't really final?

Can't do that. JIT have to update underlying hash table to correctly execute
application.

Also because of ThreadLocal accessors's complex code, some calls may not be
inlined and JIT does not know what side
effect they may have - it assumes that they can modify a value.

Not sure I'm following completely. Your concern is that the added "bulk" of ThreadLocal.get could prevent inlining? But
wouldn't this suggested optimization simply replace the get with a local variable access, thus removing the bulk. Or
isn't this the order inlining happens in?

Note, currently JIT does not treat ThreadLocal as something special. It will
inline set() and get() as normal methods.
It will try to inline all hot methods which are called. For example,
ThreadLocalMap::getEntry()
https://github.com/openjdk/jdk/blob/e1cad97049642ab201d53ff608937f7e7ef3ff3e/src/java.base/share/classes/java/lang/ThreadLocal.java#L433

It has 2 paths: miss and not miss. Depending on profile of execution getEntryAfterMiss() method may not be called or
called very little. It is considered cold call site - JIT will not inline it and it does not know what it will do with
hash table and values in it.

Regards,
Vladimir

Thanks,
Eirik.

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v4]

On Thu, 4 Mar 2021 21:10:56 GMT, Craig Andrews 
 wrote:

>> Hi, Craig
>> The commented-out lines should be removed from the change.
>> 
>> As Alan said, a regression test will be needed.  At minimum, it should test 
>> a method that returns a URL, as well as a method that returns an 
>> Enumeration (which can also lead to an IAE, as I noted in the bug 
>> report).
>> 
>> Also, though the compatibility risk is low, it would be good to include a 
>> CSR for this change to long-standing behavior.
>
>>The commented-out lines should be removed from the change.
> 
> Done!  
> 
>> As Alan said, a regression test will be needed. At minimum, it should test a 
>> method that returns a URL, as well as a method that returns an Enumeration 
>> (which can also lead to an IAE, as I noted in the bug report).
> 
> I've added a test.
> 
>> Also, the copyright year should be updated: 2019 -> 2021
> 
> Done!  
> 
>> @bchristi-git has indicated that a [compatibility and 
>> specification](https://wiki.openjdk.java.net/display/csr/Main) (CSR) request 
>> is needed for this pull request.
>> @candrews please create a CSR request and add link to it in 
>> [JDK-8262277](https://bugs.openjdk.java.net/browse/JDK-8262277). This pull 
>> request cannot be integrated until the CSR request is approved.
> 
> I'm trying to figure out how to create a CSR and I'm not having much luck. 
> According to the [CSR 
> FAQs](https://wiki.openjdk.java.net/display/csr/CSR+FAQs):
>> Q: How do I create a CSR ?
> A: Do not directly create a CSR from the Create Menu. JIRA will let you do 
> this right up until the moment you try to save it and find your typing was in 
> vain.
> Instead you should go to the target bug, select "More", and from the drop 
> down menu select "Create CSR". This is required to properly associate the CSR 
> with the main bug, just as is done for backports.
> 
> However, I don't have an account on https://bugs.openjdk.java.net so I can't 
> do as instructed.
> 
> How do I create the CSR?

# Summary
Modify implementation of `java.net.URLClassLoader` such that `findResource` and 
`java.net.URLClassLoader.html#findResources` do no longer throw undocumented 
`IllegalArgumentException`.

# Problem
The javadocs for 
[`URLClassLoader.html#findResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URLClassLoader.html#findResource(java.lang.String))
 and 
[`java.net.URLClassLoader.html#findResources`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URLClassLoader.html#findResources(java.lang.String))
 do not indicate that an `IllegalArgumentException` can be thrown. The 
implementation does throw such exceptions under specific conditions which, due 
to being undocumented, is unexpected behavior. The unexpected exception can 
cause incorrect behavior, particularly on Windows systems due to the Windows 
file system path format increasingly the likelihood of causing the unexpected 
exception. [An example of such a problem was discovered in Spring Framework 
necessitating a 
workaround.](https://github.com/spring-projects/spring-framework/pull/26574).

# Solution
Modify the implementation of `java.net.URLClassLoader` to not throw 
`IllegalArgumentException` when finding resources.

# Compatibility Risk
Low

# Compatibility Risk Description
It is possible that some code is catching the undocumented 
`IllegalArgumentException` and treating it in a specific way.

-

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

Re: ThreadLocal lookup elimination

2021-03-05 Thread Eirik Bjørsnøs

On Fri, Mar 5, 2021 at 6:41 PM Vladimir Kozlov 
wrote:

Vladimir,

Thanks a lot for taking time to consider this! Some comments inline:

> Currently it is "impossible" for JIT compiler to reliably know that value
> stored by set() in hash map is the same as
> read by get().
>

My layman (perhaps naive?) thinking was that a JIT could cheat and infer
this by definition. Since a ThreadLocal holds memory that is by definition
local to the executing thread, a JIT should have exclusive access to the
content of the ThreadLocal. Any back-to-back set/get (or get/get) should
therefore yield the same value (because no other thread can update the
contents of the ThreadLocal).

However, the JMM doesn't seem to mention ThreadLocals explicitly, so
perhaps a JIT would need to look at the implementation.. As a programmer I
would certainly be somewhat surprised if a ThreadLocal get yielded another
value than my immediately preceding set, or if my two consecutive gets
would yield different values.

What kind of access patterns could you see breaking this assumption?
Reflective access into Thread.threadLocals internals by clever frameworks,
similar to how final fields aren't really final?

> Also because of ThreadLocal accessors's complex code, some calls may not
> be inlined and JIT does not know what side
> effect they may have - it assumes that they can modify a value.
>

Not sure I'm following completely. Your concern is that the added "bulk" of
ThreadLocal.get could prevent inlining? But wouldn't this suggested
optimization simply replace the get with a local variable access, thus
removing the bulk. Or isn't this the order inlining happens in?

Thanks,
Eirik.

Re: RFR: 8263090: Avoid reading volatile fields twice in Locale.getDefault(Category) [v2]

2021-03-05 Thread Naoto Sato

On Fri, 5 Mar 2021 18:53:29 GMT, Claes Redestad  wrote:

>> This patch refactors Locale.getDefault(Category) so that the volatile field 
>> holding the Locale is typically only read once. This has a small performance 
>> advantage, and might be more robust if initialization is racy.
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix omitted synchronized

Looks good.

-

Marked as reviewed by naoto (Reviewer).

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

Re: RFR: 8263090: Avoid reading volatile fields twice in Locale.getDefault(Category) [v2]

> This patch refactors Locale.getDefault(Category) so that the volatile field 
> holding the Locale is typically only read once. This has a small performance 
> advantage, and might be more robust if initialization is racy.

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

  Fix omitted synchronized

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2845/files
  - new: https://git.openjdk.java.net/jdk/pull/2845/files/5d2f0da4..4980d2d6

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

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

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

Re: RFR: 8263090: Avoid reading volatile fields twice in Locale.getDefault(Category) [v2]

On Fri, 5 Mar 2021 17:29:16 GMT, Naoto Sato  wrote:

>> Claes Redestad has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix omitted synchronized
>
> src/java.base/share/classes/java/util/Locale.java line 959:
> 
>> 957: }
>> 958: 
>> 959: private static Locale getDisplayLocale() {
> 
> Should this be `synchronized`?

Good catch! Fixed.

-

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

Re: RFR: 8263091: Remove CharacterData.isOtherUppercase/-Lowercase

2021-03-05 Thread Naoto Sato

On Fri, 5 Mar 2021 14:24:34 GMT, Claes Redestad  wrote:

> This patch removes the CharacterData.isOtherUppercase and isOtherLowercase 
> methods. It also exploits the fact that isOtherUppercase is always false for 
> all codepoints in the CharacterDataLatin1 range for a small speed-up.
> 
> I have no means to test if this is correct on PPC, which has intrinsics for 
> isLowerCase/isUpperCase, but unless I'm reading the code wrong the intrinsic 
> for isLowerCase on PPC already appears to effectively do the fused logic of 
> isLowerCase(ch) || isOtherLowerCase(ch) since it handles the two values where 
> isLowerCase and isOtherLowercase disagrees (0xaa, 0xba), which means this 
> change should make the intrinsic and the java code be in better agreement.

Marked as reviewed by naoto (Reviewer).

-

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

Re: ThreadLocal lookup elimination

2021-03-05 Thread Vladimir Kozlov


Hi Erik,

The implementation of ThreadLocal is based on HashMap:
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/ThreadLocal.java#L76

Currently it is "impossible" for JIT compiler to reliably know that value stored by set() in hash map is the same as 
read by get().


Also because of ThreadLocal accessors's complex code, some calls may not be inlined and JIT does not know what side 
effect they may have - it assumes that they can modify a value.


Thanks,
Vladimir K

On 2/22/21 3:26 AM, Eirik Bjørsnøs wrote:

Hello,

ThreadLocals are commonly used to transfer state across method boundaries
in situations where passing them as method parameters is impractical.
Examples include crossing APIs you don't own, or instrumenting code you
don't own.

Consider the following pseudo-instrumented code (where the original code
calls a getter inside a loop):

public class Student {

 private int age;

 public int maxAge(Student[] students) {

 // Instrumented code:
 ExpensiveObject expensive = new ExpensiveObject();
 expensive.recordSomething();
 threadLocal.set(expensive);

 // Original code:
 int max = 0;
 for (Student student : students) {
 max = Math.max(max, student.getAge());
 }
 return max;
 }

 public int getAge() {
 // Instrumented code
 ExpensiveObject exp = threadLocal.get();
 exp.recordSomething();

 // Original code:
 return age;
 }

 // Instrumented field:
 private static ThreadLocal threadLocal = new
ThreadLocal<>();
}

The ThreadLocal is used here to avoid constructing ExpensiveObject
instances in each invocation of getAge.

However, once a compiler worth its salt sees this code, it immediately
wants to inline the getAge method:

// Instrumented code:
ExpensiveObject expensive = new ExpensiveObject();
expensive.recordSomething();
threadLocal.set(expensive);

for (Student student : students) {
 // Instrumented code
 ExpensiveObject exp = threadLocal.get();
 exp.recordSomething();
 // Original code
 max = Math.max(max, student.age);
}

At this point, we see that the last write to threadLocal is 'expensive', so
any following  'threadLocal.get()' should be substitutable for 'expensive'.
So we could do the following instead:

for (Student student : students) {
 // Instrumented code
 expensive.recordSomething();
 // Original code
 max = Math.max(max, student.age);
}

More generally, a compiler could record the first lookup of a ThreadLocal
in a scope and substitute any following lookup with the first read (until
the next write).

I'm pretty sure this would be immensely useful for my current use case
(which instruments methods to count invocations), but perhaps it is also a
useful optimization in a more general sense? Examples that come to mind are
enterprise apps where transaction and security contexts are passed around
using ThreadLocals.

Has this type of optimization been discussed before? Is it even possible to
implement, or did I miss some dragons hiding in the details? What would the
estimated work for an implementation look like? Are we looking at
bachelor's thesis? Master's thesis? PhD?

Would love to hear some thoughts on this idea.

Cheers,
Eirik.

Re: RFR: 8263090: Avoid reading volatile fields twice in Locale.getDefault(Category)

2021-03-05 Thread Naoto Sato

On Fri, 5 Mar 2021 14:14:14 GMT, Claes Redestad  wrote:

> This patch refactors Locale.getDefault(Category) so that the volatile field 
> holding the Locale is typically only read once. This has a small performance 
> advantage, and might be more robust if initialization is racy.

src/java.base/share/classes/java/util/Locale.java line 959:

> 957: }
> 958: 
> 959: private static Locale getDisplayLocale() {

Should this be `synchronized`?

-

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

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

2021-03-05 Thread Richard Reingruber

On Fri, 5 Mar 2021 11:11:44 GMT, Alan Hayward 
 wrote:

>>> I was building this PR on a new machine, and I now get the following error:
>>> 
>>> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:258:31:
>>> >  error: cast to smaller integer type 'MIDIClientRef' (aka 'unsigned int') 
>>> > from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
>>> > static MIDIClientRef client = (MIDIClientRef) NULL;
>>> > ^~~~
>>> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:259:29:
>>> >  error: cast to smaller integer type 'MIDIPortRef' (aka 'unsigned int') 
>>> > from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
>>> > static MIDIPortRef inPort = (MIDIPortRef) NULL;
>>> > ^~
>>> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:260:30:
>>> >  error: cast to smaller integer type 'MIDIPortRef' (aka 'unsigned int') 
>>> > from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
>>> > static MIDIPortRef outPort = (MIDIPortRef) NULL;
>>> > ^~
>>> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:466:32:
>>> >  error: cast to smaller integer type 'MIDIEndpointRef' (aka 'unsigned 
>>> > int') from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
>>> > MIDIEndpointRef endpoint = (MIDIEndpointRef) NULL;
>>> > ^~
>>> > 4 errors generated.
>>> 
>>> As far as I can tell the only difference between the two systems is the 
>>> xcode version:
>>> 
>>> New system (failing)
>>> % xcodebuild -version
>>> Xcode 12.5
>>> Build version 12E5244e
>>> 
>>> Old system (working)
>>> % xcodebuild -version
>>> Xcode 12.4
>>> Build version 12D4e
>>> 
>>> Looks like the newer version of Xcode is being a little stricter with 
>>> casting?
>>> Replacing the NULL with 0 seems to fix the issue.
>> 
>> Hello
>> there is one issue with the info you provided, it's from Xcode12.5 beta.
>> And beta license agreement forbids sharing output of beta version of 
>> compiler
>> So we can't say we have issue with newer xcode beta until that beta went 
>> public & released.
>> Fixing issues you found now will mean someone have violated xcode beta 
>> license agreement and made these issues public.
>
>> Hello
>> there is one issue with the info you provided, it's from Xcode12.5 beta.
>> And beta license agreement forbids sharing output of beta version of 
>> compiler
>> So we can't say we have issue with newer xcode beta until that beta went 
>> public & released.
>> Fixing issues you found now will mean someone have violated xcode beta 
>> license agreement and made these issues public.
> 
> Ok, I wasn't aware of that. I'll downgrade back to the non-beta version.

Hi,

@VladimirKempik reported failure of 
CompressedClassPointers.largeHeapAbove32GTest() with 
[JDK-8262895](https://bugs.openjdk.java.net/browse/JDK-8262895) on 
macos_aarch64.

He also helped analyzing the issue (thanks!).

Apparently the vm has difficulties mapping the heap of 31GB at one of the 
preferred addresses given in
[`get_attach_addresses_for_disjoint_mode()`](https://github.com/openjdk/jdk/blob/8d3de4b1bdb5dc13bb7724596dc2123ba05bbb81/src/hotspot/share/memory/virtualspace.cpp#L477).
 This causes the test failure indirectly.

IMO it could be worth the effort to find out why the heap cannot be mapped at 
the preferred addresses. Reviewers should decide on it. I wouldn't be able to 
do it myself though as I don't have access to a macos_aarch64 system.

Alternatively I'd suggest to exlude macos_aarch64 from the subtest 
largeHeapAbove32GTest.

Best regards,
Richard.

--

 Details of the analysis

In the following trace we see the vm trying to allocate the heap at addresses 
given in 
[`get_attach_addresses_for_disjoint_mode()`](https://github.com/openjdk/jdk/blob/8d3de4b1bdb5dc13bb7724596dc2123ba05bbb81/src/hotspot/share/memory/virtualspace.cpp#L477)
 without success:

images/jdk/bin/java  -XX:+UnlockDiagnosticVMOptions 
-XX:+UnlockExperimentalVMOptions -Xmx31g -XX:-UseAOT 
-Xlog:gc+metaspace=trace,cds=trace,heap+gc+exit=info,gc+heap+coops=trace 
-Xshare:off -XX:+VerifyBeforeGC -XX:HeapSearchSteps=40 -version
OpenJDK 64-Bit Server VM warning: Shared spaces are not supported in this VM
[0.005s][trace][gc,heap,coops] Trying to allocate at address 0x0010 
heap of size 0x7c100
[0.005s][trace][gc,heap,coops] Trying to allocate at address 0x0018 
heap of size 0x7c100
[0.005s][trace][gc,heap,coops] Trying to allocate at address 0x0020 
heap of size 0x7c100
[0.005s][trace][gc,heap,coops] Trying to allocate at address 0x0040 
heap of size 0x7c100
[0.005s][trace][gc,heap,coops] Trying to allocate at address 0x0050 
heap of size 0x7c100
[0.005s][trace][gc,heap,coops] Trying to allocate at address 0x0008 
heap of size 0x7c100

Re: RFR: 8263091: Remove CharacterData.isOtherUppercase/-Lowercase

On Fri, 5 Mar 2021 14:24:34 GMT, Claes Redestad  wrote:

> This patch removes the CharacterData.isOtherUppercase and isOtherLowercase 
> methods. It also exploits the fact that isOtherUppercase is always false for 
> all codepoints in the CharacterDataLatin1 range for a small speed-up.
> 
> I have no means to test if this is correct on PPC, which has intrinsics for 
> isLowerCase/isUpperCase, but unless I'm reading the code wrong the intrinsic 
> for isLowerCase on PPC already appears to effectively do the fused logic of 
> isLowerCase(ch) || isOtherLowerCase(ch) since it handles the two values where 
> isLowerCase and isOtherLowercase disagrees (0xaa, 0xba), which means this 
> change should make the intrinsic and the java code be in better agreement.

Marked as reviewed by rriggs (Reviewer).

-

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

Re: RFR: 8263090: Avoid reading volatile fields twice in Locale.getDefault(Category)

On Fri, 5 Mar 2021 14:14:14 GMT, Claes Redestad  wrote:

> This patch refactors Locale.getDefault(Category) so that the volatile field 
> holding the Locale is typically only read once. This has a small performance 
> advantage, and might be more robust if initialization is racy.

Marked as reviewed by rriggs (Reviewer).

-

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

Re: RFR: 8263038: Optimize String.format for simple specifiers

On Fri, 5 Mar 2021 17:03:34 GMT, Claes Redestad  wrote:

>> src/java.base/share/classes/java/util/Formatter.java line 3017:
>> 
>>> 3015: s = ((Character)arg).toString();
>>> 3016: } else if (arg instanceof Byte) {
>>> 3017: byte i = (Byte) arg;
>> 
>> Can the pattern matching for instanceof be used here to remove explicit 
>> casts.  (Supported in JDK 16) 
>>  s = null;
>> if (arg instanceof Character c) {
>> s = c.toString();
>> } else if (arg instanceof Byte i) {
>> if (Character.isValidCodePoint(i))
>> s = new String(Character.toChars(i));
>> else
>> throw new IllegalFormatCodePointException(i);
>> } else if (arg instanceof Short i) {
>> if (Character.isValidCodePoint(i))
>> s = new String(Character.toChars(i));
>> else
>> throw new IllegalFormatCodePointException(i);
>> } ```
>> etc..
>
> I did think about it, but it seemed to stray a bit too far from the intent of 
> this enhancement.

I only looked at it because of the updates to use switch expressions...
ok, either way.

-

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

Re: RFR: 8263038: Optimize String.format for simple specifiers

On Fri, 5 Mar 2021 16:36:03 GMT, Roger Riggs  wrote:

>> This patch optimizes String.format expressions that uses trivial specifiers. 
>> In the JDK, the most common variation of String.format is a variation of 
>> format("foo: %s", s), which gets a significant speed-up from this.
>> 
>> Various other cleanups and minor improvements reduce overhead further and 
>> ensure we get a small gain also for more complex format strings.
>
> src/java.base/share/classes/java/util/Formatter.java line 3017:
> 
>> 3015: s = ((Character)arg).toString();
>> 3016: } else if (arg instanceof Byte) {
>> 3017: byte i = (Byte) arg;
> 
> Can the pattern matching for instanceof be used here to remove explicit 
> casts.  (Supported in JDK 16) 
>  s = null;
> if (arg instanceof Character c) {
> s = c.toString();
> } else if (arg instanceof Byte i) {
> if (Character.isValidCodePoint(i))
> s = new String(Character.toChars(i));
> else
> throw new IllegalFormatCodePointException(i);
> } else if (arg instanceof Short i) {
> if (Character.isValidCodePoint(i))
> s = new String(Character.toChars(i));
> else
> throw new IllegalFormatCodePointException(i);
> } ```
> etc..

I did think about it, but it seemed to stray a bit too far from the intent of 
this enhancement.

-

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

Re: RFR: 8263038: Optimize String.format for simple specifiers

On Thu, 4 Mar 2021 17:20:40 GMT, Claes Redestad  wrote:

> This patch optimizes String.format expressions that uses trivial specifiers. 
> In the JDK, the most common variation of String.format is a variation of 
> format("foo: %s", s), which gets a significant speed-up from this.
> 
> Various other cleanups and minor improvements reduce overhead further and 
> ensure we get a small gain also for more complex format strings.

Looks good.

src/java.base/share/classes/java/util/Formatter.java line 3017:

> 3015: s = ((Character)arg).toString();
> 3016: } else if (arg instanceof Byte) {
> 3017: byte i = (Byte) arg;

Can the pattern matching for instanceof be used here to remove explicit casts.  
(Supported in JDK 16) 
 s = null;
if (arg instanceof Character c) {
s = c.toString();
} else if (arg instanceof Byte i) {
if (Character.isValidCodePoint(i))
s = new String(Character.toChars(i));
else
throw new IllegalFormatCodePointException(i);
} else if (arg instanceof Short i) {
if (Character.isValidCodePoint(i))
s = new String(Character.toChars(i));
else
throw new IllegalFormatCodePointException(i);
} ```
etc..

-

Marked as reviewed by rriggs (Reviewer).

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

Re: RFR: 8263090: Avoid reading volatile fields twice in Locale.getDefault(Category)

On Fri, 5 Mar 2021 14:14:14 GMT, Claes Redestad  wrote:

> This patch refactors Locale.getDefault(Category) so that the volatile field 
> holding the Locale is typically only read once. This has a small performance 
> advantage, and might be more robust if initialization is racy.

Results on the provided, simple microbenchmark

Baseline:
Benchmark Mode  Cnt   Score   Error  Units
LocaleDefaults.getDefault avgt5  11.142 ± 0.084  ns/op
LocaleDefaults.getDefaultDisplay  avgt5  14.024 ± 0.063  ns/op
LocaleDefaults.getDefaultFormat   avgt5  14.001 ± 0.072  ns/op
Patch:
Benchmark Mode  Cnt   Score   Error  Units
LocaleDefaults.getDefault avgt5  11.210 ± 0.057  ns/op
LocaleDefaults.getDefaultDisplay  avgt5  12.597 ± 0.022  ns/op
LocaleDefaults.getDefaultFormat   avgt5  12.608 ± 0.049  ns/op

-

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

RFR: 8263090: Avoid reading volatile fields twice in Locale.getDefault(Category)

This patch refactors Locale.getDefault(Category) so that the volatile field 
holding the Locale is typically only read once. This has a small performance 
advantage, and might be more robust if initialization is racy.

-

Commit messages:
 - Add microbenchmark
 - Reduce volatile reads in Locale.getDefault(Category)

Changes: https://git.openjdk.java.net/jdk/pull/2845/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2845=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263090
  Stats: 92 lines in 2 files changed: 72 ins; 5 del; 15 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2845.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2845/head:pull/2845

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

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v3]

On Fri, 5 Mar 2021 15:46:40 GMT, Craig Andrews 
 wrote:

>> test/jdk/java/net/URLClassLoader/TestBug8262277.java line 30:
>> 
>>> 28:  * @summary Test to see if URLClassLoader.getResource and 
>>> URLClassLoader.getResources
>>> 29:  * throw IllegalArgumentException
>>> 30:  */
>> 
>> I'd prefer if the test checked that findResource returned null and that 
>> findResources returned an empty enumeration. I think we should be able to 
>> find a better name for the test too.
>> Do you really want the author tag in the test? I know they exist in some 
>> tests but they are impossible to remove, even when tests/code are 
>> significantly changed by someone else.
>
>> I'd prefer if the test checked that findResource returned null and that 
>> findResources returned an empty enumeration.
> 
> I'll update the test accordingly.
> 
>>  think we should be able to find a better name for the test too.
>> Do you really want the author tag in the test? I know they exist in some 
>> tests but they are impossible to remove, even when tests/code are 
>> significantly changed by someone else.
> 
> I can rename the test class to be something descriptive and remove the 
> `@author` tag. I was following other tests which is why I did it this way.

I've made the changes you requested.

-

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

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v3]

On Fri, 5 Mar 2021 15:42:11 GMT, Alan Bateman  wrote:

> I'd prefer if the test checked that findResource returned null and that 
> findResources returned an empty enumeration.

I'll update the test accordingly.

>  think we should be able to find a better name for the test too.
> Do you really want the author tag in the test? I know they exist in some 
> tests but they are impossible to remove, even when tests/code are 
> significantly changed by someone else.

I can rename the test class to be something descriptive and remove the 
`@author` tag. I was following other tests which is why I did it this way.

-

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

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v3]

2021-03-05 Thread Alan Bateman

On Thu, 4 Mar 2021 22:16:10 GMT, Craig Andrews 
 wrote:

>> `java.net.URLClassLoader.getResource` can throw an undocumented 
>> `IllegalArgumentException`.
>> 
>> According to the javadoc for the `getResource` and `findResource` methods, 
>> neither should be throwing `IllegalArgumentException` - they should return 
>> null if the resource can't be resolved.
>> 
>> Quoting the javadoc for 
>> [`URLClassLoader.html#findResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URLClassLoader.html#findResource(java.lang.String))
>>> Returns:
>>> a URL for the resource, or null if the resource could not be found, or 
>>> if the loader is closed.
>> 
>> And quoting the javadoc for 
>> [`ClassLoader.html#getResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String))
>>> Returns:
>>> URL object for reading the resource; null if the resource could not be 
>>> found, a URL could not be constructed to locate the resource, the resource 
>>> is in a package that is not opened unconditionally, or access to the 
>>> resource is denied by the security manager.
>> 
>> Neither mentions throwing `IllegalArgumentException` and both are clear that 
>> when URL can't be constructed, `null` should be returned.
>> 
>> Here's a stack trace:
>> java.lang.IllegalArgumentException: name
>> at 
>> java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:600)
>> at 
>> java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:291)
>> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:655)
>> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:653)
>> at java.base/java.security.AccessController.doPrivileged(Native 
>> Method)
>> at 
>> java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:652)
>> 
>> Looking at 
>> [`URLClassPath.findResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L603)
>> URL findResource(final String name, boolean check) {
>> URL url;
>> try {
>> url = new URL(base, ParseUtil.encodePath(name, false));
>> } catch (MalformedURLException e) {
>> throw new IllegalArgumentException("name");
>> }
>> 
>> Instead of throwing `IllegalArgumentException`, that line should simply 
>> return `null`.
>> 
>> A similar issue exists at 
>> [`URLClassPath.getResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L639)
>> URL findResource(final String name, boolean check) {
>> URL url;
>> try {
>> url = new URL(base, ParseUtil.encodePath(name, false));
>> } catch (MalformedURLException e) {
>> throw new IllegalArgumentException("name");
>> }
>> 
>> Instead of throwing `IllegalArgumentException`, that line should simply 
>> return `null`.
>
> Craig Andrews 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-8262277: java.net.URLClassLoader.getResource throws undocumented 
> IllegalArgumentException

test/jdk/java/net/URLClassLoader/TestBug8262277.java line 30:

> 28:  * @summary Test to see if URLClassLoader.getResource and 
> URLClassLoader.getResources
> 29:  * throw IllegalArgumentException
> 30:  */

I'd prefer if the test checked that findResource returned null and that 
findResources returned an empty enumeration. I think we should be able to find 
a better name for the test too.
Do you really want the author tag in the test? I know they exist in some tests 
but they are impossible to remove, even when tests/code are significantly 
changed by someone else.

-

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

Re: RFR: JDK-8262277: java.net.URLClassLoader.getResource throws undocumented IllegalArgumentException [v4]

> `java.net.URLClassLoader.getResource` can throw an undocumented 
> `IllegalArgumentException`.
> 
> According to the javadoc for the `getResource` and `findResource` methods, 
> neither should be throwing `IllegalArgumentException` - they should return 
> null if the resource can't be resolved.
> 
> Quoting the javadoc for 
> [`URLClassLoader.html#findResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URLClassLoader.html#findResource(java.lang.String))
>> Returns:
>> a URL for the resource, or null if the resource could not be found, or 
>> if the loader is closed.
> 
> And quoting the javadoc for 
> [`ClassLoader.html#getResource`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#getResource(java.lang.String))
>> Returns:
>> URL object for reading the resource; null if the resource could not be 
>> found, a URL could not be constructed to locate the resource, the resource 
>> is in a package that is not opened unconditionally, or access to the 
>> resource is denied by the security manager.
> 
> Neither mentions throwing `IllegalArgumentException` and both are clear that 
> when URL can't be constructed, `null` should be returned.
> 
> Here's a stack trace:
> java.lang.IllegalArgumentException: name
> at 
> java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:600)
> at 
> java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:291)
> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:655)
> at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:653)
> at java.base/java.security.AccessController.doPrivileged(Native 
> Method)
> at 
> java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:652)
> 
> Looking at 
> [`URLClassPath.findResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L603)
> URL findResource(final String name, boolean check) {
> URL url;
> try {
> url = new URL(base, ParseUtil.encodePath(name, false));
> } catch (MalformedURLException e) {
> throw new IllegalArgumentException("name");
> }
> 
> Instead of throwing `IllegalArgumentException`, that line should simply 
> return `null`.
> 
> A similar issue exists at 
> [`URLClassPath.getResource`](https://github.com/openjdk/jdk/blob/2b00367e1154feb2c05b84a11d62fb5750e46acf/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L639)
> URL findResource(final String name, boolean check) {
> URL url;
> try {
> url = new URL(base, ParseUtil.encodePath(name, false));
> } catch (MalformedURLException e) {
> throw new IllegalArgumentException("name");
> }
> 
> Instead of throwing `IllegalArgumentException`, that line should simply 
> return `null`.

Craig Andrews 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-8262277: java.net.URLClassLoader.getResource throws undocumented 
IllegalArgumentException

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2662/files
  - new: https://git.openjdk.java.net/jdk/pull/2662/files/e456f886..79869e78

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

  Stats: 107 lines in 2 files changed: 54 ins; 53 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2662.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2662/head:pull/2662

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

Re: RFR: 8263091: Remove CharacterData.isOtherUppercase/-Lowercase

On Fri, 5 Mar 2021 14:24:34 GMT, Claes Redestad  wrote:

> This patch removes the CharacterData.isOtherUppercase and isOtherLowercase 
> methods. It also exploits the fact that isOtherUppercase is always false for 
> all codepoints in the CharacterDataLatin1 range for a small speed-up.
> 
> I have no means to test if this is correct on PPC, which has intrinsics for 
> isLowerCase/isUpperCase, but unless I'm reading the code wrong the intrinsic 
> for isLowerCase on PPC already appears to effectively do the fused logic of 
> isLowerCase(ch) || isOtherLowerCase(ch) since it handles the two values where 
> isLowerCase and isOtherLowercase disagrees (0xaa, 0xba), which means this 
> change should make the intrinsic and the java code be in better agreement.

Microbenchmark results, org.openjdk.bench.java.lang.Characters

OOTB:
Benchmark   (codePoint)  Mode  Cnt   Score   Error  Units
Characters.isLowerCase  176  avgt5  13.812 ± 0.060  ns/op
Characters.isUpperCase  176  avgt5  13.812 ± 0.062  ns/op

Benchmark   (codePoint)  Mode  Cnt   Score   Error  Units
Characters.isLowerCase  176  avgt5  13.825 ± 0.096  ns/op
Characters.isUpperCase  176  avgt5  12.555 ± 0.033  ns/op
~1.1x speed-up for Character.isUpperCase in the latin 1 range

-Xint:
Benchmark   (codePoint)  Mode  CntScoreError  Units
Characters.isLowerCase  176  avgt5  754.756 ± 20.815  ns/op
Characters.isUpperCase  176  avgt5  755.403 ± 15.645  ns/op

Benchmark   (codePoint)  Mode  CntScore   Error  Units
Characters.isLowerCase  176  avgt5  606.923 ± 1.569  ns/op
Characters.isUpperCase  176  avgt5  521.073 ± 7.439  ns/op
1.25x speed-up for isLowerCase and 1.45x speed-up for isUpperCase when 
interpreting, translating to minor startup / warmup win on some examined apps.

-

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

RFR: 8263091: Remove CharacterData.isOtherUppercase/-Lowercase