date:20220420

> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare 
> values by identity. Updated API documentation of these two methods 
> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object)))
>  to mention such behavior.

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

  Revamp test and changes. Let ci run the tests

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8259/files
  - new: https://git.openjdk.java.net/jdk/pull/8259/files/9bb9ef57..dd416079

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

  Stats: 281 lines in 3 files changed: 191 ins; 88 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8259.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8259/head:pull/8259

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

Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator [v2]

2022-04-20 Thread Jie Fu

> Hi all,
> 
> The Current Vector API doc for `LSHR` is
> 
> Produce a>>>(n&(ESIZE*8-1)). Integral only.
> 
> 
> This is misleading which may lead to bugs for Java developers.
> This is because for negative byte/short elements, the results computed by 
> `LSHR` will be different from that of `>>>`.
> For more details, please see 
> https://github.com/openjdk/jdk/pull/8276#issue-1206391831 .
> 
> After the patch, the doc for `LSHR` is
> 
> Produce zero-extended right shift of a by (n&(ESIZE*8-1)) bits. Integral only.
> 
> 
> Thanks.
> Best regards,
> Jie

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

 - Address review comments
 - Merge branch 'master' into JDK-8284992
 - 8284992: Fix misleading Vector API doc for LSHR operator

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8291/files
  - new: https://git.openjdk.java.net/jdk/pull/8291/files/50235163..1c7f4584

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

  Stats: 11427 lines in 826 files changed: 6952 ins; 1816 del; 2659 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8291.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8291/head:pull/8291

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

Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v20]

2022-04-20 Thread Yasser Bazzi

> Hi, could i get a review on this implementation proposed by Stuart Marks, i 
> decided to use the 
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html
>  interface to create the default method `asRandom()` that wraps around the 
> newer algorithms to be used on classes that do not accept the new interface.
> 
> Some things to note as proposed by the bug report, the protected method 
> next(int bits) is not overrided and setSeed() method if left blank up to 
> discussion on what to do with it.
> 
> Small test done on 
> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4

Yasser Bazzi has updated the pull request incrementally with two additional 
commits since the last revision:

 - Update setSeed docs on Random class
 - Add nicer toString method to random wrapper

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7001/files
  - new: https://git.openjdk.java.net/jdk/pull/7001/files/2208f848..a7c1818c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7001=19
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7001=18-19

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

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

Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v19]

2022-04-20 Thread Yasser Bazzi

On Wed, 16 Mar 2022 14:54:41 GMT, Yasser Bazzi  wrote:

>> Hi, could i get a review on this implementation proposed by Stuart Marks, i 
>> decided to use the 
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html
>>  interface to create the default method `asRandom()` that wraps around the 
>> newer algorithms to be used on classes that do not accept the new interface.
>> 
>> Some things to note as proposed by the bug report, the protected method 
>> next(int bits) is not overrided and setSeed() method if left blank up to 
>> discussion on what to do with it.
>> 
>> Small test done on 
>> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4
>
> Yasser Bazzi has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add since in javadoc

Well after a long time away i finally got time to finish this, sorry for the 
delay, please review if any changes are necessary

-

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

Re: RFR: 8284942: Proxy building can just iterate superinterfaces once [v2]

> Currently, in ProxyBuilder::mapToModule and ProxyBuilder::defineProxyClass, 
> the interfaces are iterated twice. The two passes can be merged into one, 
> yielding the whole proxy definition context (module, package, whether there's 
> package-private interface) when determining the module.
> 
> Split from #8278. Helpful for moving proxies to hidden classes, but is a good 
> cleanup on its own.

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

  Don't need to complexify module cache

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8281/files
  - new: https://git.openjdk.java.net/jdk/pull/8281/files/08796879..d58bb7e0

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

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

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

Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator

2022-04-20 Thread Jie Fu

On Tue, 19 Apr 2022 08:41:50 GMT, Jie Fu  wrote:

> Hi all,
> 
> The Current Vector API doc for `LSHR` is
> 
> Produce a>>>(n&(ESIZE*8-1)). Integral only.
> 
> 
> This is misleading which may lead to bugs for Java developers.
> This is because for negative byte/short elements, the results computed by 
> `LSHR` will be different from that of `>>>`.
> For more details, please see 
> https://github.com/openjdk/jdk/pull/8276#issue-1206391831 .
> 
> After the patch, the doc for `LSHR` is
> 
> Produce zero-extended right shift of a by (n&(ESIZE*8-1)) bits. Integral only.
> 
> 
> Thanks.
> Best regards,
> Jie

Add hotspot-compiler since the JBS has been moved there.

-

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

Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V)

On Thu, 21 Apr 2022 01:01:25 GMT, Stuart Marks  wrote:

>> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare 
>> values by identity. Updated API documentation of these two methods 
>> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object)))
>>  to mention such behavior.
>
> src/java.base/share/classes/java/util/IdentityHashMap.java line 1412:
> 
>> 1410: i = nextKeyIndex(i, len);
>> 1411: }
>> 1412: }
> 
> Unfortunately there's some mostly-duplicate code here. However, there's 
> similar logic and code sprinkled throughout this class, so _more_ duplication 
> isn't necessarily the wrong thing to do. However, trying to unify this logic 
> results in much more intrusive refactoring, which is harder to review, and 
> which isn't backed up with tests (see JDK-8285295) which I wouldn't encourage 
> pursuing right now. In other words, I'm ok with this duplicate logic.

On intrusive logic: I planned address it in 
https://bugs.openjdk.java.net/browse/JDK-8277520 (#6532), and if this one is 
integrated first, it may be possible to fix it up there.

-

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

Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V)

On Fri, 15 Apr 2022 05:58:32 GMT, liach  wrote:

> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare 
> values by identity. Updated API documentation of these two methods 
> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object)))
>  to mention such behavior.

test/jdk/java/util/IdentityHashMap/DefaultRemoveReplace.java line 68:

> 66: }
> 67: }
> 68: }

Overall comments on this test. It does test the right set of four cases 
(positive and negative calls to `replace` and `remove`). However, it's **one** 
test that tests the four cases, instead of four tests. Using the same state 
makes it hard to add or maintain test cases in the future. It also trusts the 
return value of the method calls and doesn't make any assertions over the 
actual contents of the map. Without assertions over the expected contents, a 
method could behavior incorrectly and cause unrelated and confusing errors 
downstream, or even cause errors to be missed. I'd thus recommend the following:

* Convert this to a Test-NG test and use a `@BeforeTest` method to set up a 
test fixture consisting of a map containing the desired entries.
* Make each test case into its own test method that performs the method call 
and then makes assertions over the return value and expected result state of 
the map. Individual test methods is probably fine; no need to use a data 
provider for this.
* Probably add a "null hypothesis" test to ensure that the test fixture itself 
has the right properties, similar to the assertions at lines 38-44.
* Instead of fiddling around with strings, which have additional complexity 
caused by interning of string literals, I'd suggest using the technique I used 
in [JDK-8285295](https://bugs.openjdk.java.net/browse/JDK-8285295) and create a 
`record Box(int i) { }` class. With this it's easy to get multiple instances 
that are != but are equals.
* After further thought, maybe additional cases are necessary. For example, 
tests of calls to `remove` and `replace` with a key that is equals() but != to 
a key in the map.

-

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

Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V)

On Fri, 15 Apr 2022 05:58:32 GMT, liach  wrote:

> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare 
> values by identity. Updated API documentation of these two methods 
> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object)))
>  to mention such behavior.

src/java.base/share/classes/java/util/IdentityHashMap.java line 1412:

> 1410: i = nextKeyIndex(i, len);
> 1411: }
> 1412: }

Unfortunately there's some mostly-duplicate code here. However, there's similar 
logic and code sprinkled throughout this class, so _more_ duplication isn't 
necessarily the wrong thing to do. However, trying to unify this logic results 
in much more intrusive refactoring, which is harder to review, and which isn't 
backed up with tests (see JDK-8285295) which I wouldn't encourage pursuing 
right now. In other words, I'm ok with this duplicate logic.

-

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

Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V)

On Fri, 15 Apr 2022 05:58:32 GMT, liach  wrote:

> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare 
> values by identity. Updated API documentation of these two methods 
> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object)))
>  to mention such behavior.

src/java.base/share/classes/java/util/IdentityHashMap.java line 1163:

> 1161: public boolean remove(Object o) {
> 1162: return o instanceof Entry entry
> 1163: && IdentityHashMap.this.remove(entry.getKey(), 
> entry.getValue());

I would prefer to keep the internal `removeMapping` method and have other 
methods call it, instead of calling a public method. In particular the 
`EntrySet.remove()` method here should call an internal method to perform the 
actual removal instead of calling the public `remove()` method, since that 
potentially exposes the "self-use" to subclasses. The the public `remove()` 
method on IDHM could call `removeMapping`.

-

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

Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V)

On Fri, 15 Apr 2022 05:58:32 GMT, liach  wrote:

> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare 
> values by identity. Updated API documentation of these two methods 
> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object)))
>  to mention such behavior.

Thanks for working on this. Yes I can review and sponsor this change.

Sorry I got a bit distracted. I started looking at the test here, and this lead 
me to inquire about what other tests we have for `IdentityHashMap`, and the 
answer is: not enough. See 
[JDK-8285295](https://bugs.openjdk.java.net/browse/JDK-8285295). (But that 
should be handled separately.)

-

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

Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]

On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter  wrote:

>> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods 
>> of `java.io.FilterInputStream`.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8284930: Also remove synchronized keyword from mark() and reset() of 
> InputStream

Oh, sorry, I had missed that synchronization had already been removed from the 
`InputStream` methods. Needless to say, I approve. :-)

-

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

Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]

2022-04-20 Thread Brian Burkhalter

On Thu, 21 Apr 2022 00:00:47 GMT, Stuart Marks  wrote:

>

> I think it's a vanishingly small possibility that anything is relying on the 
> synchronization in these methods. Synchronization here would provide proper 
> memory visibility effects across threads. To use input streams from multiple 
> threads without interleaving operations, one would have to provide some other 
> means of coordination among those threads, which itself would likely ensure 
> proper memory visibility. I'm hard pressed to see how threads could 
> coordinate solely via use of the `mark` and `reset` methods. Therefore, I 
> think it's safe to remove synchronization from them.
> 
> This reasoning also applies to `InputStream`. Perhaps synchronization can be 
> removed from there too.
> 
> I agree that a CSR is probably warranted to capture the behavior change and 
> analysis.

