Re: RFR: 8275721: Name of UTC timezone in a locale changes depending on previous code [v2]

2021-12-03 Thread Naoto Sato
> Fixing time zone name provider for CLDR. In some cases, COMPAT's `UTC` 
> display names were incorrectly substituted for CLDR. The reason it worked 
> fine after `zh-Hant-HK` was that by loading names for `zh-Hant-HK`, the names 
> for `zh-Hant` were cached and hit for the following `zh-MO` name retrieval.

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

  Fixed COMPAT substitution for FULL names and modified the priority.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6709/files
  - new: https://git.openjdk.java.net/jdk/pull/6709/files/10957100..3ec08e4d

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

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

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


Re: RFR: 8003417: WeakHashMap$HashIterator removes wrong entry

2021-12-03 Thread Stuart Marks
On Sat, 20 Nov 2021 10:08:41 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which proposes to fix the issue 
> reported in https://bugs.openjdk.java.net/browse/JDK-8003417?
> 
> The issue notes that this is applicable for `WeakHashMap` which have `null` 
> keys. However, the issue is even applicable for `WeakHashMap` instances which 
> don't have `null` keys, as reproduced and shown by the newly added jtreg test 
> case in this PR.
> 
> The root cause of the issue is that once the iterator is used to iterate till 
> the end and the `remove()` is called, then the 
> `WeakHashMap$HashIterator#remove()` implementation used to pass `null` as the 
> key to remove from the map, instead of the key of the last returned entry. 
> The commit in this PR fixes that part.
> 
> A new jtreg test has been added which reproduces the issue as well as 
> verifies the fix.
> `tier1` testing and this new test have passed after this change. However, I 
> guess this will require a JCK run to be run too, perhaps? If so, I will need 
> help from someone who has access to them to have this run against those 
> please.

I've put extensive comments and explanation into the bug report. I put them 
there instead of here because they're more closely related to the test case in 
that bug report, and they also include general statements about WeakHashMap 
iteration, not specific to the change proposed here.

TL;DR I don't think that changing remove to `remove(lastReturned.get())` is the 
correct fix. It requires revisting the invariants maintained by `HashIterator`. 
It might be as simple as removing the clearing of `currentKey` from 
`hasNext()`, as mentioned in the bug report, but this requires more thought.

-

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


Re: RFR: 8003417: WeakHashMap$HashIterator removes wrong entry

2021-12-03 Thread Stuart Marks
On Sat, 4 Dec 2021 00:02:04 GMT, Brent Christian  wrote:

>> Can I please get a review for this change which proposes to fix the issue 
>> reported in https://bugs.openjdk.java.net/browse/JDK-8003417?
>> 
>> The issue notes that this is applicable for `WeakHashMap` which have `null` 
>> keys. However, the issue is even applicable for `WeakHashMap` instances 
>> which don't have `null` keys, as reproduced and shown by the newly added 
>> jtreg test case in this PR.
>> 
>> The root cause of the issue is that once the iterator is used to iterate 
>> till the end and the `remove()` is called, then the 
>> `WeakHashMap$HashIterator#remove()` implementation used to pass `null` as 
>> the key to remove from the map, instead of the key of the last returned 
>> entry. The commit in this PR fixes that part.
>> 
>> A new jtreg test has been added which reproduces the issue as well as 
>> verifies the fix.
>> `tier1` testing and this new test have passed after this change. However, I 
>> guess this will require a JCK run to be run too, perhaps? If so, I will need 
>> help from someone who has access to them to have this run against those 
>> please.
>
> src/java.base/share/classes/java/util/WeakHashMap.java line 826:
> 
>> 824: throw new ConcurrentModificationException();
>> 825: 
>> 826: WeakHashMap.this.remove(lastReturned.get());
> 
> Unlike `currentKey`, `HashIterator` does not necessarily maintain a strong 
> reference to the referent of `lastReturned`. If the `lastReturned` 
> WeakReference were cleared between `lastReturned` being set, and the 
> `remove()` call, the null key could again be erroneously removed. It would be 
> cool to add a test case for this (maybe using large objects as keys would 
> tempt the GC to clear the weakrefs via `jdk.test.lib.util.ForceGC`).
> 
> Since we store the `NULL_KEY` sentinel value for the null key, I believe 
> `Entry.get()` will only return null in the case that the weakref has been 
> cleared.  So could we instead fix this with:
> 
> 
> if (lastReturned.get() != null) {
> WeakHashMap.this.remove(lastReturned.get());
> }
> 
> 
> ?
> Or, if we're worried about the ref being cleared between the null check and 
> remove(), maybe:
> 
> Object lastReturnedKey = lastReturned.get();
> if (lastReturnedKey != null) {
> WeakHashMap.this.remove(lastReturned.get());
> }