Commit f210cbf5f6bf58326a379b4b3f55fddf49d30f5c removed the synchronization 
from `InputStream`'s `mark()` and `reset()`; a CSR is on file.

-

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

Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]

On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter  wrote:

>> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods 
>> of `java.io.FilterInputStream`.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8284930: Also remove synchronized keyword from mark() and reset() of 
> InputStream

I think it's a vanishingly small possibility that anything is relying on the 
synchronization in these methods. Synchronization here would provide proper 
memory visibility effects across threads. To use input streams from multiple 
threads without interleaving operations, one would have to provide some other 
means of coordination among those threads, which itself would likely ensure 
proper memory visibility. I'm hard pressed to see how threads could coordinate 
solely via use of the `mark` and `reset` methods. Therefore, I think it's safe 
to remove synchronization from them.

This reasoning also applies to `InputStream`. Perhaps synchronization can be 
removed from there too.

I agree that a CSR is probably warranted to capture the behavior change and 
analysis.

-

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

RFR: 8283324: CLDRConverter run time increased by 3x

2022-04-20 Thread Naoto Sato

Fixing performance regression caused by the fix to 
https://bugs.openjdk.java.net/browse/JDK-8176706. The fix introduced extra 
looping through the resource map multiple times which was not necessary. The 
execution time of the tool now got back on par with close to JDK18.

-

Commit messages:
 - 8283324: CLDRConverter run time increased by 3x

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

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

Integrated: 8284548: Invalid XPath expression causes StringIndexOutOfBoundsException

On Wed, 20 Apr 2022 20:08:01 GMT, Joe Wang  wrote:

> Patch note:
> 
> A previous patch had a bug that missed the boundary check, that will cause 
> StringIndexOutOfBoundsException to be thrown instead of 
> XPathExpressionException as expected.
> 
> Fix: the fix is to check the boundaries of the parameter "index". 
> Objects.checkIndex is removed as it's redundant.

This pull request has now been integrated.

Changeset: 994f2e92
Author:Joe Wang 
URL:   
https://git.openjdk.java.net/jdk/commit/994f2e9271355bebf355279d0208c1d2054bab27
Stats: 86 lines in 2 files changed: 82 ins; 2 del; 2 mod

8284548: Invalid XPath expression causes StringIndexOutOfBoundsException

Reviewed-by: naoto, lancea

-

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

Re: RFR: 8284548: Invalid XPath expression causes StringIndexOutOfBoundsException [v2]

2022-04-20 Thread Lance Andersen

On Wed, 20 Apr 2022 21:53:46 GMT, Joe Wang  wrote:

>> Patch note:
>> 
>> A previous patch had a bug that missed the boundary check, that will cause 
>> StringIndexOutOfBoundsException to be thrown instead of 
>> XPathExpressionException as expected.
>> 
>> Fix: the fix is to check the boundaries of the parameter "index". 
>> Objects.checkIndex is removed as it's redundant.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add the test

Marked as reviewed by lancea (Reviewer).

-

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

Re: RFR: 8284548: Invalid XPath expression causes StringIndexOutOfBoundsException [v2]

> Patch note:
> 
> A previous patch had a bug that missed the boundary check, that will cause 
> StringIndexOutOfBoundsException to be thrown instead of 
> XPathExpressionException as expected.
> 
> Fix: the fix is to check the boundaries of the parameter "index". 
> Objects.checkIndex is removed as it's redundant.

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

  add the test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8323/files
  - new: https://git.openjdk.java.net/jdk/pull/8323/files/4205ea6f..17b8c3e6

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

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

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16 [v3]

On Wed, 20 Apr 2022 21:19:14 GMT, Roger Riggs  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   remove = check
>
> Can you run the JMH against the code before either change (or an existing 
> JDK).
> It would be interesting to quantify the improvements of going straight to 
> Latin1.
> 
> (Understanding current hardware architectures and their parallelism is hard 
> to understand well.
> They do clever things with branch prediction and potentially optimistically 
> executing both paths
> and then discarding the non-branch case.  The existing code for toLower and 
> toUpper already includes a branch or two; adding one more branch to the 
> sequence likely can't be optimized.)
> 
> These interactions at the instruction level is why measuring is important.
> Thanks

@RogerRiggs 

seems all 4 tests related runs very very slightlier slower in original codes, 
before change it to CharacterDataLatin1.instance


Jmh Result (original)


# JMH version: 1.34
# VM version: JDK 19-internal, OpenJDK 64-Bit Server VM, 19-internal-adhoc..jdk
# VM invoker: 
F:\workspace\jdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe
# VM options: 
-Djava.library.path=f:\workspace\jdk\build\windows-x86_64-server-release\images\test\micro\native
# Blackhole mode: compiler (auto-detected, use -Djmh.blackhole.autoDetect=false 
to disable)
# Warmup: 10 iterations, 1000 ms each
# Measurement: 5 iterations, 1000 ms each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Average time, time/op
# Benchmark: org.openjdk.bench.java.lang.StringOther.charAt

# Run progress: 0.00% complete, ETA 00:06:00
# Fork: 1 of 3
# Warmup Iteration   1: 70.457 ns/op
# Warmup Iteration   2: 73.255 ns/op
# Warmup Iteration   3: 71.446 ns/op
# Warmup Iteration   4: 73.454 ns/op
# Warmup Iteration   5: 73.618 ns/op
# Warmup Iteration   6: 72.853 ns/op
# Warmup Iteration   7: 74.493 ns/op
# Warmup Iteration   8: 74.862 ns/op
# Warmup Iteration   9: 74.323 ns/op
# Warmup Iteration  10: 73.759 ns/op
Iteration   1: 73.053 ns/op
Iteration   2: 70.180 ns/op
Iteration   3: 70.337 ns/op
Iteration   4: 73.237 ns/op
Iteration   5: 73.156 ns/op

# Run progress: 4.17% complete, ETA 00:05:52
# Fork: 2 of 3
# Warmup Iteration   1: 72.617 ns/op
# Warmup Iteration   2: 71.344 ns/op
# Warmup Iteration   3: 73.582 ns/op
# Warmup Iteration   4: 73.309 ns/op
# Warmup Iteration   5: 74.078 ns/op
# Warmup Iteration   6: 75.177 ns/op
# Warmup Iteration   7: 72.578 ns/op
# Warmup Iteration   8: 72.028 ns/op
# Warmup Iteration   9: 72.897 ns/op
# Warmup Iteration  10: 69.317 ns/op
Iteration   1: 69.303 ns/op
Iteration   2: 69.157 ns/op
Iteration   3: 69.078 ns/op
Iteration   4: 69.142 ns/op
Iteration   5: 70.084 ns/op

# Run progress: 8.33% complete, ETA 00:05:36
# Fork: 3 of 3
# Warmup Iteration   1: 72.149 ns/op
# Warmup Iteration   2: 73.753 ns/op
# Warmup Iteration   3: 70.621 ns/op
# Warmup Iteration   4: 74.384 ns/op
# Warmup Iteration   5: 71.787 ns/op
# Warmup Iteration   6: 74.946 ns/op
# Warmup Iteration   7: 74.463 ns/op
# Warmup Iteration   8: 73.403 ns/op
# Warmup Iteration   9: 71.675 ns/op
# Warmup Iteration  10: 72.550 ns/op
Iteration   1: 66.963 ns/op
Iteration   2: 72.209 ns/op
Iteration   3: 71.639 ns/op
Iteration   4: 71.262 ns/op
Iteration   5: 74.514 ns/op


Result "org.openjdk.bench.java.lang.StringOther.charAt":
  70.888 ±(99.9%) 2.207 ns/op [Average]
  (min, avg, max) = (66.963, 70.888, 74.514), stdev = 2.064
  CI (99.9%): [68.681, 73.094] (assumes normal distribution)


# JMH version: 1.34
# VM version: JDK 19-internal, OpenJDK 64-Bit Server VM, 19-internal-adhoc..jdk
# VM invoker: 
F:\workspace\jdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe
# VM options: 
-Djava.library.path=f:\workspace\jdk\build\windows-x86_64-server-release\images\test\micro\native
# Blackhole mode: compiler (auto-detected, use -Djmh.blackhole.autoDetect=false 
to disable)
# Warmup: 10 iterations, 1000 ms each
# Measurement: 5 iterations, 1000 ms each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Average time, time/op
# Benchmark: org.openjdk.bench.java.lang.StringOther.compareTo

# Run progress: 12.50% complete, ETA 00:05:21
# Fork: 1 of 3
# Warmup Iteration   1: 7.648 ns/op
# Warmup Iteration   2: 7.576 ns/op
# Warmup Iteration   3: 7.024 ns/op
# Warmup Iteration   4: 7.595 ns/op
# Warmup Iteration   5: 7.605 ns/op
# Warmup Iteration   6: 7.555 ns/op
# Warmup Iteration   7: 7.573 ns/op
# Warmup Iteration   8: 7.592 ns/op
# Warmup Iteration   9: 7.673 ns/op
# Warmup Iteration  10: 7.255 ns/op
Iteration   1: 7.268 ns/op
Iteration   2: 7.183 ns/op
Iteration   3: 7.267 ns/op
Iteration   4: 7.253 ns/op
Iteration   5: 7.248 ns/op

# Run progress: 16.67% complete, ETA 00:05:06
# Fork: 2 of 3
# Warmup Iteration   1: 7.654 ns/op
# Warmup Iteration   2: 7.626 ns/op
# Warmup Iteration   3: 7.006

Re: Improve `finally` block exception handling

2022-04-20 Thread some-java-user-99206970363698485155

Thanks a lot for your feedback! My original message was a bit vague in parts, 
sorry for that.

The proposal consists of the following:
1. Forbidding usage of `break`, `continue`, `yield` and `return` in `finally` 
blocks
2. Adding exceptions as suppressed exceptions:
   If exception E1 led to execution of the `finally` block and the block is 
left due to
   another exception E2, then either E1 or E2 should be thrown with the other 
one added
   as suppressed exception. For consistency with try-with-resources and because 
E1 is
   most likely more relevant ideally E1 would be thrown and E2 should be 
suppressed. But
   if you think the impact on backward compatibility would be too large, then 
E2 should
   be thrown (the current behavior), but E1 should at least be added as 
suppressed exception.

The following replies to your comments hopefully make the rationale for these 
proposed
changes clearer.


> The behaviour of try/catch/finally is not "obvious at all", you have to read 
> what it means
> and when you do that you clearly see what happens regarding exceptions.

For try-catch you see that the code catches some specific exception and then 
handles it in a
certain way, whether that is by rethrowing, logging or itentionally ignoring it.
The issue with `finally` is that in its current form, exceptions which occurred 
in the `try`
block might just silently disappear. Consider this simple example:
```
try {
throw new RuntimeException();
} finally {
return true;
}
```
Here it is not clear at all, unless you have read the JLS in detail, that the 
thrown exception
just vanishes. There is no explicit indication that the `finally` has any 
effect on the
thrown exception. Of course this is a contrived example, but consider the same 
situation where
the `try` block calls a method which might throw an exception (or the general 
case that a
VirtualMachineError occurs), and that the `return` (or any of the other 
affected statements)
is not the only statement in the `finally` block.

Similar confusing code can be written where the `try` or `catch` block returns 
a value
(or continues or breaks loop iteration), but the `finally` block overwrites 
that action:
```
try {
return 1;
} finally {
return 2;
}
```
Again, contrived, but consider the same code with additional statements in the 
`try` and
`finally` blocks. This breaks fundamental assumptions one has about the 
behavior of the
statements, such as `return`.

>> Are there any plans to forbid usage of `break`, `continue`, `yield` and 
>> `return` in
>> `finally` blocks?
>
> Why would we do that? What would that gain?

It would prevent code such as the one shown above, where code, most likely 
unintentionally,
discards exceptions. Now also consider that inside the `try` block a 
VirtualMachineError
(or a subclass of it) may occur which you really should not catch. Yet the code 
in the
`finally` block silently discards it without you ever noticing the error, 
before it occurs
later in another part of the application again and has possibly already 
corrupted the state
of the application.


>> Similarly for `throw` and any implicitly thrown exceptions, is there a plan 
>> to at least
>> add the original exception as suppressed one to the newly thrown?
>
> That suggestion is not completely without merit, but nor is it a "slam-dunk" 
> obvious thing to do. The semantic implications of the exceptions matter, and 
> semantics come from the programmers intent. There's no reasonable way to 
> automagically determine that when an exception is created that another 
> exception (that led to the finally block) should be inserted as a "suppressed 
> exception". That would actually be completely wrong to do in many 
> circumstances you would instead need to act when the exception would 
> terminate the finally block and then add the original exception as the 
> "suppressed" exception.

To clarify my suggestion: If a `finally` block is entered due to an exception 
E1, and is
exited due to an exception E2 (regardless of whether explicitly thrown by a 
`throw` statement),
and E1 != E2, then both exceptions should be preserved, with one being added as 
suppressed
exception to the other one.


> But really the programmer is in the best position to decide how exceptions 
> need to be handled.

Except that in a `finally` block they don't have access to the exception which 
led to execution
of the `finally` block, unless they write verbose hacky code to first have a 
`catch (Throwable)`
which stores the caught exception in a local variable. To me it appears for 
many `finally`
blocks in existing code it was not actively decided how exception handling 
should be done,
but rather it was forgotten that the original exception might get discarded 
when the `finally`
throws a new exception; or that behavior was considered acceptable because 
currently Java does
not allow you to handle it in a better way (which is one of the main points of 
this proposal).

>> Of course one

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16 [v3]

On Wed, 20 Apr 2022 21:08:19 GMT, XenoAmess  wrote:

>> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
>> https://github.com/openjdk/jdk/pull/8292/
>> 
>> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
>> continue;
>> }
>> 
>> should be changed to 
>> 
>> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
>> c1) == Character.toLowerCase(u2)) {
>> continue;
>> }
>> 
>> as:
>> 
>> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
>> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
>> don't need a lowercase cauculation.
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove = check

Can you run the JMH against the code before either change (or an existing JDK).
It would be interesting to quantify the improvements of going straight to 
Latin1.

(Understanding current hardware architectures and their parallelism is hard to 
understand well.
They do clever things with branch prediction and potentially optimistically 
executing both paths
and then discarding the non-branch case.  The existing code for toLower and 
toUpper already includes a branch or two; adding one more branch to the 
sequence likely can't be optimized.)

These interactions at the instruction level is why measuring is important.
Thanks

-

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16 [v2]

On Wed, 20 Apr 2022 20:56:50 GMT, Roger Riggs  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   add jmh test
>
> Thanks for the JMH tests and data.
> 
> The only part needed from the JMH run is the last 9 lines.  The rest is noise.
> If it was formatted as a literal it would be easier to read.
> 
> What I see is that the run with == is quite a bit slower.
> 
> With the == check:
> 
> 
> StringOther.regionMatchesU1024LL  avgt   15   187.258 ±   1.038  ns/op
> StringOther.regionMatchesU1024LU  avgt   15  2589.833 ±   8.823  ns/op
> StringOther.regionMatchesU1024UL  avgt   15  2379.645 ±   6.481  ns/op
> StringOther.regionMatchesU1024UU  avgt   15   191.587 ±   7.069  ns/op
> 
> 
> Without the == check:
> 
> 
> StringOther.regionMatchesU1024LL  avgt   15   187.732 ±   1.914  ns/op
> StringOther.regionMatchesU1024LU  avgt   15  1324.156 ±  11.761  ns/op
> StringOther.regionMatchesU1024UL  avgt   15  1331.857 ±  22.509  ns/op
> StringOther.regionMatchesU1024UU  avgt   15   188.872 ±   2.396  ns/op
> 
> 
> In the JMH cases, does the long prefix of latin1 characters distort the 
> timings?

@RogerRiggs

> What I see is that the run with == is quite a bit slower.

Yes, the result is amazing to me. Before you reply I re-run several times but 
similar result. So I respect the truth.

> In the JMH cases, does the long prefix of latin1 characters distort the 
> timings?

No, the long prefix part is where real difference comes.

So according to jmh result, the = check removed.

-

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16 [v3]

> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
> https://github.com/openjdk/jdk/pull/8292/
> 
> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> should be changed to 
> 
> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
> c1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> as:
> 
> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
> don't need a lowercase cauculation.

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

  remove = check

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8308/files
  - new: https://git.openjdk.java.net/jdk/pull/8308/files/ac6d75e4..58a1732c

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

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

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16 [v2]

On Wed, 20 Apr 2022 20:09:06 GMT, XenoAmess  wrote:

>> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
>> https://github.com/openjdk/jdk/pull/8292/
>> 
>> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
>> continue;
>> }
>> 
>> should be changed to 
>> 
>> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
>> c1) == Character.toLowerCase(u2)) {
>> continue;
>> }
>> 
>> as:
>> 
>> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
>> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
>> don't need a lowercase cauculation.
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add jmh test

Thanks for the JMH tests and data.

The only part needed from the JMH run is the last 9 lines.  The rest is noise.
If it was formatted as a literal it would be easier to read.

What I see is that the run with == is quite a bit slower.

With the == check:


StringOther.regionMatchesU1024LL  avgt   15   187.258 ±   1.038  ns/op
StringOther.regionMatchesU1024LU  avgt   15  2589.833 ±   8.823  ns/op
StringOther.regionMatchesU1024UL  avgt   15  2379.645 ±   6.481  ns/op
StringOther.regionMatchesU1024UU  avgt   15   191.587 ±   7.069  ns/op


Without the == check:


StringOther.regionMatchesU1024LL  avgt   15   187.732 ±   1.914  ns/op
StringOther.regionMatchesU1024LU  avgt   15  1324.156 ±  11.761  ns/op
StringOther.regionMatchesU1024UL  avgt   15  1331.857 ±  22.509  ns/op
StringOther.regionMatchesU1024UU  avgt   15   188.872 ±   2.396  ns/op


In the JMH cases, does the long prefix of latin1 characters distort the timings?

-

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

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v5]

2022-04-20 Thread Jan Lahoda

On Wed, 20 Apr 2022 14:50:42 GMT, Vicente Romero  wrote:

> nit: just ran langtools tests as part of my routine and these seem to be 
> failing, `CheckExamples.java` due to:
> 
> ```
> test/langtools/tools/javac/diags/examples/FeatureTotalPatternsInInstanceof.java
>  and
> test/langtools/tools/javac/diags/examples/FeatureTotalPatternsInInstanceof.java
> ```
> 
> and this one too:
> 
> ```
> test/langtools/tools/javac/patterns/InstanceofTotalPattern.java (seems to be 
> a golden file issue)
> ```

Thanks for checking and sorry for that. Will fix in next update.

-

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

Re: RFR: 8284548: Invalid XPath expression causes StringIndexOutOfBoundsException

2022-04-20 Thread Naoto Sato

On Wed, 20 Apr 2022 20:08:01 GMT, Joe Wang  wrote:

> Patch note:
> 
> A previous patch had a bug that missed the boundary check, that will 
> cause StringIndexOutOfBoundsException to be thrown instead of 
> XPathExpressionException as expected.
> 
> Fix: the fix is to check the boundaries of the parameter "index". 
> Objects.checkIndex is removed as it's redundant.

Marked as reviewed by naoto (Reviewer).

-

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16 [v2]

On Wed, 20 Apr 2022 20:09:06 GMT, XenoAmess  wrote:

>> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
>> https://github.com/openjdk/jdk/pull/8292/
>> 
>> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
>> continue;
>> }
>> 
>> should be changed to 
>> 
>> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
>> c1) == Character.toLowerCase(u2)) {
>> continue;
>> }
>> 
>> as:
>> 
>> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
>> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
>> don't need a lowercase cauculation.
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add jmh test


Jmh Result(without = check)


# JMH version: 1.34
# VM version: JDK 19-internal, OpenJDK 64-Bit Server VM, 19-internal-adhoc..jdk
# VM invoker: 
F:\workspace\jdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe
# VM options: 
-Djava.library.path=f:\workspace\jdk\build\windows-x86_64-server-release\images\test\micro\native
# Blackhole mode: compiler (auto-detected, use -Djmh.blackhole.autoDetect=false 
to disable)
# Warmup: 10 iterations, 1000 ms each
# Measurement: 5 iterations, 1000 ms each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Average time, time/op
# Benchmark: org.openjdk.bench.java.lang.StringOther.charAt