Yes, this is my concern too; lastReturned is a weak ref, and it is possible for 
it to have been cleared at this point, so get() can return null. I will post a 
more complete explanation.

-

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


RFR: 8275721: Name of UTC timezone in a locale changes depending on previous code

2021-12-03 Thread Naoto Sato
Fixing time zone name provider for CLDR. In some cases, COMPAT's `UTC` display 
names were incorrectly substituted for CLDR. The reason it worked fine after 
`zh-Hant-HK` was that by loading names for `zh-Hant-HK`, the names for 
`zh-Hant` were cached and hit for the following `zh-MO` name retrieval.

-

Commit messages:
 - 8275721: Name of UTC timezone in a locale changes depending on previous code

Changes: https://git.openjdk.java.net/jdk/pull/6709/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6709=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8275721
  Stats: 87 lines in 2 files changed: 73 ins; 6 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6709.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6709/head:pull/6709

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


Re: RFR: 8003417: WeakHashMap$HashIterator removes wrong entry

2021-12-03 Thread Brent Christian
On Sat, 20 Nov 2021 10:08:41 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which proposes to fix the issue 
> reported in https://bugs.openjdk.java.net/browse/JDK-8003417?
> 
> The issue notes that this is applicable for `WeakHashMap` which have `null` 
> keys. However, the issue is even applicable for `WeakHashMap` instances which 
> don't have `null` keys, as reproduced and shown by the newly added jtreg test 
> case in this PR.
> 
> The root cause of the issue is that once the iterator is used to iterate till 
> the end and the `remove()` is called, then the 
> `WeakHashMap$HashIterator#remove()` implementation used to pass `null` as the 
> key to remove from the map, instead of the key of the last returned entry. 
> The commit in this PR fixes that part.
> 
> A new jtreg test has been added which reproduces the issue as well as 
> verifies the fix.
> `tier1` testing and this new test have passed after this change. However, I 
> guess this will require a JCK run to be run too, perhaps? If so, I will need 
> help from someone who has access to them to have this run against those 
> please.

Changes requested by bchristi (Reviewer).

src/java.base/share/classes/java/util/WeakHashMap.java line 826:

> 824: throw new ConcurrentModificationException();
> 825: 
> 826: WeakHashMap.this.remove(lastReturned.get());

Unlike `currentKey`, `HashIterator` does not necessarily maintain a strong 
reference to the referent of `lastReturned`. If the `lastReturned` 
WeakReference were cleared between `lastReturned` being set, and the `remove()` 
call, the null key could again be erroneously removed. It would be cool to add 
a test case for this (maybe using large objects as keys would tempt the GC to 
clear the weakrefs via `jdk.test.lib.util.ForceGC`).

Since we store the `NULL_KEY` sentinel value for the null key, I believe 
`Entry.get()` will only return null in the case that the weakref has been 
cleared.  So could we instead fix this with:


if (lastReturned.get() != null) {
WeakHashMap.this.remove(lastReturned.get());
}


?
Or, if we're worried about the ref being cleared between the null check and 
remove(), maybe:

Object lastReturnedKey = lastReturned.get();
if (lastReturnedKey != null) {
WeakHashMap.this.remove(lastReturned.get());
}

-

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


Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation [v2]

2021-12-03 Thread Rajan Halade
On Thu, 2 Dec 2021 12:13:03 GMT, Andrew Leonard  wrote:

>> Addition of a configure option --with-cacerts-src='user cacerts folder' to 
>> allow developers to specify their own cacerts PEM folder for generation of 
>> the cacerts store using the deterministic openjdk GenerateCacerts tool.
>> 
>> Signed-off-by: Andrew Leonard 
>
> Andrew Leonard 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 four additional 
> commits since the last revision:
> 
>  - 8278080: Add --with-cacerts-src='user cacerts folder' to enable 
> deterministic cacerts generation
>
>Signed-off-by: Andrew Leonard 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into cacertssrc
>  - 8278080: Add --with-cacerts-src='user cacerts folder' to enable 
> determinsitic cacerts generation
>
>Signed-off-by: Andrew Leonard 
>  - 8278080: Add --with-cacerts-src='user cacerts folder' to enable 
> determinsitic cacerts generation
>
>Signed-off-by: Andrew Leonard 

The purpose of this test is to ensure integrity of the cacerts file along with 
basic validation of included roots. Having a config file with this information 
sounds like a good idea for now to be able to handle multiple files.

-

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


Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation

2021-12-03 Thread Sergey Bylokhov
On Thu, 2 Dec 2021 10:55:57 GMT, Andrew Leonard  wrote:

> This is the case at Adoptium for example, which uses the Mozilla trusted CA 
> certs.

But they didn't think skipping this test was too strong a step? For example 
validation of the certs expiration is quite useful. I tried to update the test 
to take into account additional certs, but it caused a merge conflict each time 
the certs in OpenJDK are updated. Probably we can add a config file that can 
inject/override some info in the test(at least skip the checksum validation)? 
By default this config file will be empty and will not be modified in the 
OpenJDK, but the vendors will be able to modify it. @wangweij @rhalade what do 
you think?

-

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


Integrated: 8275821: Optimize random number generators developed in JDK-8248862 using Math.unsignedMultiplyHigh()

2021-12-03 Thread Vamsi Parasa
On Tue, 2 Nov 2021 03:02:42 GMT, Vamsi Parasa  wrote:

> This change optimizes random number generators using 
> Math.unsignedMultiplyHigh()

This pull request has now been integrated.

Changeset: 38f525e9
Author:vamsi-parasa 
Committer: Sandhya Viswanathan 
URL:   
https://git.openjdk.java.net/jdk/commit/38f525e96e767597d16698d4b243b681782acc9f
Stats: 103 lines in 4 files changed: 72 ins; 28 del; 3 mod

8275821: Optimize random number generators developed in JDK-8248862 using 
Math.unsignedMultiplyHigh()

Reviewed-by: psandoz, jlaskey

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v17]

2021-12-03 Thread Andrew Leonard
On Fri, 3 Dec 2021 10:05:43 GMT, Andrew Leonard  wrote:

>> Add a new --source-date  (epoch seconds) option to jar and jmod 
>> to allow specification of time to use for created/updated jar/jmod entries. 
>> This then allows the ability to make the content deterministic.
>> 
>> Signed-off-by: Andrew Leonard 
>
> Andrew Leonard has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 25 commits:
> 
>  - Merge jdk:master
>
>Signed-off-by: Andrew Leonard 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> jarjmodtimestamps
>  - 8276766: Enable jar and jmod to produce deterministic timestamped content
>
>Signed-off-by: Andrew Leonard 
>  - Merge branch 'jarjmodtimestamps' of github.com:andrew-m-leonard/jdk into 
> jarjmodtimestamps
>  - Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java
>
>Co-authored-by: Magnus Ihse Bursie 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> jarjmodtimestamps
>  - 8276766: Enable jar and jmod to produce deterministic timestamped content
>
>Signed-off-by: Andrew Leonard 
>  - 8276766: Enable jar and jmod to produce deterministic timestamped content
>
>Signed-off-by: Andrew Leonard 
>  - 8276766: Enable jar and jmod to produce deterministic timestamped content
>
>Signed-off-by: Andrew Leonard 
>  - 8276766: Enable jar and jmod to produce deterministic timestamped content
>
>Signed-off-by: Andrew Leonard 
>  - ... and 15 more: 
> https://git.openjdk.java.net/jdk/compare/45da3aea...06863697