# Run progress: 0.00% complete, ETA 00:06:00
# Fork: 1 of 3
# Warmup Iteration   1: 74.478 ns/op
# Warmup Iteration   2: 72.357 ns/op
# Warmup Iteration   3: 72.555 ns/op
# Warmup Iteration   4: 73.901 ns/op
# Warmup Iteration   5: 73.573 ns/op
# Warmup Iteration   6: 73.771 ns/op
# Warmup Iteration   7: 70.427 ns/op
# Warmup Iteration   8: 75.018 ns/op
# Warmup Iteration   9: 74.755 ns/op
# Warmup Iteration  10: 69.792 ns/op
Iteration   1: 72.330 ns/op
Iteration   2: 73.277 ns/op
Iteration   3: 73.678 ns/op
Iteration   4: 68.865 ns/op
Iteration   5: 74.953 ns/op

# Run progress: 4.17% complete, ETA 00:05:52
# Fork: 2 of 3
# Warmup Iteration   1: 74.439 ns/op
# Warmup Iteration   2: 73.358 ns/op
# Warmup Iteration   3: 71.799 ns/op
# Warmup Iteration   4: 70.913 ns/op
# Warmup Iteration   5: 71.575 ns/op
# Warmup Iteration   6: 69.340 ns/op
# Warmup Iteration   7: 69.536 ns/op
# Warmup Iteration   8: 69.717 ns/op
# Warmup Iteration   9: 70.526 ns/op
# Warmup Iteration  10: 74.645 ns/op
Iteration   1: 73.770 ns/op
Iteration   2: 74.756 ns/op
Iteration   3: 73.574 ns/op
Iteration   4: 72.319 ns/op
Iteration   5: 72.092 ns/op

# Run progress: 8.33% complete, ETA 00:05:36
# Fork: 3 of 3
# Warmup Iteration   1: 73.593 ns/op
# Warmup Iteration   2: 69.462 ns/op
# Warmup Iteration   3: 72.013 ns/op
# Warmup Iteration   4: 68.641 ns/op
# Warmup Iteration   5: 73.384 ns/op
# Warmup Iteration   6: 69.624 ns/op
# Warmup Iteration   7: 73.325 ns/op
# Warmup Iteration   8: 72.080 ns/op
# Warmup Iteration   9: 73.454 ns/op
# Warmup Iteration  10: 73.144 ns/op
Iteration   1: 66.782 ns/op
Iteration   2: 73.245 ns/op
Iteration   3: 73.835 ns/op
Iteration   4: 67.903 ns/op
Iteration   5: 72.810 ns/op


Result "org.openjdk.bench.java.lang.StringOther.charAt":
  72.279 ±(99.9%) 2.632 ns/op [Average]
  (min, avg, max) = (66.782, 72.279, 74.953), stdev = 2.462
  CI (99.9%): [69.648, 74.911] (assumes normal distribution)


# JMH version: 1.34
# VM version: JDK 19-internal, OpenJDK 64-Bit Server VM, 19-internal-adhoc..jdk
# VM invoker: 
F:\workspace\jdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe
# VM options: 
-Djava.library.path=f:\workspace\jdk\build\windows-x86_64-server-release\images\test\micro\native
# Blackhole mode: compiler (auto-detected, use -Djmh.blackhole.autoDetect=false 
to disable)
# Warmup: 10 iterations, 1000 ms each
# Measurement: 5 iterations, 1000 ms each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Average time, time/op
# Benchmark: org.openjdk.bench.java.lang.StringOther.compareTo

# Run progress: 12.50% complete, ETA 00:05:21
# Fork: 1 of 3
# Warmup Iteration   1: 7.684 ns/op
# Warmup Iteration   2: 7.539 ns/op
# Warmup Iteration   3: 7.072 ns/op
# Warmup Iteration   4: 7.529 ns/op
# Warmup Iteration   5: 7.712 ns/op
# Warmup Iteration   6: 7.514 ns/op
# Warmup Iteration   7: 7.935 ns/op
# Warmup Iteration   8: 7.585 ns/op
# Warmup Iteration   9: 7.511 ns/op
# Warmup Iteration  10: 7.243 ns/op
Iteration   1: 7.202 ns/op
Iteration   2: 7.207 ns/op
Iteration   3: 7.122 ns/op
Iteration   4: 7.140 ns/op
Iteration   5: 7.291 ns/op

# Run progress: 16.67% complete, ETA 00:05:06
# Fork: 2 of 3
# Warmup Iteration   1: 7.665 ns/op
# Warmup Iteration   2: 7.611 ns/op
# Warmup Iteration   3: 6.974 ns/op
# Warmup Iteration   4: 7.627 ns/op
# Warmup Iteration   5: 7.605 ns/op
# Warmup Iteration   6: 7.526 ns/op
# Warmup Iteration   7: 7.535

RFR: 8284548: Invalid XPath expression causes StringIndexOutOfBoundsException

Patch note:

A previous patch had a bug that missed the boundary check, that will cause 
StringIndexOutOfBoundsException to be thrown instead of 
XPathExpressionException as expected.

Fix: the fix is to check the boundaries of the parameter "index". 
Objects.checkIndex is removed as it's redundant.

-

Commit messages:
 - 8284548: Invalid XPath expression causes StringIndexOutOfBoundsException

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

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16

On Tue, 19 Apr 2022 21:15:29 GMT, XenoAmess  wrote:

> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
> https://github.com/openjdk/jdk/pull/8292/
> 
> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> should be changed to 
> 
> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
> c1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> as:
> 
> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
> don't need a lowercase cauculation.


Jmh Result(with = check)


# JMH version: 1.34
# VM version: JDK 19-internal, OpenJDK 64-Bit Server VM, 19-internal-adhoc..jdk
# VM invoker: 
F:\workspace\jdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe
# VM options: 
-Djava.library.path=f:\workspace\jdk\build\windows-x86_64-server-release\images\test\micro\native
# Blackhole mode: compiler (auto-detected, use -Djmh.blackhole.autoDetect=false 
to disable)
# Warmup: 10 iterations, 1000 ms each
# Measurement: 5 iterations, 1000 ms each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Average time, time/op
# Benchmark: org.openjdk.bench.java.lang.StringOther.charAt

# Run progress: 0.00% complete, ETA 00:06:00
# Fork: 1 of 3
# Warmup Iteration   1: 72.769 ns/op
# Warmup Iteration   2: 74.375 ns/op
# Warmup Iteration   3: 72.471 ns/op
# Warmup Iteration   4: 73.477 ns/op
# Warmup Iteration   5: 74.972 ns/op
# Warmup Iteration   6: 73.593 ns/op
# Warmup Iteration   7: 74.270 ns/op
# Warmup Iteration   8: 74.981 ns/op
# Warmup Iteration   9: 74.273 ns/op
# Warmup Iteration  10: 71.556 ns/op
Iteration   1: 73.743 ns/op
Iteration   2: 72.647 ns/op
Iteration   3: 73.701 ns/op
Iteration   4: 74.033 ns/op
Iteration   5: 73.786 ns/op

# Run progress: 4.17% complete, ETA 00:05:52
# Fork: 2 of 3
# Warmup Iteration   1: 75.489 ns/op
# Warmup Iteration   2: 73.533 ns/op
# Warmup Iteration   3: 72.524 ns/op
# Warmup Iteration   4: 73.259 ns/op
# Warmup Iteration   5: 73.346 ns/op
# Warmup Iteration   6: 72.259 ns/op
# Warmup Iteration   7: 73.705 ns/op
# Warmup Iteration   8: 71.332 ns/op
# Warmup Iteration   9: 70.587 ns/op
# Warmup Iteration  10: 69.392 ns/op
Iteration   1: 67.762 ns/op
Iteration   2: 73.029 ns/op
Iteration   3: 69.133 ns/op
Iteration   4: 70.344 ns/op
Iteration   5: 72.041 ns/op

# Run progress: 8.33% complete, ETA 00:05:37
# Fork: 3 of 3
# Warmup Iteration   1: 74.307 ns/op
# Warmup Iteration   2: 74.440 ns/op
# Warmup Iteration   3: 73.053 ns/op
# Warmup Iteration   4: 74.739 ns/op
# Warmup Iteration   5: 75.348 ns/op
# Warmup Iteration   6: 72.801 ns/op
# Warmup Iteration   7: 73.524 ns/op
# Warmup Iteration   8: 73.278 ns/op
# Warmup Iteration   9: 74.633 ns/op
# Warmup Iteration  10: 73.481 ns/op
Iteration   1: 73.787 ns/op
Iteration   2: 73.809 ns/op
Iteration   3: 74.020 ns/op
Iteration   4: 73.751 ns/op
Iteration   5: 73.674 ns/op


Result "org.openjdk.bench.java.lang.StringOther.charAt":
  72.617 ±(99.9%) 2.106 ns/op [Average]
  (min, avg, max) = (67.762, 72.617, 74.033), stdev = 1.970
  CI (99.9%): [70.511, 74.724] (assumes normal distribution)


# JMH version: 1.34
# VM version: JDK 19-internal, OpenJDK 64-Bit Server VM, 19-internal-adhoc..jdk
# VM invoker: 
F:\workspace\jdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe
# VM options: 
-Djava.library.path=f:\workspace\jdk\build\windows-x86_64-server-release\images\test\micro\native
# Blackhole mode: compiler (auto-detected, use -Djmh.blackhole.autoDetect=false 
to disable)
# Warmup: 10 iterations, 1000 ms each
# Measurement: 5 iterations, 1000 ms each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Average time, time/op
# Benchmark: org.openjdk.bench.java.lang.StringOther.compareTo

# Run progress: 12.50% complete, ETA 00:05:21
# Fork: 1 of 3
# Warmup Iteration   1: 7.600 ns/op
# Warmup Iteration   2: 7.552 ns/op
# Warmup Iteration   3: 7.001 ns/op
# Warmup Iteration   4: 7.529 ns/op
# Warmup Iteration   5: 7.541 ns/op
# Warmup Iteration   6: 7.531 ns/op
# Warmup Iteration   7: 7.524 ns/op
# Warmup Iteration   8: 7.507 ns/op
# Warmup Iteration   9: 7.506 ns/op
# Warmup Iteration  10: 7.184 ns/op
Iteration   1: 7.119 ns/op
Iteration   2: 7.111 ns/op
Iteration   3: 7.125 ns/op
Iteration   4: 7.113 ns/op
Iteration   5: 7.111 ns/op