I've got a mac so can test that, a little tricking building, but i'll do that 
this evening

-

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


Re: RFR: 8275821: Optimize random number generators developed in JDK-8248862 using Math.unsignedMultiplyHigh() [v4]

2021-12-03 Thread Vamsi Parasa
On Fri, 3 Dec 2021 17:19:17 GMT, Paul Sandoz  wrote:

>> @JimLaskey Could you please review this PR which optimizes three of the 
>> Pseudo-Random Number Generators you implemented in 
>> https://bugs.openjdk.java.net/browse/JDK-8248862 ?
>
> @vamsi-parasa you can now integrate this and then I or someone else will 
> sponsor it

Thank you @PaulSandoz  and @JimLaskey for reviewing the PR!

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v14]

2021-12-03 Thread John Neffenger
On Thu, 2 Dec 2021 01:53:47 GMT, John Neffenger  wrote:

>>> I'm testing it now. So far, it's working great!
>>> 
>>> I found one boundary problem. The magic instant of `1980-01-01T00:00:00Z` 
>>> triggers an extended header with the wrong time:
>>> 
>>> ```
>>>   file last modified on (DOS date/time):  1980 Jan 1 00:00:00
>>>   file last modified on (UT extra field modtime): 1980 Jan 1 00:00:00 local
>>>   file last modified on (UT extra field modtime): 1980 Jan 1 08:00:00 UTC
>>> ```
>>> 
>>> One second later `1980-01-01T00:00:01Z` works fine (considering the 
>>> rounding down to even seconds):
>>> 
>>> ```
>>>   file last modified on (DOS date/time):  1980 Jan 1 00:00:00
>>> ```
>> 
>> Thanks John. The 1980-01-01T00:00:00Z is a known issue because the zip 
>> xdostime format uses that value as the "marker" for date before-1980. I can 
>> adjust the --date value validation to be >  1980-01-01T00:00:00Z ?
>
>> I can adjust the --date value validation to be > 1980-01-01T00:00:00Z ?
> 
> I know, nit picky, right?  I'll take no offense if you ignore it. The thing 
> is, everyone else implementing this feature gave that specific instant a wide 
> margin. (See my previous comment about Debian and Gradle.) Debian adds 12 
> hours 1 minute just to avoid it (`1980-01-01T12:01:00Z`), while Gradle even 
> skips to the next month (`1980-02-01T00:00:00`).
> 
> Because developers looking into this discover the magic earliest date, some 
> may think it's a great choice for a default deterministic value and use it! 
> Then this current implementation breaks with no warning. If you add one 
> second, the `ZipEntry` method rounds down, and you get the exact same "DOS 
> date and time" in the output (1980 Jan 1 00:00:00), but now it works. Kind of 
> confusing.
> 
> So my latest recommendation is to define the earliest permitted instant at 
> `1980-01-01T00:00:02Z` to avoid any possibility of setting the magic local 
> date and time at all. The Gradle folks might explain it better in [their 
> comment here][1].
> 
> [1]: 
> https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/java/org/gradle/api/internal/file/archive/ZipCopyAction.java#L42-L56

> @jgneff thank you John, for your help with this as well, your platform 
> testing has been most useful as well.

You're welcome. I did not test on *x64* macOS or Windows, by the way. I'll test 
JavaFX builds on those platforms as soon as this enhancement is included in an 
[early-access build][1].

[1]: https://jdk.java.net/18/

-

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


Re: RFR: 8275821: Optimize random number generators developed in JDK-8248862 using Math.unsignedMultiplyHigh() [v4]

2021-12-03 Thread Paul Sandoz
On Thu, 2 Dec 2021 20:56:46 GMT, Vamsi Parasa  wrote:

>> Vamsi Parasa has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   add seeds for the random generators to eliminate run-to-run variance
>
> @JimLaskey Could you please review this PR which optimizes three of the 
> Pseudo-Random Number Generators you implemented in 
> https://bugs.openjdk.java.net/browse/JDK-8248862 ?

@vamsi-parasa you can now integrate this and then I or someone else will 
sponsor it

-

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


Integrated: 8278171: [vectorapi] Mask incorrectly computed for zero extending cast

2021-12-03 Thread Mai Đặng Quân Anh
On Wed, 1 Dec 2021 12:03:15 GMT, Mai Đặng Quân Anh  
wrote:

> When doing an unsigned upcast, we perform a signed cast followed by a bitwise 
> and operation to clip the extended signed bit. The sign clip mask is 
> currently calculated incorrectly, the correct mask should have the number of 
> least significant bit set equal to the size of the source elements. This 
> patch fixes this trivial issue and adds several tests for zero extension 
> casts.
> 
> Thank you very much.

This pull request has now been integrated.

Changeset: 2e30fa93
Author:merykitty 
Committer: Paul Sandoz 
URL:   
https://git.openjdk.java.net/jdk/commit/2e30fa936dd5fffceb17d338271f5e725c85801c
Stats: 322 lines in 2 files changed: 321 ins; 0 del; 1 mod

8278171: [vectorapi] Mask incorrectly computed for zero extending cast

Reviewed-by: psandoz

-

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


Re: RFR: 8278171: [vectorapi] Mask incorrectly computed for zero extending cast

2021-12-03 Thread Paul Sandoz
On Fri, 3 Dec 2021 03:44:09 GMT, Mai Đặng Quân Anh  
wrote:

> Thanks a lot for your support and review. Do I need another review here?

No need for this one, for next PR for the intrinsics will likely need two 
HotSpot reviewers.

-

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


Re: RFR: 8003417: WeakHashMap$HashIterator removes wrong entry

2021-12-03 Thread Roger Riggs
On Sat, 20 Nov 2021 10:08:41 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which proposes to fix the issue 
> reported in https://bugs.openjdk.java.net/browse/JDK-8003417?
> 
> The issue notes that this is applicable for `WeakHashMap` which have `null` 
> keys. However, the issue is even applicable for `WeakHashMap` instances which 
> don't have `null` keys, as reproduced and shown by the newly added jtreg test 
> case in this PR.
> 
> The root cause of the issue is that once the iterator is used to iterate till 
> the end and the `remove()` is called, then the 
> `WeakHashMap$HashIterator#remove()` implementation used to pass `null` as the 
> key to remove from the map, instead of the key of the last returned entry. 
> The commit in this PR fixes that part.
> 
> A new jtreg test has been added which reproduces the issue as well as 
> verifies the fix.
> `tier1` testing and this new test have passed after this change. However, I 
> guess this will require a JCK run to be run too, perhaps? If so, I will need 
> help from someone who has access to them to have this run against those 
> please.

Looks good to me.  Another set of eyes would be good on this fringe case.

-

Marked as reviewed by rriggs (Reviewer).

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


RFR: 8278044: ObjectInputStream methods invoking the OIF.CFG.getSerialFilterFactory() silent about error cases.

2021-12-03 Thread Roger Riggs
The specification of ObjectInputStream constructors that invoke 
`ObjectInputFilter.Config.getSerialFilterFactory()` do not mention exceptions 
that may be thrown by the apply() method.

In both constructors, add the following to the paragraph the describes invoking 
the factory:

 * When the filter factory {@code apply} method is invoked it may throw a 
runtime exception
 * preventing the {@code ObjectInputStream} from being constructed.

-

Commit messages:
 - 8278044: ObjectInputStream methods invoking the 
OIF.CFG.getSerialFilterFactory() silent about error cases.

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

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


Re: RFR: 8278087: Deserialization filter and filter factory property error reporting under specified

2021-12-03 Thread Roger Riggs
On Wed, 1 Dec 2021 18:19:05 GMT, Roger Riggs  wrote:

> The effects of invalid values of `jdk.serialFilter` and 
> `jdk.serialFilterFactory` properties are 
> incompletely specified. The behavior for invalid values of the properties is 
> different and
> use an unconventional exception type, `ExceptionInInitializerError` and leave 
> the `OIF.Config` class
> uninitialized. 
> 
> The exceptions in the `ObjectInputFilter.Config` class initialization caused 
> by invalid values of the two properties, 
> either by system properties supplied on the command line or security 
> properties are logged.
> The `Config` class marks either or both the filter and filter factory values 
> as unusable
> and remembers the exception message.
> 
> Subsequent calls to the methods that get or set the filter or filter factory 
> or create 
> an `ObjectInputStream` throw `java.lang.IllegalStateException` with the 
> remembered exception message.
> Constructing an `ObjectInputStream` calls both `Config.getSerialFilter` and 
> `Config.getSerialFilterFactory`.
> The nature of the invalid property is reported as an `IllegalStateException` 
> on first use.
> 
> This PR supercedes https://github.com/openjdk/jdk/pull/6508 Document that 
> setting an invalid property jdk.serialFilter disables deserialization

@stuart-marks, @jaikiran Please review,  Thanks

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v17]

2021-12-03 Thread Alan Bateman
On Mon, 29 Nov 2021 17:07:52 GMT, Alan Bateman  wrote:

>> @andrew-m-leonard I see you've add several APIs to ZipEntry in this PR. I 
>> think this part will require discussion as it's not immediately clear that 
>> we should over burden the ZipEntry API as proposed.
>
>> @AlanBateman yes, see above comment, thanks
> 
> This is a significant change to the ZipEntry API that will require discussion 
> and agreement. Can you start a discussion on core-libs-dev about the topic? 
> You could start with a summary of the problem and the API and non-API options 
> that have been explored.

> @AlanBateman @LanceAndersen i'd also like to get your final reviews as well 
> please?

I plan to review this in the coming days.

-

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v14]

2021-12-03 Thread Andrew Leonard
On Thu, 2 Dec 2021 01:53:47 GMT, John Neffenger  wrote:

>>> I'm testing it now. So far, it's working great!
>>> 
>>> I found one boundary problem. The magic instant of `1980-01-01T00:00:00Z` 
>>> triggers an extended header with the wrong time:
>>> 
>>> ```
>>>   file last modified on (DOS date/time):  1980 Jan 1 00:00:00
>>>   file last modified on (UT extra field modtime): 1980 Jan 1 00:00:00 local
>>>   file last modified on (UT extra field modtime): 1980 Jan 1 08:00:00 UTC
>>> ```
>>> 
>>> One second later `1980-01-01T00:00:01Z` works fine (considering the 
>>> rounding down to even seconds):
>>> 
>>> ```
>>>   file last modified on (DOS date/time):  1980 Jan 1 00:00:00
>>> ```
>> 
>> Thanks John. The 1980-01-01T00:00:00Z is a known issue because the zip 
>> xdostime format uses that value as the "marker" for date before-1980. I can 
>> adjust the --date value validation to be >  1980-01-01T00:00:00Z ?
>
>> I can adjust the --date value validation to be > 1980-01-01T00:00:00Z ?
> 
> I know, nit picky, right?  I'll take no offense if you ignore it. The thing 
> is, everyone else implementing this feature gave that specific instant a wide 
> margin. (See my previous comment about Debian and Gradle.) Debian adds 12 
> hours 1 minute just to avoid it (`1980-01-01T12:01:00Z`), while Gradle even 
> skips to the next month (`1980-02-01T00:00:00`).
> 
> Because developers looking into this discover the magic earliest date, some 
> may think it's a great choice for a default deterministic value and use it! 
> Then this current implementation breaks with no warning. If you add one 
> second, the `ZipEntry` method rounds down, and you get the exact same "DOS 
> date and time" in the output (1980 Jan 1 00:00:00), but now it works. Kind of 
> confusing.
> 
> So my latest recommendation is to define the earliest permitted instant at 
> `1980-01-01T00:00:02Z` to avoid any possibility of setting the magic local 
> date and time at all. The Gradle folks might explain it better in [their 
> comment here][1].
> 
> [1]: 
> https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/java/org/gradle/api/internal/file/archive/ZipCopyAction.java#L42-L56