# Run progress: 16.67% complete, ETA 00:05:06
# Fork: 2 of 3
# Warmup Iteration   1: 7.616 ns/op
# Warmup Iteration   2: 7.503 ns/op
# Warmup Iteration   3: 7.013 ns/op
# Warmup Iteration   4: 7.543 ns/op
# Warmup Iteration   5: 7.530 ns/op
# Warmup Iteration   6: 7.489 ns/op
# Warmup Iteration   7: 7.513 ns/op
# Warmup Iteration   8: 7.495 ns/op
# Warmup Iteration   9: 7.493 ns/op
# Warmup Iteration  10: 7.124 ns/op
Iteration   1: 7.156 ns/op
Iteration

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16 [v2]

> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
> https://github.com/openjdk/jdk/pull/8292/
> 
> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> should be changed to 
> 
> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
> c1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> as:
> 
> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
> don't need a lowercase cauculation.

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

  add jmh test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8308/files
  - new: https://git.openjdk.java.net/jdk/pull/8308/files/9371519f..ac6d75e4

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

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

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

Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]

2022-04-20 Thread Joe Darcy

On Wed, 20 Apr 2022 18:40:13 GMT, Alan Bateman  wrote:

> Thanks for the update. I can't think how anything could depend on this but 
> unfortunately it's been like that since early JDK releases. We will need a 
> release note, I'm sure Joe will give his opinion on whether a CSR should be 
> created.

Hmm. It is true that the presence or absence of the "synchronized" modifier is 
not part of a method's contract. However, the multi-thread safety, or not, of a 
class is part of its behavioral contract.

Without a lot of investigation, my current inclination is to run a CSR for this 
change for the (small) behavioral compatibility hazard.

-

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

Re: [External] : Re: jpackage usage problems

2022-04-20 Thread Hiran Chaudhuri

On Wed, 2022-04-20 at 12:38 -0400, Alexey Semenyuk wrote:
>
> On 4/18/2022 7:06 PM, Hiran Chaudhuri wrote:
> > On Mon, 2022-04-18 at 18:41 -0400, Alexey Semenyuk wrote:
> > > I've filed [1] and [2] CRs to track the issues.
> > >
> > > [1] https://bugs.openjdk.java.net/browse/JDK-8284973
> > > [2] https://bugs.openjdk.java.net/browse/JDK-8284974
> > >
> > > - Alexey
> > Sounds great. Thank you.
> >
> > While we are at improving JPackage, there are a few more items I
> > stumbled over:
> >
> > a) When running jpackage on Linux, a freedesktop.org style starter
> > file
> > is created - but one more line in there would make it a lot more
> > usable:
> > https://urldefense.com/v3/__https://stackoverflow.com/questions/70420618/jpackage-linux-creates-insufficient-desktop-file__;!!ACWV5N9M2RV99hQ!bccLoHcm78kdEbI7iOTUvaVrkbLhw5V7ZjrWS68OXiTaKqbHw-dMGrH3tv8DzhG0W4A9$
> >
> > b) The solution is to override resource files. Meanwhile I found
> > out
> > the resource files are templates, and jpackage replaces specific
> > strings in these files. This however is nowhere mentioned in the
> > documentation.
> Must the value of "StartupWMClass" property in .desktop file be
> synchronized with the value used in the app or it can be any unique
> value?
> If the latter, jpackage can create unique value for every launcher
> based
> on package and launcher names.
> Otherwise, if the value must be synchronized with the value used in
> the
> app it must be supplied to jpackage. In this case, overriding
> .desktop
> template is the way to go.

The WMClass of a swing application cannot be configured by the
developer. Yet as much as I saw it is related to the fully qualified
class name that resembles the swing window. So if you use something
like

package org.myproject.mypackage;

public class MyWindow extends JFrame {
...
}

and display an instance of that, it will carry the WMClass parameter as

org-myproject-mypackage-MyWindow

In many cases this class the is the main class at the same time, which
is either specified on the JPackage command line or via the executable
jar's manifest.

I think a reasonable default could be to derive the name from the main
class, with a way to override allowing coverage of exceptional cases.
Most IDEs generate the JFrame classes with a main method. If not
handled you'd force every developer to override resources. If that
becomes the mainstream use case I guess the documentation should be
improved.

>
> > c) Running jpackage in two phases as I do (1: create app-image, 2:
> > create final package) I learned that --name has to be specified in
> > both
> > runs. When running jpackage on MacOS however the second run needs
> > the
> > application name postfixed with .app otherwise jpackage won't find
> > the
> > directory it created in the first run. See the logs
> > https://urldefense.com/v3/__https://github.com/HiranChaudhuri/settlers-installer/runs/6055932278?check_suite_focus=true__;!!ACWV5N9M2RV99hQ!bccLoHcm78kdEbI7iOTUvaVrkbLhw5V7ZjrWS68OXiTaKqbHw-dMGrH3tv8DzplJmV2G$
> > and
> > https://urldefense.com/v3/__https://github.com/HiranChaudhuri/settlers-installer/runs/6055948346?check_suite_focus=true__;!!ACWV5N9M2RV99hQ!bccLoHcm78kdEbI7iOTUvaVrkbLhw5V7ZjrWS68OXiTaKqbHw-dMGrH3tv8Dzmh3le2h$
> --name is optional parameter in both runs. Check out
> https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/test/jdk/tools/jpackage/share/MultiNameTwoPhaseTest.java
> testing this feature.
> I guess you mean "--app-image" parameter, not "--name" parameter.
> ".app"
> suffix in app image name on macOS is a legacy jpackage inherited
> from
> JavaFX's javapackager.
>
> I've filed [1] to track this issue. It may be discarded for the sake
> of
> backward compatibility though.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8285265
>

Oh you are right. I am talking about app-image.

Somehow I'd like to have a more homogenous behaviour across the
platforms. It would allow to keep the build files simple.
This may be difficult to achieve as they impose different requirements.
After all I also stumbled across version numbers on MacOS which are
more restrictive than even on Linux.

Breaking backwards compatibility is a huge pain. Someone will have to
take a decision here.

Thank you for considering. :-)

Hiran

RFR: 8285285: Avoid redundant allocations in WindowsPreferences

2022-04-20 Thread Andrey Turbanov

1. No need to call `String.substring` if you need need to compare start of 
string with some constant. `String.startWith` suites better.
2. Unused String array is allocated in `childrenNamesSpi` method

-

Commit messages:
 - [PATCH] Avoid redundant allocations in WindowsPreferences
 - [PATCH] Avoid redundant String allocation in WindowsPreferences.toJavaName

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

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

Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v8]

> as title.

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

  add more replaces

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8302/files
  - new: https://git.openjdk.java.net/jdk/pull/8302/files/704e895b..363fcc50

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

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

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

Integrated: 8284920: Incorrect Token type causes XPath expression to return incorrect results

On Wed, 20 Apr 2022 17:40:24 GMT, Joe Wang  wrote:

> Patch note:
> 
> The previous patch changed all literal tokens to use constants. However, 
> replacing "." with Token.DOT introduced this bug. 
> While tokens with a single char are inherently of type char, due to the 
> different implementation of the overloaded method "tokenIs" that takes String 
> or char, a wrong input type will produce incorrect result. It may be worth it 
> to take a closer look at the overloaded method, but for now, a quick fix is 
> to reverse the input type back to String (DOT_STR).
> 
> Test: the issue affect the processing of the short form of the parent axis 
> "..". The test verifies that all form of the parent axis shall return the 
> same result.
> 
> Test: all XML tests passed.

This pull request has now been integrated.

Changeset: 81a8e2f8
Author:Joe Wang 
URL:   
https://git.openjdk.java.net/jdk/commit/81a8e2f8b32ad27aed45c4f6966e8d9ecf8b0fc9
Stats: 124 lines in 3 files changed: 122 ins; 0 del; 2 mod

8284920: Incorrect Token type causes XPath expression to return incorrect 
results

Reviewed-by: naoto, lancea

-

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

Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v7]

> as title.

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

  add more replaces

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8302/files
  - new: https://git.openjdk.java.net/jdk/pull/8302/files/53fdb7cd..704e895b

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

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

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

Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v6]

> as title.

XenoAmess has updated the pull request incrementally with two additional 
commits since the last revision:

 - add more replaces
 - add more replaces

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8302/files
  - new: https://git.openjdk.java.net/jdk/pull/8302/files/26bb5792..53fdb7cd

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

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

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

Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner

On Wed, 20 Apr 2022 00:32:25 GMT, Brent Christian  wrote:

> Please review this change to replace the finalizer in 
> `AbstractLdapNamingEnumeration` with a Cleaner.
> 
> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult 
> res`, and `LdapClient enumClnt`) are moved to a static inner class . From 
> there, the change is fairly mechanical.
> 
> Details of note: 
> 1. Some operations need to change the state values (the update() method is 
> probably the most interesting).
> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read 
> `homeCtx` from the superclass's `state`.
> 
> The test case is based on a copy of 
> `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case 
> might be possible, but this was done for expediency.
> 
> The test only confirms that the new Cleaner use does not keep the object 
> reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` 
> or `LdapBindingEnumeration`, though all are subclasses of 
> `AbstractLdapNamingEnumeration`). 
> 
> Thanks.

src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
 line 132:

> 130: this.state = new CleaningAction(homeCtx, answer, 
> homeCtx.clnt);
> 131: // Ensures that context won't get closed from underneath us
> 132: this.state.homeCtx.incEnumCount();

Readability might be improved by using the new `homeCtx()` method instead of 
`state.homeCtx` in this file.
It would then be consistent with how subclasses access it.
It depends which style you prefer to be more consistent with.

-

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

Re: RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner

On Wed, 20 Apr 2022 00:32:25 GMT, Brent Christian  wrote:

> Please review this change to replace the finalizer in 
> `AbstractLdapNamingEnumeration` with a Cleaner.
> 
> The pieces of state required for cleanup (`LdapCtx homeCtx`, `LdapResult 
> res`, and `LdapClient enumClnt`) are moved to a static inner class . From 
> there, the change is fairly mechanical.
> 
> Details of note: 
> 1. Some operations need to change the state values (the update() method is 
> probably the most interesting).
> 2. Subclasses need to access `homeCtx`; I added a `homeCtx()` method to read 
> `homeCtx` from the superclass's `state`.
> 
> The test case is based on a copy of 
> `com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java`. A more minimal test case 
> might be possible, but this was done for expediency.
> 
> The test only confirms that the new Cleaner use does not keep the object 
> reachable. It only tests `LdapSearchEnumeration` (not `LdapNamingEnumeration` 
> or `LdapBindingEnumeration`, though all are subclasses of 
> `AbstractLdapNamingEnumeration`). 
> 
> Thanks.

src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
 line 59:

> 57:  * cleanup, which happens by calling cleanup(), or is done by the 
> Cleaner.
> 58:  */
> 59: private static class CleaningAction implements Runnable {

This nested class might be more aptly named  `EnumCtx` since it is used during 
the enumeration process.

The mention of the `cleanup()` method in the nested class javadoc is a bit 
ambiguous, it seems there should be a cleanup() method in the class, but it is 
in AbstractLdapNamingEnumeration.

The `state` field might also be renamed `enumCtx`.

-

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

Integrated: 8282120: optimal capacity tests and test library need to be cleaned up

On Tue, 19 Apr 2022 18:12:04 GMT, Stuart Marks  wrote:

> The test `java/lang/Enum/ConstantDirectoryOptimalCapacity.java` was 
> problem-listed by 
> [JDK-8281631](https://bugs.openjdk.java.net/browse/JDK-8281631) and 
> essentially superseded by 
> [JDK-8186958](https://bugs.openjdk.java.net/browse/JDK-8186958). Given this, 
> the test should simply be removed. This test is the only test that depends on 
> a test library utility `test/lib/jdk/test/lib/util/OptimalCapacity.java` 
> which has a number of issues of its own, so it should simply be removed as 
> well.
> 
> See the bug report, in particular the comment
> 
> https://bugs.openjdk.java.net/browse/JDK-8282120?focusedCommentId=14489450=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14489450
> 
> for detailed discussion.

This pull request has now been integrated.

Changeset: b2c33f0f
Author:Stuart Marks 
URL:   
https://git.openjdk.java.net/jdk/commit/b2c33f0f86174f5a8cf2229a3f766a2a8cff9d27
Stats: 355 lines in 3 files changed: 0 ins; 355 del; 0 mod

8282120: optimal capacity tests and test library need to be cleaned up

Reviewed-by: naoto

-

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

Re: RFR: 8284920: Incorrect Token type causes XPath expression to return incorrect results [v2]

2022-04-20 Thread Lance Andersen

On Wed, 20 Apr 2022 18:28:19 GMT, Joe Wang  wrote:

>> Patch note:
>> 
>> The previous patch changed all literal tokens to use constants. However, 
>> replacing "." with Token.DOT introduced this bug. 
>> While tokens with a single char are inherently of type char, due to the 
>> different implementation of the overloaded method "tokenIs" that takes 
>> String or char, a wrong input type will produce incorrect result. It may be 
>> worth it to take a closer look at the overloaded method, but for now, a 
>> quick fix is to reverse the input type back to String (DOT_STR).
>> 
>> Test: the issue affect the processing of the short form of the parent axis 
>> "..". The test verifies that all form of the parent axis shall return the 
>> same result.
>> 
>> Test: all XML tests passed.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add test cases for the self axis

Looks good to me Joe

-

Marked as reviewed by lancea (Reviewer).

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

Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v2]

2022-04-20 Thread Severin Gehwolf

On Wed, 20 Apr 2022 18:18:25 GMT, xpbob  wrote:

>> set memory.swappiness to 0,swap space will not be used 
>> determine the value of memory.swappiness
>> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
>> 
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 100.00M
>> Maximum Processes Limit: 4194305 
>> 
>> =>
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 50.00M
>> Maximum Processes Limit: 4194305
>
> xpbob has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   add test and hotspot data

Changes requested by sgehwolf (Reviewer).

test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java line 164:

> 162: opts.addDockerOpts("--memory-swappiness", "0");
> 163: } else {
> 164: opts.addDockerOpts("--memory-swappiness", "60");

Unfortunately this breaks on a cgroups v2 system as `--memory-swappiness` is 
not supported there. I'd prefer if this wouldn't piggy back on the existing 
test, but actually assert that swap is properly reported as the same as the 
memory limit if `--memory-swappiness=0`. Also, this test only verifies the Java 
(core-libs) change, not the hotspot change. That would have to be done via some 
`TestMisc` variant which uses `print_container_info()`.

-

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

Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]

2022-04-20 Thread Alan Bateman

On Wed, 20 Apr 2022 15:33:26 GMT, Brian Burkhalter  wrote:

>> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods 
>> of `java.io.FilterInputStream`.
>
> Brian Burkhalter has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8284930: Also remove synchronized keyword from mark() and reset() of 
> InputStream

Thanks for the update. I can't think how anything could depend on this but 
unfortunately it's been like that since early JDK releases. We will need a 
release note, I'm sure Joe will give his opinion on whether a CSR should be 
created.

-

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

Re: RFR: 8284920: Incorrect Token type causes XPath expression to return incorrect results [v2]

2022-04-20 Thread Naoto Sato

On Wed, 20 Apr 2022 18:28:19 GMT, Joe Wang  wrote:

>> Patch note:
>> 
>> The previous patch changed all literal tokens to use constants. However, 
>> replacing "." with Token.DOT introduced this bug. 
>> While tokens with a single char are inherently of type char, due to the 
>> different implementation of the overloaded method "tokenIs" that takes 
>> String or char, a wrong input type will produce incorrect result. It may be 
>> worth it to take a closer look at the overloaded method, but for now, a 
>> quick fix is to reverse the input type back to String (DOT_STR).
>> 
>> Test: the issue affect the processing of the short form of the parent axis 
>> "..". The test verifies that all form of the parent axis shall return the 
>> same result.
>> 
>> Test: all XML tests passed.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add test cases for the self axis

Marked as reviewed by naoto (Reviewer).

-

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

Re: RFR: 8284920: Incorrect Token type causes XPath expression to return incorrect results [v2]

> Patch note:
> 
> The previous patch changed all literal tokens to use constants. However, 
> replacing "." with Token.DOT introduced this bug. 
> While tokens with a single char are inherently of type char, due to the 
> different implementation of the overloaded method "tokenIs" that takes String 
> or char, a wrong input type will produce incorrect result. It may be worth it 
> to take a closer look at the overloaded method, but for now, a quick fix is 
> to reverse the input type back to String (DOT_STR).
> 
> Test: the issue affect the processing of the short form of the parent axis 
> "..". The test verifies that all form of the parent axis shall return the 
> same result.
> 
> Test: all XML tests passed.

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

  add test cases for the self axis

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8321/files
  - new: https://git.openjdk.java.net/jdk/pull/8321/files/cb5e5ec3..3b61789e

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

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

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

Re: RFR: 8284950: Swappiness disables swap space usage [v2]

2022-04-20 Thread xpbob

> set memory.swappiness to 0,swap space will not be used 
> determine the value of memory.swappiness
> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
> 
> 
> Memory Limit: 50.00M
> Memory Soft Limit: Unlimited
> Memory & Swap Limit: 100.00M
> Maximum Processes Limit: 4194305 
> 
> =>
> 
> Memory Limit: 50.00M
> Memory Soft Limit: Unlimited
> Memory & Swap Limit: 50.00M
> Maximum Processes Limit: 4194305

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

  add test and hotspot data

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8285/files
  - new: https://git.openjdk.java.net/jdk/pull/8285/files/52cd6411..cda7f85b

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

  Stats: 23 lines in 2 files changed: 19 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8285.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8285/head:pull/8285

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

RFR: 8284920: Incorrect Token type causes XPath expression to return incorrect results

Patch note:

The previous patch changed all literal tokens to use constants. However, 
replacing "." with Token.DOT introduced this bug. 
While tokens with a single char are inherently of type char, due to the 
different implementation of the overloaded method "tokenIs" that takes String 
or char, a wrong input type will produce incorrect result. It may be worth it 
to take a closer look at the overloaded method, but for now, a quick fix is to 
reverse the input type back to String (DOT_STR).

Test: the issue affect the processing of the short form of the parent axis 
"..". The test verifies that all form of the parent axis shall return the same 
result.

Test: all XML tests passed.

-

Commit messages:
 - 8284920: Incorrect Token type causes XPath expression to return incorrect 
results

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

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16

On Tue, 19 Apr 2022 21:15:29 GMT, XenoAmess  wrote:

> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
> https://github.com/openjdk/jdk/pull/8292/
> 
> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> should be changed to 
> 
> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
> c1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> as:
> 
> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
> don't need a lowercase cauculation.

The test should be able to use public string classes with selected Latin1 and 
utf-16 arguments.
The implementation of regionMatches will end up using the corresponding 
implementations.
The JMH in PR#8292 might be a start.

-

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16

On Wed, 20 Apr 2022 16:50:06 GMT, Roger Riggs  wrote:

> Point 1 make sense. Point 2 adds a branch and branches aren't free. Can you 
> show some jmh data for these cases?

@RogerRiggs

Bad news is I don't know how to make a jmh benchmark for a not-public class...

-

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

Re: RFR: 8284992: Fix misleading Vector API doc for LSHR operator

2022-04-20 Thread Paul Sandoz

On Tue, 19 Apr 2022 08:41:50 GMT, Jie Fu  wrote:

> Hi all,
> 
> The Current Vector API doc for `LSHR` is
> 
> Produce a>>>(n&(ESIZE*8-1)). Integral only.
> 
> 
> This is misleading which may lead to bugs for Java developers.
> This is because for negative byte/short elements, the results computed by 
> `LSHR` will be different from that of `>>>`.
> For more details, please see 
> https://github.com/openjdk/jdk/pull/8276#issue-1206391831 .
> 
> After the patch, the doc for `LSHR` is
> 
> Produce zero-extended right shift of a by (n&(ESIZE*8-1)) bits. Integral only.
> 
> 
> Thanks.
> Best regards,
> Jie

We can raise attention to that:

/** Produce {@code a>>>(n&(ESIZE*8-1))} 
  * (The operand and result are converted if the operand type is {@code byte} 
or {@code short}, see below).  Integral only.
  * ...
  */

-

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

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v5]

2022-04-20 Thread Vicente Romero

On Tue, 19 Apr 2022 18:47:16 GMT, Jan Lahoda  wrote:

>> This is a (preliminary) patch for javac implementation for the third preview 
>> of pattern matching for switch (type patterns in switches).
>> 
>> Draft JLS:
>> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html
>> 
>> The changes are:
>> -there are no guarded patterns anymore, guards are not bound to the 
>> CaseElement (JLS 15.28)
>> -a new contextual keyword `when` is used to add a guard, instead of `&&`
>> -`null` selector value is handled on switch level (if a switch has `case 
>> null`, it is used, otherwise a NPE is thrown), rather than on pattern 
>> matching level.
>> -total patterns are allowed in `instanceof`
>> -`java.lang.MatchException` is added for the case where a switch is 
>> exhaustive (due to sealed types) at compile-time, but not at runtime.
>> 
>> Feedback is welcome!
>> 
>> Thanks!
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Cleanup - more total -> unconditional pattern renaming.

I don't have any other comments, looks good

-

Marked as reviewed by vromero (Reviewer).

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

Integrated: JDK-8281006 Module::getResourceAsStream should check if the resource is open unconditionally when caller is null

2022-04-20 Thread Tim Prinzing

On Thu, 7 Apr 2022 00:38:07 GMT, Tim Prinzing  wrote:

> Created a test called NullCallerGetResource to test 
> Module::getResourceAsStream and Class::getResourceAsStream from the native 
> level.
> 
> At the java level the test builds a test module called 'n' which opens the 
> package 'open' to everyone. There is also a package 'closed' which is neither 
> opened or exported. Both packages have a text file called 'test.txt'. The 
> open package has a class called OpenResources and the closed package has a 
> class called ClosedResources. The native test is launched with the test 
> module n added.
> 
> At the native level the test tries to read both the open and closed resource 
> from both the classes and the module. It performs the equivalent of the 
> following java operations:
> 
> Class c = open.OpenResources.fetchClass();
> InputStream in = c.getResourceAsStream("test.txt");
> assert(in != null); in.close();
> 
> Module n = c.getModule();
> in = n.getResourceAsStream("open/test.txt");
> assert(in != null); in.close();
> 
> Class closed = closed.ClosedResources.fetchClass();
> assert(closedsStream("test.txt") == null);
> assert(n.getResourceAsStream("closed/test.txt") == null);
> 
> The test initially threw an NPE when trying to fetch the open resource. The 
> Module class was fixed by removing the fragment with the (caller == null) 
> test in getResourceAsStream, and changing the call to isOpen(String,Module) 
> to use EVERYONE_MODULE if the caller module is null.

This pull request has now been integrated.

Changeset: e8016f74
Author:Tim Prinzing 
Committer: Mandy Chung 
URL:   
https://git.openjdk.java.net/jdk/commit/e8016f74438ca5c64a8aab81e2fc2533e9b9f8ad
Stats: 393 lines in 8 files changed: 385 ins; 0 del; 8 mod

8281006: Module::getResourceAsStream should check if the resource is open 
unconditionally when caller is null

Reviewed-by: alanb, erikj, mchung

-

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

Re: RFR: JDK-8281006 Module::getResourceAsStream should check if the resource is open unconditionally when caller is null [v5]

2022-04-20 Thread Mandy Chung

On Wed, 20 Apr 2022 01:03:23 GMT, Tim Prinzing  wrote:

>> Created a test called NullCallerGetResource to test 
>> Module::getResourceAsStream and Class::getResourceAsStream from the native 
>> level.
>> 
>> At the java level the test builds a test module called 'n' which opens the 
>> package 'open' to everyone. There is also a package 'closed' which is 
>> neither opened or exported. Both packages have a text file called 
>> 'test.txt'. The open package has a class called OpenResources and the closed 
>> package has a class called ClosedResources. The native test is launched with 
>> the test module n added.
>> 
>> At the native level the test tries to read both the open and closed resource 
>> from both the classes and the module. It performs the equivalent of the 
>> following java operations:
>> 
>> Class c = open.OpenResources.fetchClass();
>> InputStream in = c.getResourceAsStream("test.txt");
>> assert(in != null); in.close();
>> 
>> Module n = c.getModule();
>> in = n.getResourceAsStream("open/test.txt");
>> assert(in != null); in.close();
>> 
>> Class closed = closed.ClosedResources.fetchClass();
>> assert(closedsStream("test.txt") == null);
>> assert(n.getResourceAsStream("closed/test.txt") == null);
>> 
>> The test initially threw an NPE when trying to fetch the open resource. The 
>> Module class was fixed by removing the fragment with the (caller == null) 
>> test in getResourceAsStream, and changing the call to isOpen(String,Module) 
>> to use EVERYONE_MODULE if the caller module is null.
>
> Tim Prinzing has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   some cleanup of the test

Thanks for the test update.  Looks good.

-

Marked as reviewed by mchung (Reviewer).

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

Re: RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16

On Tue, 19 Apr 2022 21:15:29 GMT, XenoAmess  wrote:

> some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
> https://github.com/openjdk/jdk/pull/8292/
> 
> if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> should be changed to 
> 
> if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
> c1) == Character.toLowerCase(u2)) {
> continue;
> }
> 
> as:
> 
> 1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
> 2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
> don't need a lowercase cauculation.

Point 1 make sense.
Point 2 adds a branch and branches aren't free.
Can you show some jmh data for these cases?

-

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v12]

> @jmehrens what about this then?
> I think it safe now(actually this mechanism is learned from Reader)

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

  add documents

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5872/files
  - new: https://git.openjdk.java.net/jdk/pull/5872/files/47d8e195..fbc24743

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=11
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=10-11

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

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v9]

On Wed, 20 Apr 2022 16:35:25 GMT, liach  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   remove uselsee MIN_SKIP_BUFFER_SIZE
>
> src/java.base/share/classes/java/io/InputStream.java line 62:
> 
>> 60: 
>> 61: private byte[] skipBufferReference(long remaining) {
>> 62: int size = (int) Math.min(MAX_SKIP_BUFFER_SIZE, remaining);
> 
> This is now redundant. Can change parameter to (int size) directly

@liach done.

-

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v11]

> @jmehrens what about this then?
> I think it safe now(actually this mechanism is learned from Reader)

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

  skipBufferReference(long) -> skipBufferReference(int)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5872/files
  - new: https://git.openjdk.java.net/jdk/pull/5872/files/878deebf..47d8e195

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=10
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=09-10

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

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v9]

On Wed, 20 Apr 2022 16:30:19 GMT, XenoAmess  wrote:

>> @jmehrens what about this then?
>> I think it safe now(actually this mechanism is learned from Reader)
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove uselsee MIN_SKIP_BUFFER_SIZE

src/java.base/share/classes/java/io/InputStream.java line 62:

> 60: 
> 61: private byte[] skipBufferReference(long remaining) {
> 62: int size = (int) Math.min(MAX_SKIP_BUFFER_SIZE, remaining);

This is now redundant. Can change parameter to (int size) directly

-

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v7]

On Tue, 19 Apr 2022 22:37:21 GMT, jmehrens  wrote:

>>> > @jmehrens what about this then? I think it safe now(actually this 
>>> > mechanism is learned from Reader)
>>> 
>>> Reader uses a lock object and it appears that InputStream locks on this 
>>> (per make/reset) I would assume now that you have some object member fields 
>>> you need to hold some lock while accessing those. Even though single thread 
>>> access would be the expected case.
>> 
>> no, we use other way, but yes for we take care of thread safety.
>> 
>>> Not related but, it looks like the static buffer issue has come up a few 
>>> times. Maybe time to add a test to: 
>>> https://github.com/openjdk/jdk/blob/master/test/jdk/java/io/InputStream/Skip.java
>>>  that would fail if the skip buffer is static?
>> 
>> I think it is worthy to add some comments, but a test for it sound a little 
>> bit extreme.
>> 
>>> Not really the primary issue but, it seems questionable to me that we don't 
>>> have to fill the skip buffer with zeros after the last read. That way any 
>>> sensitive information that was skipped would also be forgotten. Matching 
>>> with Reader seems more important for now.
>> 
>> Don't think it necessary... I think making it cannot touched by other object 
>> (with security manager on) is enough.
>
>> no, we use other way, but yes for we take care of thread safety.
> 
> Fair enough.
> 
>> Don't think it necessary... I think making it cannot touched by other object 
>> (with security manager on) is enough.
> 
> I was thinking more for heapdumps due to extending the life of the skip 
> buffer in this patch.  At least we don't have to waste more cycles.
> 
> Oh I forgot last time but, it looks like MIN_SKIP_BUFFER_SIZE is not used.  
> Unless I'm missing something I assume that is a bug.

@jmehrens comments about not making it static added.

-

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v10]

> @jmehrens what about this then?
> I think it safe now(actually this mechanism is learned from Reader)

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

  comment added.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5872/files
  - new: https://git.openjdk.java.net/jdk/pull/5872/files/696aa53b..878deebf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=09
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=08-09

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

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

Re: [External] : Re: jpackage usage problems

2022-04-20 Thread Alexey Semenyuk

On 4/18/2022 7:06 PM, Hiran Chaudhuri wrote:

On Mon, 2022-04-18 at 18:41 -0400, Alexey Semenyuk wrote:

I've filed [1] and [2] CRs to track the issues.

[1] https://bugs.openjdk.java.net/browse/JDK-8284973
[2] https://bugs.openjdk.java.net/browse/JDK-8284974

- Alexey

Sounds great. Thank you.

While we are at improving JPackage, there are a few more items I
stumbled over:

a) When running jpackage on Linux, a freedesktop.org style starter file
is created - but one more line in there would make it a lot more
usable:
https://urldefense.com/v3/__https://stackoverflow.com/questions/70420618/jpackage-linux-creates-insufficient-desktop-file__;!!ACWV5N9M2RV99hQ!bccLoHcm78kdEbI7iOTUvaVrkbLhw5V7ZjrWS68OXiTaKqbHw-dMGrH3tv8DzhG0W4A9$

b) The solution is to override resource files. Meanwhile I found out
the resource files are templates, and jpackage replaces specific
strings in these files. This however is nowhere mentioned in the
documentation.
Must the value of "StartupWMClass" property in .desktop file be
synchronized with the value used in the app or it can be any unique value?
If the latter, jpackage can create unique value for every launcher based
on package and launcher names.
Otherwise, if the value must be synchronized with the value used in the
app it must be supplied to jpackage. In this case, overriding .desktop
template is the way to go.

c) Running jpackage in two phases as I do (1: create app-image, 2:
create final package) I learned that --name has to be specified in both
runs. When running jpackage on MacOS however the second run needs the
application name postfixed with .app otherwise jpackage won't find the
directory it created in the first run. See the logs
https://urldefense.com/v3/__https://github.com/HiranChaudhuri/settlers-installer/runs/6055932278?check_suite_focus=true__;!!ACWV5N9M2RV99hQ!bccLoHcm78kdEbI7iOTUvaVrkbLhw5V7ZjrWS68OXiTaKqbHw-dMGrH3tv8DzplJmV2G$
and
https://urldefense.com/v3/__https://github.com/HiranChaudhuri/settlers-installer/runs/6055948346?check_suite_focus=true__;!!ACWV5N9M2RV99hQ!bccLoHcm78kdEbI7iOTUvaVrkbLhw5V7ZjrWS68OXiTaKqbHw-dMGrH3tv8Dzmh3le2h$
--name is optional parameter in both runs. Check out
https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/test/jdk/tools/jpackage/share/MultiNameTwoPhaseTest.java
testing this feature.
I guess you mean "--app-image" parameter, not "--name" parameter. ".app"
suffix in app image name on macOS is a legacy jpackage inherited from
JavaFX's javapackager.

I've filed [1] to track this issue. It may be discarded for the sake of
backward compatibility though.

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

- Alexey

Hiran

Re: RFR: 8284638: store skip buffers in InputStream Object [v8]

On Wed, 20 Apr 2022 16:16:05 GMT, liach  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   remove useless skipBuffer
>
> src/java.base/share/classes/java/io/InputStream.java line 57:
> 
>> 55: private static final int MAX_SKIP_BUFFER_SIZE = 2048;
>> 56: 
>> 57: private static final int MIN_SKIP_BUFFER_SIZE = 128;
> 
> @jmehrens mentioned that this one can be removed as well since it's not used.

@jmehrens @liach you are correct. done.

-

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v9]

> @jmehrens what about this then?
> I think it safe now(actually this mechanism is learned from Reader)

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

  remove uselsee MIN_SKIP_BUFFER_SIZE

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5872/files
  - new: https://git.openjdk.java.net/jdk/pull/5872/files/31652c3a..696aa53b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5872=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5872=07-08

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

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

Integrated: JDK-8280594: Refactor annotation invocation handler handling to use Objects.toIdentityString

2022-04-20 Thread Joe Darcy

On Tue, 19 Apr 2022 23:34:01 GMT, Joe Darcy  wrote:

> Simple refactoring to use new-in19 library code.

This pull request has now been integrated.

Changeset: e6c5f288
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/e6c5f2886c39a95e660dd3d83d894fd3761b7468
Stats: 10 lines in 1 file changed: 0 ins; 9 del; 1 mod

8280594: Refactor annotation invocation handler handling to use 
Objects.toIdentityString

Reviewed-by: bpb

-

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v8]

On Wed, 20 Apr 2022 16:07:17 GMT, XenoAmess  wrote:

>> @jmehrens what about this then?
>> I think it safe now(actually this mechanism is learned from Reader)
>
> XenoAmess has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove useless skipBuffer

src/java.base/share/classes/java/io/InputStream.java line 57:

> 55: private static final int MAX_SKIP_BUFFER_SIZE = 2048;
> 56: 
> 57: private static final int MIN_SKIP_BUFFER_SIZE = 128;

@jmehrens mentioned that this one can be removed as well since it's not used.

-

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

Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v4]

On Tue, 19 Apr 2022 22:17:54 GMT, liach  wrote:

> Need to add apiNote documentation section to capacity-based constructors like 
> for maps.

@liach done.

-

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

Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v5]

> as title.

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

  add apiNote s

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8302/files
  - new: https://git.openjdk.java.net/jdk/pull/8302/files/f051c1fa..26bb5792

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

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

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

Re: RFR: JDK-8280594: Refactor annotation invocation handler handling to use Objects.toIdentityString

2022-04-20 Thread Brian Burkhalter

On Tue, 19 Apr 2022 23:34:01 GMT, Joe Darcy  wrote:

> Simple refactoring to use new-in19 library code.

Marked as reviewed by bpb (Reviewer).

-

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

Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v4]

On Tue, 19 Apr 2022 20:15:08 GMT, XenoAmess  wrote:

>> as title.
>
> XenoAmess 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 two new 
> commits since the last revision:
> 
>  - migrate HashSet usages
>  - migrate LinkedHashSet usage

> /csr needed

@stuart-marks csr please

-

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v8]

> @jmehrens what about this then?
> I think it safe now(actually this mechanism is learned from Reader)

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

  remove useless skipBuffer

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5872/files
  - new: https://git.openjdk.java.net/jdk/pull/5872/files/9854f523..31652c3a

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

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

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

Re: RFR: 8284638: store skip buffers in InputStream Object [v7]

On Tue, 19 Apr 2022 22:47:42 GMT, liach  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   change to liach operation.
>
> src/java.base/share/classes/java/io/InputStream.java line 62:
> 
>> 60: 
>> 61: /** Skip buffer, null until allocated */
>> 62: private byte[] skipBuffer = null;
> 
> This can be removed too as you have the soft reference already.

@liach done.

-

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

RFR: 8285255: refine StringLatin1.regionMatchesCI_UTF16

some thoughts after watching 8285001: Simplify StringLatin1.regionMatches  
https://github.com/openjdk/jdk/pull/8292/

if (Character.toLowerCase(u1) == Character.toLowerCase(u2)) {
continue;
}

should be changed to 

if (((u1 == c1) ? CharacterDataLatin1.instance.toLowerCase(c1) : 
c1) == Character.toLowerCase(u2)) {
continue;
}

as:

1. c1 is LATIN1, so CharacterDataLatin1.instance.toLowerCase seems faster.
2. because c1 is LATIN1, so if u1 != c1, then c1 is already lowercase, and 
don't need a lowercase cauculation.

-

Commit messages:
 - 9073087: refine StringLatin1.regionMatchesCI_UTF16

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

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

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v5]

2022-04-20 Thread Vicente Romero

On Tue, 19 Apr 2022 18:47:16 GMT, Jan Lahoda  wrote:

>> This is a (preliminary) patch for javac implementation for the third preview 
>> of pattern matching for switch (type patterns in switches).
>> 
>> Draft JLS:
>> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html
>> 
>> The changes are:
>> -there are no guarded patterns anymore, guards are not bound to the 
>> CaseElement (JLS 15.28)
>> -a new contextual keyword `when` is used to add a guard, instead of `&&`
>> -`null` selector value is handled on switch level (if a switch has `case 
>> null`, it is used, otherwise a NPE is thrown), rather than on pattern 
>> matching level.
>> -total patterns are allowed in `instanceof`
>> -`java.lang.MatchException` is added for the case where a switch is 
>> exhaustive (due to sealed types) at compile-time, but not at runtime.
>> 
>> Feedback is welcome!
>> 
>> Thanks!
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Cleanup - more total -> unconditional pattern renaming.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeMaker.java line 2:

> 1: /*
> 2:  * Copyright (c) 1999, 20222, Oracle and/or its affiliates. All rights 
> reserved.

typo: `20222` instead of `2022`

-

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

Re: RFR: 8284930: Re-examine FilterInputStream mark/reset [v2]

2022-04-20 Thread Brian Burkhalter

> Remove the `synchronized` keyword from the `mark(int)` and `reset()` methods 
> of `java.io.FilterInputStream`.

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

  8284930: Also remove synchronized keyword from mark() and reset() of 
InputStream

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8309/files
  - new: https://git.openjdk.java.net/jdk/pull/8309/files/85546c9e..f210cbf5

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

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

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

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v5]

2022-04-20 Thread Vicente Romero

On Tue, 19 Apr 2022 18:47:16 GMT, Jan Lahoda  wrote:

>> This is a (preliminary) patch for javac implementation for the third preview 
>> of pattern matching for switch (type patterns in switches).
>> 
>> Draft JLS:
>> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html
>> 
>> The changes are:
>> -there are no guarded patterns anymore, guards are not bound to the 
>> CaseElement (JLS 15.28)
>> -a new contextual keyword `when` is used to add a guard, instead of `&&`
>> -`null` selector value is handled on switch level (if a switch has `case 
>> null`, it is used, otherwise a NPE is thrown), rather than on pattern 
>> matching level.
>> -total patterns are allowed in `instanceof`
>> -`java.lang.MatchException` is added for the case where a switch is 
>> exhaustive (due to sealed types) at compile-time, but not at runtime.
>> 
>> Feedback is welcome!
>> 
>> Thanks!
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Cleanup - more total -> unconditional pattern renaming.

nit: just ran langtools tests as part of my routine and these seem to be 
failing, CheckExamples.java due to:
test/langtools/tools/javac/diags/examples/FeatureTotalPatternsInInstanceof.java 
and
test/langtools/tools/javac/diags/examples/FeatureTotalPatternsInInstanceof.java

and this one too:
test/langtools/tools/javac/patterns/InstanceofTotalPattern.java (seems to be a 
golden file issue)

-

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

Re: RFR: 8284853: Fix various 'expected' typo [v2]

2022-04-20 Thread Kevin Walls

On Thu, 14 Apr 2022 18:04:16 GMT, Andrey Turbanov  wrote:

> I found [yet another 
> typo](https://github.com/kelthuzadx/jdk/commit/acb9e15bc0bf5395d1c0875f36992f692734f948)
>  ...

I didn't think "JVMInvokeMethodSlack" was a typo.  I think it's the idea of 
"slack space" meaning leftover space.  We require a certain amount of this 
space.  We need some slack on the stack, in order to invoke.

-

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

Re: RFR: 8284930: Re-examine FilterInputStream mark/reset