@jgneff thank you John, for your help with this as well, your platform testing 
has been most useful as well.
I've Proposed the CSR now (https://bugs.openjdk.java.net/browse/JDK-8277755), 
so that should go through the approval process..
@AlanBateman @LanceAndersen i'd also like to get your final reviews as well 
please?
Many thanks
Andrew

-

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


Integrated: 8278144: Javadoc for MemorySegment::set/MemorySegment::setAtIndex is missing throws tag

2021-12-03 Thread Maurizio Cimadamore
On Thu, 2 Dec 2021 12:26:33 GMT, Maurizio Cimadamore  
wrote:

> This patch adds missing `@throws` tags to the javadoc of `MemorySegment::set` 
> and `MemorySegment::setAtIndex`.
> These methods can fail with `UnsupportedOperationException` if the segment is 
> read-only.

This pull request has now been integrated.

Changeset: 3f28a214
Author:Maurizio Cimadamore 
URL:   
https://git.openjdk.java.net/jdk/commit/3f28a21414be32375dc0f4b12d349826bacd4810
Stats: 17 lines in 1 file changed: 17 ins; 0 del; 0 mod

8278144: Javadoc for MemorySegment::set/MemorySegment::setAtIndex is missing 
throws tag

Reviewed-by: sundar

-

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


Integrated: 8278205: jlink plugins should dump .class file in debug mode

2021-12-03 Thread Athijegannathan Sundararajan
On Fri, 3 Dec 2021 10:03:30 GMT, Athijegannathan Sundararajan 
 wrote:

> jlink plugins dump .class files in debug mode. jpackage tests already set 
> jlink.debug property to true.

This pull request has now been integrated.

Changeset: ba2a8e5a
Author:Athijegannathan Sundararajan 
URL:   
https://git.openjdk.java.net/jdk/commit/ba2a8e5a496799451095362279b9dd4b6df20b67
Stats: 41 lines in 5 files changed: 35 ins; 0 del; 6 mod

8278205: jlink plugins should dump .class file in debug mode

Reviewed-by: jlaskey

-

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


Re: RFR: 8275821: Optimize random number generators developed in JDK-8248862 using Math.unsignedMultiplyHigh() [v6]

2021-12-03 Thread Jim Laskey
On Fri, 3 Dec 2021 02:14:51 GMT, Vamsi Parasa  wrote:

>> This change optimizes random number generators using 
>> Math.unsignedMultiplyHigh()
>
> Vamsi Parasa has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   minor changes to the benchmark

LGTM

-

Marked as reviewed by jlaskey (Reviewer).

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


Re: RFR: 8278205: jlink plugins should dump .class file in debug mode

2021-12-03 Thread Jim Laskey
On Fri, 3 Dec 2021 10:03:30 GMT, Athijegannathan Sundararajan 
 wrote:

> jlink plugins dump .class files in debug mode. jpackage tests already set 
> jlink.debug property to true.

LGTM

-

Marked as reviewed by jlaskey (Reviewer).

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


Re: RFR: 8278185: Custom JRE cannot find non-ASCII named module inside

2021-12-03 Thread Jim Laskey
On Fri, 3 Dec 2021 07:29:17 GMT, Toshio Nakamura  wrote:

> Could you review this fix?
> 
> Problem:
> Custom JRE generated by jlink cannot find non-ASCII named modules included 
> inside the JRE.
> 
> Cause and fix:
> If module or package name was composed by ASCII then non-ASCII characters, 
> ImageStringsReader:stringFromByteBufferMatches() miscalculated the length of 
> matched string. The first part of ASCII characters was missing. This patch 
> corrected the value.
> 
> Testing:
> tier1 and tier2 on Linux have no regression.
> I wasn't able to create an automate test for this issue. I appreciate any 
> advice.

Change looks reasonable. A regression test should be provided. Contact me 
directly for advice.

-

Changes requested by jlaskey (Reviewer).

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


RFR: 8278205: jlink plugins should dump .class file in debug mode

2021-12-03 Thread Athijegannathan Sundararajan
jlink plugins dump .class files in debug mode. jpackage tests already set 
jlink.debug property to true.

-

Commit messages:
 - 8278205: jlink plugins should dump .class file in debug mode

Changes: https://git.openjdk.java.net/jdk/pull/6696/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6696=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8278205
  Stats: 41 lines in 5 files changed: 35 ins; 0 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6696.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6696/head:pull/6696

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


Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v17]

2021-12-03 Thread Andrew Leonard
> Add a new --source-date  (epoch seconds) option to jar and jmod to 
> allow specification of time to use for created/updated jar/jmod entries. This 
> then allows the ability to make the content deterministic.
> 
> Signed-off-by: Andrew Leonard 

Andrew Leonard has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 25 commits:

 - Merge jdk:master
   
   Signed-off-by: Andrew Leonard 
 - Merge branch 'master' of https://github.com/openjdk/jdk into 
jarjmodtimestamps
 - 8276766: Enable jar and jmod to produce deterministic timestamped content
   
   Signed-off-by: Andrew Leonard 
 - Merge branch 'jarjmodtimestamps' of github.com:andrew-m-leonard/jdk into 
jarjmodtimestamps
 - Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java
   
   Co-authored-by: Magnus Ihse Bursie 
 - Merge branch 'master' of https://github.com/openjdk/jdk into 
jarjmodtimestamps
 - 8276766: Enable jar and jmod to produce deterministic timestamped content
   
   Signed-off-by: Andrew Leonard 
 - 8276766: Enable jar and jmod to produce deterministic timestamped content
   
   Signed-off-by: Andrew Leonard 
 - 8276766: Enable jar and jmod to produce deterministic timestamped content
   
   Signed-off-by: Andrew Leonard 
 - 8276766: Enable jar and jmod to produce deterministic timestamped content
   
   Signed-off-by: Andrew Leonard 
 - ... and 15 more: https://git.openjdk.java.net/jdk/compare/45da3aea...06863697

-

Changes: https://git.openjdk.java.net/jdk/pull/6481/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6481=16
  Stats: 329 lines in 8 files changed: 306 ins; 0 del; 23 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6481.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6481/head:pull/6481

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


Re: RFR: 8277992: Add fast jdk_svc subtests to jdk:tier3

2021-12-03 Thread Alan Bateman
On Thu, 2 Dec 2021 12:15:51 GMT, David Holmes  wrote:

> I've solicited feedback from core-libs folk as this affects them the most. At 
> present we, Oracle, run the jdk_svc tests as part of hotspot testing, but 
> this change will suddenly cause jdk testing to include them. That is probably 
> not an issue for testing within the CI framework but developers running 
> jdk_tier3 locally, often, may have an unpleasant surprise at running over a 
> thousand additional tests that probably have zero relevance to what they are 
> trying to test. Local machines may take considerably longer than the 8+ 
> minutes that you listed.

I don't expect there are many people that run jdk_tier3 locally. It's a mixed 
bag of tests for the Panama Vector API, RMI and JFR. Adding jdk_svc to this 
list just adds to the mix. The only synergy is that the RMI tests will be in 
the same tier as the JMX tests that use the RMI connectors. No objection to the 
change, I think it is just a re-balancing of tiers for CI systems.

-

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


Integrated: 8278163: --with-cacerts-src variable resolved after GenerateCacerts recipe setup

2021-12-03 Thread Andrew Leonard
On Thu, 2 Dec 2021 21:07:30 GMT, Andrew Leonard  wrote:

> PR https://github.com/openjdk/jdk/pull/6647 resolved the GENDATA_CACERTS_SRC 
> fir --with-cacerts-src after the recipe, which meant the dependencies were 
> wrong.
> This PR moves it before the recipe.
> 
> Signed-off-by: Andrew Leonard 

This pull request has now been integrated.

Changeset: 45da3aea
Author:Andrew Leonard 
URL:   
https://git.openjdk.java.net/jdk/commit/45da3aea22fd85f214e661b2c98631cb91ddb55d
Stats: 8 lines in 1 file changed: 4 ins; 3 del; 1 mod

8278163: --with-cacerts-src variable resolved after GenerateCacerts recipe setup

Reviewed-by: ihse

-

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