Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v3]

[Peter Levart]

On Tue, 30 Nov 2021 11:25:48 GMT, Roman Kennke  wrote:

>> The caches in ObjectStreamClass basically map WeakReference to 
>> SoftReference, where the ObjectStreamClass also 
>> references the same Class. That means that the cache entry, and thus the 
>> class and its class-loader, will not get reclaimed, unless the GC determines 
>> that memory pressure is very high.
>> 
>> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
>> all its classes alive much longer than necessary: as soon as a ClassLoader 
>> (and all its classes) become unreachable, there is no point in retaining the 
>> stuff in OSC's caches.
>> 
>> The proposed change is to use WeakReference instead of SoftReference for the 
>> values in caches.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [x] tier3
>>  - [ ] tier4
>
> Roman Kennke 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 six additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'jdk-plevart/JDK-8277072-peter' into 
> JDK-8277072
>  - Use ClassValue to solve JDK-8277072
>  - Use ForceGC instead of System.gc()
>  - Convert test to testng
>  - Fix indentation of new testcase
>  - 8277072: ObjectStreamClass caches keep ClassLoaders alive

I think most "hard work" (the tests) is still yours. I just removed a chunk of 
legacy code and replaced it with one-liners :-). I'm glad that this actually 
works! Please, continue...

-

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

Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v14]

[Ravi Reddy]

> Hi all,
> 
> Please review this fix for Infinite loop in ZipOutputStream.close().
> The main issue here is when ever there is an exception during close 
> operations on GZip we are not setting the deflator to a finished state which 
> is leading to an infinite loop when we try writing on the same GZip instance( 
> since we use while(!def.finished()) inside the write operation).
> 
> Thanks,
> Ravi

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

  Change in test case name from GZipLoopTest.java to CloseDeflaterTest , moved 
testcase to util/zip/

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5522/files
  - new: https://git.openjdk.java.net/jdk/pull/5522/files/47697f96..6541de46

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5522=13
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5522=12-13

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

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

Withdrawn: 8217496: Matcher.group() can return null after usePattern

[duke]

On Tue, 5 Oct 2021 19:11:57 GMT, Ian Graves  wrote:

> Specification update to clarify Matcher behavior to include a null return 
> value.

This pull request has been closed without being integrated.

-

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

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

[David Holmes]

On Tue, 30 Nov 2021 18:48:15 GMT, Aleksey Shipilev  wrote:

> OpenJDK tiered tests definitions have the catch-all `tier4` that runs all 
> tests not defined in the lower tiers. `hotspot:tier4` has lots of them, 
> mostly long-running vmTestbase tests, which take many hours even on a very 
> parallel machines.
> 
> This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom 
> run by contributors, because `hotspot:tier4` is in the way. But, there are 
> plenty of fast and stable tests in `jdk:tier4` that can be run in 
> `jdk:tier3`. `jdk_svc` is the example of such tests: management features 
> (including but not limited to JFR) are important to keep from regressions, 
> and significant subset of them runs pretty fast.
> 
> So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it 
> to more contributors. I think the only group we don't want to run is 
> `svc_tools`, which includes lots of non-parallel tests that are rather slow.
> 
> Sample run before:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3174   174 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  2m38.242s
> user  80m7.216s
> sys   2m13.846s
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   2904  2901 3 0 <<
> ==
> TEST FAILURE
> 
> real  18m13.933s
> user  546m50.556s
> sys   25m7.086s
> 
> 
> Sample run after:
> 
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>jtreg:test/jdk:tier3   1296  1296 0 0  
>  
> ==
> TEST SUCCESS
> 
> Finished building target 'run-test' in configuration 
> 'linux-x86_64-server-fastdebug'
> 
> real  7m49.017s
> user  287m30.943s
> sys   13m20.060s
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier4   1783  1780 3 0 <<
> ==
> TEST FAILURE
> 
> 
> real  12m19.973s
> user  351m48.561s
> sys   14m51.566s

Hi @shipilev ,
We need to have someone look at the impact of this on our CI. I don't think we 
run the tier4 group as defined in our tier 4 so we may not see those execution 
time savings that offset the extra cost to tier3.

-

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[Alan Snyder]

>> It is almost impossible to write any non-trivial code that is 
>> async-exception-safe and no JDK library code is written to be 
>> async-exception-safe including thread tear-down. So while you can say 
>> "stop() is the only way to disrupt this piece of code", you cannot ensure 
>> that it is disrupted safely. Once stop is used you need to throw away _all_ 
>> stateful objects that may have been in active use while ThreadDeath was 
>> propagated. And even during propagation you can easily trigger secondary 
>> exceptions.

It seems that it should be possible to stop a thread spawned to execute code in 
a native library that works on data in native memory..
If what you say about thread tear-down is true, then I guess I would need to 
spawn the thread from native code as well.



> On Nov 30, 2021, at 5:58 PM, David Holmes  wrote:
> 
> On 1/12/2021 3:13 am, Alan Snyder wrote:
>> Although I understand the potential dangers of using Thread.stop, it seems 
>> to me there are cases where its use is legitimate and valuable.
> 
> No there really aren't. :) The perceived utility of stop() is an illusion. It 
> is almost impossible to write any non-trivial code that is 
> async-exception-safe and no JDK library code is written to be 
> async-exception-safe including thread tear-down. So while you can say "stop() 
> is the only way to disrupt this piece of code", you cannot ensure that it is 
> disrupted safely. Once stop is used you need to throw away _all_ stateful 
> objects that may have been in active use while ThreadDeath was propagated. 
> And even during propagation you can easily trigger secondary exceptions.
> 
> Cheers,
> David
> 
> 
>> The examples I am thinking of involve a potentially long running computation 
>> whose result is no longer needed.
>> In particular, I am thinking of pure computations such as image analysis or 
>> audio analysis that do not involve waiting (so that interrupt is not useful)
>> and probably are implemented using some C library (which is not feasible to 
>> modify to insert code to support graceful interruption).
>> Is there some alternative that can be used in such cases?
>> Perhaps a version of stop() that only works if no locks are held?
>>   Alan
>>> On Nov 30, 2021, at 7:51 AM, Roger Riggs  wrote:
>>> 
>>> On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman  wrote:
>>> 
 Thread.stop is inherently unsafe and has been deprecated since Java 1.2 
 (1998). It's time to terminally deprecate this method so it can be 
 degraded and removed in the future.
 
 This PR does not propose any changes to the JVM TI StopThread function (or 
 the corresponding JDWP command or JDI method).
>>> 
>>> Past time for this to go.
>>> 
>>> 
> 

Re: RFR: 8277861: Terminally deprecate Thread.stop

[David Holmes]

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman  wrote:

> Thread.stop is inherently unsafe and has been deprecated since Java 1.2 
> (1998). It's time to terminally deprecate this method so it can be degraded 
> and removed in the future.
> 
> This PR does not propose any changes to the JVM TI StopThread function (or 
> the corresponding JDWP command or JDI method).

Approved with extreme prejudice. :)

-

Marked as reviewed by dholmes (Reviewer).

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[David Holmes]

On 1/12/2021 3:13 am, Alan Snyder wrote:

Although I understand the potential dangers of using Thread.stop, it seems to 
me there are cases where its use is legitimate and valuable.


No there really aren't. :) The perceived utility of stop() is an 
illusion. It is almost impossible to write any non-trivial code that is 
async-exception-safe and no JDK library code is written to be 
async-exception-safe including thread tear-down. So while you can say 
"stop() is the only way to disrupt this piece of code", you cannot 
ensure that it is disrupted safely. Once stop is used you need to throw 
away _all_ stateful objects that may have been in active use while 
ThreadDeath was propagated. And even during propagation you can easily 
trigger secondary exceptions.


Cheers,
David



The examples I am thinking of involve a potentially long running computation 
whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or 
audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to 
modify to insert code to support graceful interruption).

Is there some alternative that can be used in such cases?

Perhaps a version of stop() that only works if no locks are held?

   Alan






On Nov 30, 2021, at 7:51 AM, Roger Riggs  wrote:

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman  wrote:


Thread.stop is inherently unsafe and has been deprecated since Java 1.2 (1998). 
It's time to terminally deprecate this method so it can be degraded and removed 
in the future.

This PR does not propose any changes to the JVM TI StopThread function (or the 
corresponding JDWP command or JDI method).


Past time for this to go.

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

[John Neffenger]

On Tue, 30 Nov 2021 21:56:51 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 incrementally with one additional 
> commit since the last revision:
> 
>   8276766: Enable jar and jmod to produce deterministic timestamped content
>   
>   Signed-off-by: Andrew Leonard 

I found it helpful to look at what others have done.

The Debian `strip-nondeterminism` tool clamps the low end of the date to 
`1980-01-01T12:01:00`. All other dates are permitted no matter the consequence. 
That tool is written in Perl and is able to access all of the timestamp fields 
individually. See the [`zip.pm` file][1] for details.

Gradle decided that the only permissible date is `1980-02-01T00:00:00` in the 
default time zone of the JVM. Period. End of story. No customization at all. 
[They use a trick][2] with the old `GregorianCalendar` class to counteract the 
effect in `ZipEntry.setTime` of the default time zone. They use the 
`org.apache.tools.zip.ZipEntry` subclass of `java.util.zip.ZipEntry`, but many 
methods pass through to the super class. See the [`ZipCopyAction.java` file][3] 
for details.

The crux of our problem, as Andrew made more clear to me, is that the methods 
of `ZipEntry` do not allow for independent access to the two fields: the "DOS 
date/time" field and the "UT extra field modtime". When trying to set either 
one of them, the `ZipEntry` class can overwrite the other using the default 
time zone of the JVM.

@andrew-m-leonard So I agree. Either document that the archives are 
reproducible only within a certain range of dates, or disallow the dates that 
are out of range. Then we should be good for at least the next 78 years.

[1]: 
https://salsa.debian.org/reproducible-builds/strip-nondeterminism/-/blob/master/lib/File/StripNondeterminism/handlers/zip.pm#L40
[2]: https://github.com/gradle/gradle/issues/12895#issuecomment-618850095
[3]: 
https://github.com/gradle/gradle/blob/master/subprojects/core/src/main/java/org/gradle/api/internal/file/archive/ZipCopyAction.java#L42-L56

-

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

Re: RFR: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

[Jaikiran Pai]

On Tue, 30 Nov 2021 14:28:05 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which fixes a typo in the javadoc 
> of `java.util.zip.ZipEntry.setTime()` method?

Thank you everyone for the reviews.

-

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

Integrated: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

[Jaikiran Pai]

On Tue, 30 Nov 2021 14:28:05 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which fixes a typo in the javadoc 
> of `java.util.zip.ZipEntry.setTime()` method?

This pull request has now been integrated.

Changeset: 0a01baaf
Author:Jaikiran Pai 
URL:   
https://git.openjdk.java.net/jdk/commit/0a01baaf2dd31a0fe2bc8b1327fb072cc3909eeb
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

Reviewed-by: alanb, iris, lancea

-

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

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

[John Neffenger]

On Tue, 30 Nov 2021 21:56:51 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 incrementally with one additional 
> commit since the last revision:
> 
>   8276766: Enable jar and jmod to produce deterministic timestamped content
>   
>   Signed-off-by: Andrew Leonard 

Here is the 32-line Java program that I've been modifying to find problems and 
solutions:


import java.io.FileOutputStream;
import java.io.IOException;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.time.temporal.ChronoUnit;
import java.util.TimeZone;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

public class Time {

static void writeZipFile(String name, ZipEntry entry) throws IOException {
var output = new ZipOutputStream(new FileOutputStream(name));
output.putNextEntry(entry);
output.closeEntry();
output.close();
}

public static void main(String[] args) throws IOException {
var instant = Instant.now().truncatedTo(ChronoUnit.SECONDS);
if (args.length > 0) {
instant = Instant.parse(args[0]);
}
System.out.println("Build timestamp = " + instant);

var entry = new ZipEntry("Entry");
var local = LocalDateTime.ofInstant(instant, ZoneOffset.UTC);

TimeZone.setDefault(TimeZone.getTimeZone("America/Nome"));
entry.setTimeLocal(local);
writeZipFile("Zipped_in_Nome.zip", entry);

TimeZone.setDefault(TimeZone.getTimeZone("Europe/Rome"));
entry.setTimeLocal(local);
writeZipFile("Zipped_in_Rome.zip", entry);
}
}


For example, first I pick up the timestamp of the last commit in my JavaFX fork:


$ export SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct)
$ date --date=@$SOURCE_DATE_EPOCH --iso-8601=seconds --utc
2021-11-22T05:00:22+00:00
$ git log -1 --pretty=%cI
2021-11-21T21:00:22-08:00


Then I can verify that the timestamp in the archive is unaffected by the 
default time zone of the JVM:


$ java Time 2021-11-21T21:00:22-08:00
Build timestamp = 2021-11-22T05:00:22Z
$ for f in *.zip; do zipinfo -v $f | grep -e Archive -e modified; done
Archive:  Zipped_in_Nome.zip
  file last modified on (DOS date/time):  2021 Nov 22 05:00:22
Archive:  Zipped_in_Rome.zip
  file last modified on (DOS date/time):  2021 Nov 22 05:00:22


Even when I create the ISO 8601 date and time string on a system in a different 
time zone than the two simulated build machines in Nome and Rome, it still 
works:


$ java Time 2021-11-22T08:00:22+03:00
Build timestamp = 2021-11-22T05:00:22Z
$ for f in *.zip; do zipinfo -v $f | grep -e Archive -e modified; done
Archive:  Zipped_in_Nome.zip
  file last modified on (DOS date/time):  2021 Nov 22 05:00:22
Archive:  Zipped_in_Rome.zip
  file last modified on (DOS date/time):  2021 Nov 22 05:00:22


But it all falls apart when we venture outside the permitted DOS date and time 
range:


$ java Time 1975-11-22T05:00:22+00:00
Build timestamp = 1975-11-22T05:00:22Z
$ for f in *.zip; do zipinfo -v $f | grep -e Archive -e modified; done
Archive:  Zipped_in_Nome.zip
  file last modified on (DOS date/time):  1980 Jan 1 00:00:00
  file last modified on (UT extra field modtime): 1975 Nov 22 08:00:22 local
  file last modified on (UT extra field modtime): 1975 Nov 22 16:00:22 UTC
Archive:  Zipped_in_Rome.zip
  file last modified on (DOS date/time):  1980 Jan 1 00:00:00
  file last modified on (UT extra field modtime): 1975 Nov 21 20:00:22 local
  file last modified on (UT extra field modtime): 1975 Nov 22 04:00:22 UTC

-

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

Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v5]

[Paul Sandoz]

On Wed, 24 Nov 2021 15:20:42 GMT, kabutz  wrote:

>> BigInteger currently uses three different algorithms for multiply. The 
>> simple quadratic algorithm, then the slightly better Karatsuba if we exceed 
>> a bit count and then Toom Cook 3 once we go into the several thousands of 
>> bits. Since Toom Cook 3 is a recursive algorithm, it is trivial to 
>> parallelize it. I have demonstrated this several times in conference talks. 
>> In order to be consistent with other classes such as Arrays and Collection, 
>> I have added a parallelMultiply() method. Internally we have added a 
>> parameter to the private multiply method to indicate whether the calculation 
>> should be done in parallel.
>> 
>> The performance improvements are as should be expected. Fibonacci of 100 
>> million (using a single-threaded Dijkstra's sum of squares version) 
>> completes in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with 
>> the sequential multiply() method. This is on my 1-8-2 laptop. The final 
>> multiplications are with very large numbers, which then benefit from the 
>> parallelization of Toom-Cook 3. Fibonacci 100 million is a 347084 bit number.
>> 
>> We have also parallelized the private square() method. Internally, the 
>> square() method defaults to be sequential.
>> 
>> Some benchmark results, run on my 1-6-2 server:
>> 
>> 
>> Benchmark  (n)  Mode  Cnt  Score 
>>  Error  Units
>> BigIntegerParallelMultiply.multiply100ss4 51.707 
>> ±   11.194  ms/op
>> BigIntegerParallelMultiply.multiply   1000ss4988.302 
>> ±  235.977  ms/op
>> BigIntegerParallelMultiply.multiply  1ss4  24662.063 
>> ± 1123.329  ms/op
>> BigIntegerParallelMultiply.parallelMultiply100ss4 49.337 
>> ±   26.611  ms/op
>> BigIntegerParallelMultiply.parallelMultiply   1000ss4527.560 
>> ±  268.903  ms/op
>> BigIntegerParallelMultiply.parallelMultiply  1ss4   9076.551 
>> ± 1899.444  ms/op
>> 
>> 
>> We can see that for larger calculations (fib 100m), the execution is 2.7x 
>> faster in parallel. For medium size (fib 10m) it is 1.873x faster. And for 
>> small (fib 1m) it is roughly the same. Considering that the fibonacci 
>> algorithm that we used was in itself sequential, and that the last 3 
>> calculations would dominate, 2.7x faster should probably be considered quite 
>> good on a 1-6-2 machine.
>
> kabutz has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   Made forkOrInvoke() method protected to avoid strange compiler error

IIRC you are not overly concerned about the additional object creation of 
`RecursiveOp` instances?

If that changes the operation method could return `Object` and choose to 
perform the operation directly returning the `BigInteger` result or returning 
the particular `RecursiveOp`.


private static Object multiply(BigInteger a, BigInteger b, boolean parallel, 
int depth) {
if (isParallel(parallel, depth)) {
return new RecursiveMultiply(a, b, parallel, depth).fork();
} else {
// Also called by RecursiveMultiply.compute()
return RecursiveMultiply.compute(a, b, false, depth);
}
}


Then we could have another method on `RecursiveOp` that pattern matches e.g.:

  static BigInteger join(Object o) {
// replace with pattern match switch when it exits preview
if (o instanceof BigInteger b) {
return b;
} else if (o instanceof RecursiveOp r) {
return r.join();
} else {
throw new InternalError("Cannot reach here);
}
  }


That seems simple enough it might be worth doing anyway.

src/java.base/share/classes/java/math/BigInteger.java line 1874:

> 1872:  */
> 1873: private static final int PARALLEL_FORK_THRESHOLD = 
> Integer.getInteger(
> 1874: "java.math.BigInteger.parallelForkThreshold",

I suspect we don't need this system property, there is no such property for 
streams. We use `ForkJoinPool.getCommonPoolParallelism()`, which is 
configurable as an escape hatch.

src/java.base/share/classes/java/math/BigInteger.java line 1875:

> 1873: private static final int PARALLEL_FORK_THRESHOLD = 
> Integer.getInteger(
> 1874: "java.math.BigInteger.parallelForkThreshold",
> 1875: (int) 
> Math.ceil(Math.log(Runtime.getRuntime().availableProcessors()) / 
> Math.log(2)));

We can simplify to `32 - 
Integer.numberOfLeadingZeros(ForkJoinPool.getCommonPoolParallelism() - 1))`.

`ForkJoinPool.getCommonPoolParallelism()` is guaranteed to return a value `> 0`

src/java.base/share/classes/java/math/BigInteger.java line 1878:

> 1876: 
> 1877: @Serial
> 1878: private static final long serialVersionUID = 0L;

I don't think you need to declare these, the instances are never intended to 
support serialization e.g. in the stream implementation for 

Re: RFR: JDK-8278014: [vectorapi] Remove test run script [v2]

[Paul Sandoz]

> Remove Vector API scripts for building and running tests. `jtreg` should be 
> used instead.
> 
> Also updated the test generation script to remove options that assume 
> mercurial as the code repository.

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

  Update copywrite year.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6621/files
  - new: https://git.openjdk.java.net/jdk/pull/6621/files/65fa1fbe..4a5bf3b9

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

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

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

Re: RFR: JDK-8278014: [vectorapi] Remove test run script

[Paul Sandoz]

On Tue, 30 Nov 2021 23:24:24 GMT, Jie Fu  wrote:

> Shall we also update the copyright year?

Doh! of course, updated.

-

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

Re: RFR: JDK-8278014: [vectorapi] Remove test run script [v2]

[Jie Fu]

On Tue, 30 Nov 2021 23:31:21 GMT, Paul Sandoz  wrote:

>> Remove Vector API scripts for building and running tests. `jtreg` should be 
>> used instead.
>> 
>> Also updated the test generation script to remove options that assume 
>> mercurial as the code repository.
>
> Paul Sandoz has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update copywrite year.

Thanks for your update.
LGTM

-

Marked as reviewed by jiefu (Reviewer).

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

Re: RFR: JDK-8278014: [vectorapi] Remove test run script

[Jie Fu]

On Tue, 30 Nov 2021 19:22:53 GMT, Paul Sandoz  wrote:

> Remove Vector API scripts for building and running tests. `jtreg` should be 
> used instead.
> 
> Also updated the test generation script to remove options that assume 
> mercurial as the code repository.

Shall we also update the copyright year?

-

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

Integrated: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently

[Naoto Sato]

On Mon, 29 Nov 2021 18:48:45 GMT, Naoto Sato  wrote:

> Fixing tests that fail at DST->STD offset transition. Simply skipping the 
> tests on that occasion.

This pull request has now been integrated.

Changeset: f1c20e91
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/f1c20e91d822c6df4c5da895f2abd6305e00bf8b
Stats: 12 lines in 2 files changed: 6 ins; 0 del; 6 mod

8190748: java/text/Format/DateFormat/DateFormatTest.java and 
NonGregorianFormatTest fail intermittently

Reviewed-by: rriggs, joehw, lancea

-

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

Re: RFR: 8277924: Small tweaks to foreign function and memory API [v4]

[Maurizio Cimadamore]

> Following integration of the second incubator of the foreign function and 
> memory API [1], we detected few divergences between the contents of the jdk 
> repo and the panama repo:
> 
> * the name of some of the `FunctionDescriptor` wither methods is different 
> (e.g. `withAppendedLayoutArguments` vs. `appendLayoutArguments`), as it has 
> been simplified and improved following a change that was not incorporated in 
> [1].
> 
> * TestUpcall does not execute all the test combinations, because of an issue 
> in the jtreg header (also fixed in the panama repo)
> 
> * Addressing some feedback, we would like to bring back alignment to JAVA_INT 
> layout constants (and related constants). 
> 
> Javadoc: 
> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/javadoc/jdk/incubator/foreign/package-summary.html
> Specdiff: 
> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/spec_diff/overview-summary.html
> 
> [1] - #5907

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

  Simplify FunctionDescriptor::insertArgumentLayouts

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6589/files
  - new: https://git.openjdk.java.net/jdk/pull/6589/files/e2dfb83b..c305199c

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

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

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

Re: RFR: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions

[Joe Darcy]

On Thu, 11 Nov 2021 13:59:51 GMT, Jim Laskey  wrote:

> The modified ziggurat algorithm is not correctly implemented in 
> `java.base/jdk/internal/util/random/RandomSupport.java`. 
> 
> Create a histogram of a million samples using 2000 uniform bins with the 
> following range: 
> Exponential range from 0 to 12. Gaussian range from -8 to 8. 
> 
> This does not pass a Chi-square test. If you look at the histogram it is 
> obviously not showing the shape of the PDF for these distributions. Look 
> closely at the range around zero (e.g. +/- 0.5).

Should there be a regression test here?

-

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

Re: RFR: JDK-8273146: Start of release updates for JDK 19 [v6]

[Joe Darcy]

> The time to get JDK 19 underway draws nigh, please review this usual set of 
> start-of-release updates, including CSRs for the javac and javax.lang.model 
> updates:
> 
> JDK-8277512: Add SourceVersion.RELEASE_19
> https://bugs.openjdk.java.net/browse/JDK-8277512
> 
> JDK-8277514: Add source 19 and target 19 to javac
> https://bugs.openjdk.java.net/browse/JDK-8277514
> 
> Clean local tier 1 test runs for langtools, jdk, and hotspot.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The pull request now contains ten commits:

 - Merge branch 'master' into JDK-8273146
 - Update symbol information for JDK 18 b25.
 - Merge branch 'master' into JDK-8273146
 - Merge branch 'master' into JDK-8273146
 - Remove SharedSpaces options from VMDeprecatedOptions.java.
 - Merge branch 'master' into JDK-8273146
 - Respond to review feedback.
 - Add --release 18 information. Good tier1 test results.
 - JDK-8273146: Start of release updates for JDK 19

-

Changes: https://git.openjdk.java.net/jdk/pull/6493/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6493=05
  Stats: 3286 lines in 68 files changed: 3242 ins; 4 del; 40 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6493.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6493/head:pull/6493

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

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

[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 incrementally with one additional 
commit since the last revision:

  8276766: Enable jar and jmod to produce deterministic timestamped content
  
  Signed-off-by: Andrew Leonard 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6481/files
  - new: https://git.openjdk.java.net/jdk/pull/6481/files/62e2f7c8..283e435a

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

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions

[Brian Burkhalter]

On Thu, 11 Nov 2021 13:59:51 GMT, Jim Laskey  wrote:

> The modified ziggurat algorithm is not correctly implemented in 
> `java.base/jdk/internal/util/random/RandomSupport.java`. 
> 
> Create a histogram of a million samples using 2000 uniform bins with the 
> following range: 
> Exponential range from 0 to 12. Gaussian range from -8 to 8. 
> 
> This does not pass a Chi-square test. If you look at the histogram it is 
> obviously not showing the shape of the PDF for these distributions. Look 
> closely at the range around zero (e.g. +/- 0.5).

Looks fine.

src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 
1191:

> 1189: // At this point, the high-order bits of U1 have not 
> been used yet,
> 1190: // but we need the value in U1 to be positive.
> 1191: for (U1 = (U1 >>> 1);; U1 = (rng.nextLong() >>> 1)) {

A minor thing, but I would probably write `U1 >>>= 1` instead of `U1 = (U1 >>> 
1)`.

-

Marked as reviewed by bpb (Reviewer).

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

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

[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 incrementally with one additional 
commit since the last revision:

  8276766: Enable jar and jmod to produce deterministic timestamped content
  
  Signed-off-by: Andrew Leonard 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6481/files
  - new: https://git.openjdk.java.net/jdk/pull/6481/files/06aadf1e..62e2f7c8

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

  Stats: 21 lines in 1 file changed: 0 ins; 17 del; 4 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: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently [v2]

[Lance Andersen]

On Mon, 29 Nov 2021 21:59:31 GMT, Naoto Sato  wrote:

>> Fixing tests that fail at DST->STD offset transition. Simply skipping the 
>> tests on that occasion.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Changed to not skipping the test in DateFormatTest.java

looks good to me

-

Marked as reviewed by lancea (Reviewer).

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

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

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

 - 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 
 - 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 
 - 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 4 more: https://git.openjdk.java.net/jdk/compare/98a9f037...06aadf1e

-

Changes: https://git.openjdk.java.net/jdk/pull/6481/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6481=09
  Stats: 288 lines in 9 files changed: 260 ins; 0 del; 28 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: JDK-8278014: [vectorapi] Remove test run script

[Sandhya Viswanathan]

On Tue, 30 Nov 2021 19:22:53 GMT, Paul Sandoz  wrote:

> Remove Vector API scripts for building and running tests. `jtreg` should be 
> used instead.
> 
> Also updated the test generation script to remove options that assume 
> mercurial as the code repository.

Looks good to me.

-

Marked as reviewed by sviswanathan (Reviewer).

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v2]

[Roger Riggs]

On Mon, 29 Nov 2021 08:18:47 GMT, Сергей Цыпанов  wrote:

>> Instead of something like
>> 
>> long x;
>> long y;
>> return (x < y) ? -1 : ((x == y) ? 0 : 1);
>> 
>> we can use `return Long.compare(x, y);`
>> 
>> All replacements are done with IDE.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8277868: Use Integer.signum() in BasicTableUI

The core-libs file changes look fine.
A 'client' reviewer should take a look too.

-

Marked as reviewed by rriggs (Reviewer).

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

Re: RFR: 8277606: String(String) constructor could copy hashIsZero

[Roger Riggs]

On Mon, 22 Nov 2021 22:59:21 GMT, PROgrm_JARvis  wrote:

> This is a trivial fix to make `String(String)` constructor copy the value of 
> `hashIsZero` field.

Marked as reviewed by rriggs (Reviewer).

-

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

Integrated: 8277606: String(String) constructor could copy hashIsZero

[PROgrm_JARvis]

On Mon, 22 Nov 2021 22:59:21 GMT, PROgrm_JARvis  wrote:

> This is a trivial fix to make `String(String)` constructor copy the value of 
> `hashIsZero` field.

This pull request has now been integrated.

Changeset: e30e6767
Author:Petr Portnov 
Committer: Roger Riggs 
URL:   
https://git.openjdk.java.net/jdk/commit/e30e67670981ee905724787c109b7b7fd2b70b42
Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod

8277606: String(String) constructor could copy hashIsZero

Reviewed-by: redestad, rriggs

-

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[Alan Snyder]

I think you are saying that to kill a thread running native code I would need 
to use native code. Is that right?

> On Nov 30, 2021, at 10:17 AM, Alan Bateman  wrote:
> 
> On 30/11/2021 17:13, Alan Snyder wrote:
>> Although I understand the potential dangers of using Thread.stop, it seems 
>> to me there are cases where its use is legitimate and valuable.
>> 
>> The examples I am thinking of involve a potentially long running computation 
>> whose result is no longer needed.
>> In particular, I am thinking of pure computations such as image analysis or 
>> audio analysis that do not involve waiting (so that interrupt is not useful)
>> and probably are implemented using some C library (which is not feasible to 
>> modify to insert code to support graceful interruption).
>> 
> 
> JCiP Ch.7 has some good advice on this topic. In general, it needs the task 
> to poll a cancel status or test Thread.currentThread().isInterrupted() to 
> check for interrupt. In your scenario, with image analysis in native code, 
> then Thread.stop won't help as it would need to return from the native code 
> to detect the async exception.
> 
> -Alan
> 

Re: RFR: 8277322: Document that setting an invalid property jdk.serialFilter disables deserialization

[Roger Riggs]

On Wed, 24 Nov 2021 15:39:13 GMT, Roger Riggs  wrote:

>> If the intent is to disable serialization entirely, then this state should 
>> be represented explicitly. Having things throw `NoClassDefFoundError` looks 
>> like a mistake and a bug that needs to be fixed. In addition, it requires 
>> that all code paths to deserialization use `OIF.Config` in order to provoke 
>> the NCDFE. This might be true today, but under maintenance a different code 
>> path might be introduced that happens not to use that class, allowing 
>> deserialization to proceed.
>> 
>> An alternative policy might be to disallow any deserialization that would 
>> use the default filter. This could be accomplished by having a "fail-safe" 
>> or "fallback" filter that always returns REJECT. This would be at least as 
>> restrictive and thus no less safe than any valid filter that could be 
>> installed. In addition, it would allow things that don't use the global 
>> filter to proceed, such as,
>> 
>> var ois = new ObjectInputStream(...);
>> ois.setObjectInputFilter(new ObjectInputFilter() { ... });
>> 
>> or
>> 
>> var ois = new ObjectInputStream(...);
>> ois.setObjectInputFilter(ObjectInputFilter.Config.createFilter(...));
>> 
>> There could be a reasonable policy discussion about whether it's preferable 
>> to disable all deserialization or just deserialization that depends on the 
>> (invalidly specified) global filter.
>
> This is about the second line of defense; what happens when the developer 
> deliberately ignores the first error.
> If the command line parameters are invalid it might be an option to call 
> `System.exit(1)` but there
> is no precedent for that and it seems undesirable.
> 
> Any and all deserialization is only possible after the command line or 
> security properties, if any, are successfully applied.
> In the examples above, the constructors for `ObjectInputStream` do not 
> succeed if either the serial filter or filter factory are not valid.  The 
> builtin filter factory applies the default filter regardless of the setting 
> of an `ObjectInputFilter` set on the stream.  The only way to completely 
> control the filter combinations is to provide
> a valid filter factory on the command line; but that is not the case raising 
> the issue here.
> 
> The initialization of both could be re-specified and re-implemented to allow 
> the initialization of `Config` to
> complete successfully but defer throwing an exception (or Error) until either 
> filter or filter factory
> was requested from `Config.getSerialFilter` or 
> `Config.getSerialFilterFactory`.
> Both of them are called from the `ObjectInputStream` constructors. 
> At present, it is incompletely specified and implemented to throw 
> `IllegalStateException` for `getSerialFilterFactory`.
> 
> The `NCDFE` is a more reliable backstop against misuse from any source, 
> including reflection, than the alternative.

The use of `ExceptionInInitializerError` can be completely replaced for  
invalid values of `jdk.serialFilter` and `jdk.serialFilterFactory` with:

- If either property is supplied and is invalid; deserialization is disabled by:
- `OIF.Config.getSerialFilter()` and `OIF.Config.setSerialFilter(...)` throw 
IllegalStateException with the exception message thrown from 
`Config.createFilter(pattern)`
- `OIF.Config.getSerialFilterFactory()` and 
`OIF.Config.setSerialFilterFactory(...)` throw IllegalStateException with the 
exception message from the attempt to construct the filter factory.
- Both `new ObjectInputStream(...)` constructors call both 
`OIF.Config.getSerialFilter()` and `OIF.Config.getSerialFilterFactory()` and so 
will throw the appropriate `IllegalStateException` for invalid values of the 
properties.
- The static initialization of `OIF.Config` does not throw any exceptions (so 
no `ExceptionInInitializerError`) but hold the state so that the method above 
can throw `IllegalStateException` with the informative message.
- The `IllegalStateException`s will be thrown just slightly later than 
previously, now after the `Config` class is initialized instead of during 
initialization.
- The javadoc of the `Config` class will replace the descriptions of the 
previous error with descriptions of `ISE` and each of the methods mentioned 
above will have an added `IllegalStateException` documented referring to the 
property values.

Its not strictly compatible with the previous behavior but occurs only in the 
case of badly formed parameters on the command line.

-

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

Re: RFR: 8276657: XSLT compilter tries to define a class with empty name

[Joe Wang]

On Tue, 30 Nov 2021 18:53:26 GMT, Joe Wang  wrote:

> The result of Util.baseName(systemId) can be empty, causing the compiler to 
> set an empty classname. Add a check to make sure it will not set the empty 
> classname.
> 
> Alternatively, it may report an error, but that would be disruptive. As the 
> transform can proceed without the provided classname (by using the default), 
> adding a check is better than reporting an error.
> 
> I've verified the patch with the proposed fix for JDK-8276241. Harold has 
> also confirmed it fixes the tests in his builds.

Thanks Naoto. Updated with isEmpty.

-

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

Re: RFR: 8276657: XSLT compilter tries to define a class with empty name [v2]

[Joe Wang]

> The result of Util.baseName(systemId) can be empty, causing the compiler to 
> set an empty classname. Add a check to make sure it will not set the empty 
> classname.
> 
> Alternatively, it may report an error, but that would be disruptive. As the 
> transform can proceed without the provided classname (by using the default), 
> adding a check is better than reporting an error.
> 
> I've verified the patch with the proposed fix for JDK-8276241. Harold has 
> also confirmed it fixes the tests in his builds.

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

  s/equals/isEmpty

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6620/files
  - new: https://git.openjdk.java.net/jdk/pull/6620/files/ef0e6b88..24b0152a

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

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

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

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

[Andrew Leonard]

On Tue, 30 Nov 2021 20:00:19 GMT, John Neffenger  wrote:

> > Whichever we use, we have to use e.setTimeLocal(), so can't see what the 
> > difference is?
> 
> Okay, here's a brief command-line example before posting the code example. In 
> my experience, most people trying to set up a common, project-wide build 
> timestamp use the formatted string in UTC:
> 
> ```
> $ date -d @$SOURCE_DATE_EPOCH --iso-8601=seconds --utc
> 2021-11-29T17:36:06+00:00
> ```
> 
> In JavaFX, we're using the more concise Java version of that string: 
> `2021-11-29T17:36:06Z`.
> 
> So here's what happens when you get that exact same instant in time from 
> different sources on different machines:
> 
> ```
> $ TZ=America/Nome date -d @$SOURCE_DATE_EPOCH --iso-8601=seconds
> 2021-11-29T08:36:06-09:00
> $ TZ=Europe/Rome date -d @$SOURCE_DATE_EPOCH --iso-8601=seconds
> 2021-11-29T18:36:06+01:00
> $ git log -1 --pretty=%cI
> 2021-11-29T09:36:06-08:00
> ```
> 
> All of a sudden it makes a big difference when you're discarding the time 
> zone. You end up with differences in the archive files depending on (a) the 
> time zone of the build machine, and (b) the source you use to obtain an ISO 
> 8601 string to represent that singular instant.

AH! got ya, so I was looking at the requirement from a slightly different 
perspective, ie. reproducibility is same input == same output, so to my mind:
--date "2021-11-29T08:36:06-09:00"
is a different input to:
--date "2021-11-29T09:36:06-08:00"

ie. the String is different! but as you say, the use case is actually same 
input === "same INSTANT in TIME", my mind was totally not seeing that, thank 
you!
ok all makes sense now :-)

-

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

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

[John Neffenger]

On Tue, 30 Nov 2021 19:37:23 GMT, Andrew Leonard  wrote:

> Whichever we use, we have to use e.setTimeLocal(), so can't see what the 
> difference is?

Okay, here's a brief command-line example before posting the code example. In 
my experience, most people trying to set up a common, project-wide build 
timestamp use the formatted string in UTC:


$ date -d @$SOURCE_DATE_EPOCH --iso-8601=seconds --utc
2021-11-29T17:36:06+00:00


In JavaFX, we're using the more concise Java version of that string: 
`2021-11-29T17:36:06Z`.

So here's what happens when you get that exact same instant in time from 
different sources on different machines:


$ TZ=America/Nome date -d @$SOURCE_DATE_EPOCH --iso-8601=seconds
2021-11-29T08:36:06-09:00
$ TZ=Europe/Rome date -d @$SOURCE_DATE_EPOCH --iso-8601=seconds
2021-11-29T18:36:06+01:00
$ git log -1 --pretty=%cI
2021-11-29T09:36:06-08:00


All of a sudden it makes a big difference when you're discarding the time zone. 
You end up with differences in the archive files depending on (a) the time zone 
of the build machine, and (b) the source you use to obtain an ISO 8601 string 
to represent that singular instant.

-

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

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

[John Neffenger]

On Tue, 30 Nov 2021 18:57:55 GMT, Andrew Leonard  wrote:

> I'm also adding --date validation of 1980->2099

I am only now catching up with you on that issue.   I wrote a very short 
program that illustrates that problem (and all the other ones) very clearly for 
anyone to see. I'll post it shortly.

-

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

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

[Andrew Leonard]

On Tue, 30 Nov 2021 19:31:37 GMT, John Neffenger  wrote:

> > Both basically truncate the timezone.
> 
> There's a difference. The first option saves a DOS date and time that depends 
> on the time zone of the build machine due to the ISO 8601 string returned by 
> default from the `git` and `date` commands, which is what everyone will be 
> using. Those commands return the local time zone offset by default, which is 
> what breaks your current implementation.

@jgneff I think you've lost me on this one so example may help me please..
Whichever we use, we have to use e.setTimeLocal(), so can't see what the 
difference is?

-

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

RFR: JDK-8278014: [vectorapi] Remove test run script

[Paul Sandoz]

Remove Vector API scripts for building and running tests. `jtreg` should be 
used instead.

Also updated the test generation script to remove options that assume mercurial 
as the code repository.

-

Commit messages:
 - JDK-8278014: [vectorapi] Remove test run script

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

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

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

[John Neffenger]

On Tue, 30 Nov 2021 18:52:56 GMT, Andrew Leonard  wrote:

> Both basically truncate the timezone.

There's a difference. The first option saves a DOS date and time that depends 
on the time zone of the build machine due to the ISO 8601 string returned by 
default from the `git` and `date` commands, which is what everyone will be 
using. Those commands return the local time zone offset by default, which is 
what breaks your current implementation.

The second option converts to a DOS date and time that is independent of the 
time zone of the build machine, as long as you stay withing the range of its 
values, as you discovered.

-

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

Re: RFR: 8276657: XSLT compilter tries to define a class with empty name

[Naoto Sato]

On Tue, 30 Nov 2021 18:53:26 GMT, Joe Wang  wrote:

> The result of Util.baseName(systemId) can be empty, causing the compiler to 
> set an empty classname. Add a check to make sure it will not set the empty 
> classname.
> 
> Alternatively, it may report an error, but that would be disruptive. As the 
> transform can proceed without the provided classname (by using the default), 
> adding a check is better than reporting an error.
> 
> I've verified the patch with the proposed fix for JDK-8276241. Harold has 
> also confirmed it fixes the tests in his builds.

Looks good. `isEmpty()` can be used in place for `.equals("")`.

-

Marked as reviewed by naoto (Reviewer).

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

Re: RFR: 8277606: String(String) constructor could copy hashIsZero

[PROgrm_JARvis]

On Mon, 22 Nov 2021 22:59:21 GMT, PROgrm_JARvis  wrote:

> This is a trivial fix to make `String(String)` constructor copy the value of 
> `hashIsZero` field.

Hi there, could anyone please sponsor this change if it is still applicable?

Thanks in advance!

-

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

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

[Aleksey Shipilev]

OpenJDK tiered tests definitions have the catch-all `tier4` that runs all tests 
not defined in the lower tiers. `hotspot:tier4` has lots of them, mostly 
long-running vmTestbase tests, which take many hours even on a very parallel 
machines.

This, unfortunately, has a chilling effect on `jdk:tier4`, which is seldom run 
by contributors, because `hotspot:tier4` is in the way. But, there are plenty 
of fast and stable tests in `jdk:tier4` that can be run in `jdk:tier3`. 
`jdk_svc` is the example of such tests: management features (including but not 
limited to JFR) are important to keep from regressions, and significant subset 
of them runs pretty fast.

So, it makes sense to move some `jdk_svc` tests to `jdk:tier3` to expose it to 
more contributors. I think the only group we don't want to run is `svc_tools`, 
which includes lots of non-parallel tests that are rather slow.

Sample run before:


==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk:tier3174   174 0 0   
==
TEST SUCCESS

Finished building target 'run-test' in configuration 
'linux-x86_64-server-fastdebug'

real2m38.242s
user80m7.216s
sys 2m13.846s


==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
>> jtreg:test/jdk:tier4   2904  2901 3 0 <<
==
TEST FAILURE

real18m13.933s
user546m50.556s
sys 25m7.086s


Sample run after:


==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
   jtreg:test/jdk:tier3   1296  1296 0 0   
==
TEST SUCCESS

Finished building target 'run-test' in configuration 
'linux-x86_64-server-fastdebug'

real7m49.017s
user287m30.943s
sys 13m20.060s

==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
>> jtreg:test/jdk:tier4   1783  1780 3 0 <<
==
TEST FAILURE


real12m19.973s
user351m48.561s
sys 14m51.566s

-

Commit messages:
 - Fix

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

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

RFR: 8276657: XSLT compilter tries to define a class with empty name

[Joe Wang]

The result of Util.baseName(systemId) can be empty, causing the compiler to set 
an empty classname. Add a check to make sure it will not set the empty 
classname.

Alternatively, it may report an error, but that would be disruptive. As the 
transform can proceed without the provided classname (by using the default), 
adding a check is better than reporting an error.

I've verified the patch with the proposed fix for JDK-8276241. Harold has also 
confirmed it fixes the tests in his builds.

-

Commit messages:
 - 8276657: XSLT compilter tries to define a class with empty name

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

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

Re: RFR: JDK-8273056 java.util.random does not correctly sample exponential or Gaussian distributions

[Jim Laskey]

On Thu, 11 Nov 2021 13:59:51 GMT, Jim Laskey  wrote:

> The modified ziggurat algorithm is not correctly implemented in 
> `java.base/jdk/internal/util/random/RandomSupport.java`. 
> 
> Create a histogram of a million samples using 2000 uniform bins with the 
> following range: 
> Exponential range from 0 to 12. Gaussian range from -8 to 8. 
> 
> This does not pass a Chi-square test. If you look at the histogram it is 
> obviously not showing the shape of the PDF for these distributions. Look 
> closely at the range around zero (e.g. +/- 0.5).

Still waiting for a review on these changes.

-

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

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

[Andrew Leonard]

On Mon, 29 Nov 2021 16:22:30 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 incrementally with two additional 
> commits since the last revision:
> 
>  - 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 

I'm also adding --date validation of 1980->2099

-

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

Re: RFR: 8277924: Small tweaks to foreign function and memory API [v3]

[Paul Sandoz]

On Tue, 30 Nov 2021 13:20:32 GMT, Maurizio Cimadamore  
wrote:

>> Following integration of the second incubator of the foreign function and 
>> memory API [1], we detected few divergences between the contents of the jdk 
>> repo and the panama repo:
>> 
>> * the name of some of the `FunctionDescriptor` wither methods is different 
>> (e.g. `withAppendedLayoutArguments` vs. `appendLayoutArguments`), as it has 
>> been simplified and improved following a change that was not incorporated in 
>> [1].
>> 
>> * TestUpcall does not execute all the test combinations, because of an issue 
>> in the jtreg header (also fixed in the panama repo)
>> 
>> * Addressing some feedback, we would like to bring back alignment to 
>> JAVA_INT layout constants (and related constants). 
>> 
>> Javadoc: 
>> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/javadoc/jdk/incubator/foreign/package-summary.html
>> Specdiff: 
>> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/spec_diff/overview-summary.html
>> 
>> [1] - #5907
>
> Maurizio Cimadamore has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Drop changes to alignment of layout constants
>  - Simplify logic to access package-private method in ValueLayout

Marked as reviewed by psandoz (Reviewer).

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/FunctionDescriptor.java
 line 140:

> 138: public FunctionDescriptor insertArgumentLayouts(int index, 
> MemoryLayout... addedLayouts) {
> 139: Objects.requireNonNull(addedLayouts);
> 140: Arrays.stream(addedLayouts).forEach(Objects::requireNonNull);

FWIW you can remove this line if you wish, since the `List.of()` will check for 
null elements. You could replace lines 139 to 143 with:

if (index < 0 || index > argLayouts.size())
throw new IllegalArgumentException("Index out of bounds: " + index);
List added = List.of(addedLayouts); // null check on array and 
its elements
List newLayouts = new ArrayList<>(argLayouts.size() + 
added.size());

-

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

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

[Andrew Leonard]

On Tue, 30 Nov 2021 16:26:28 GMT, John Neffenger  wrote:

> > So what you suggest sounds reasonable, although it would be a "behaviour 
> > change" in that whereas now if the date is between 1980->2106 only a 
> > xdostime is stored, we would now always be storing the additional "mtime" 
> > field ...
> 
> No, sorry, that is not at all what I suggested.
> 
> I am suggesting to immediately convert the value of the `--date` command-line 
> option to an `Instant`. Once you have an `Instant` object, it's just like 
> before when you had the value of `SOURCE_DATE_EPOCH` (which is in fact an 
> instant on the time line), so you can revert to using your prior revision 
> that handled that situation just perfectly!
> 
> In fact, you always want to avoid storing the addition "extended timestamp" 
> field when possible because it causes a rather surprising increase in the 
> size of the archive.
> 
> I wanted to send this right away, because you're such a fast coder, and in a 
> different time zone, but please give me some time to look over your and 
> Jaikiran's comments in more detail this morning. Thanks! 
@jgneff John, sorry got confused, but I see what you're saying now. So what 
you're saying is take the --date , work out a LocalDateTime for that 
"instant" but in UTC, then call e.setTimeLocal(localDateTime).
So I think i'm in fact just putting together a new commit for option (1) using 
setTimeLocal() similar to this, just very slightly different but efftecively 
doing the same thing, i'm converting the user input --date string to a 
ZonedDateTime, from which I get the LocalDateTime for calling 
setTimeLocal(zonedDateTime.toLocalDateTime()). That way when you unzip it the 
files have the same local date time.
eg.
--date="2022-03-15T14:36:00+06:00"

-

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

Re: RFR: JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream

[Pavel Rappo]

On Thu, 25 Nov 2021 00:31:46 GMT, Pavel Rappo  wrote:

>> JDK-8276674 : Malformed Javadoc inline tags in JDK source in java/util/stream
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/util/RawDiagnosticFormatter.java
>  line 63:
> 
>> 61:  * Helper class to generate stable positions for AST nodes occurring 
>> in diagnostic arguments.
>> 62:  * If the AST node appears in the same line number as the main 
>> diagnostic, the line is information is omitted.
>> 63:  * Otherwise both line and column information is included, using the 
>> format {@code line:col}".
> 
> Not a showstopper by any means. But while you are at it, maybe you could also 
> fix this?
> 
> Suggestion:
> 
>  * If the AST node appears in the same line number as the main 
> diagnostic, the line information is omitted.
>  * Otherwise both line and column information is included, using the 
> format {@code line:col}.

This part was recently integrated in https://github.com/openjdk/jdk/pull/6602. 
I assume there will be a trivial merge conflict to resolve.

-

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

Re: RFR: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

[Lance Andersen]

On Tue, 30 Nov 2021 14:28:05 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which fixes a typo in the javadoc 
> of `java.util.zip.ZipEntry.setTime()` method?

Marked as reviewed by lancea (Reviewer).

-

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[Uwe Schindler]

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman  wrote:

> Thread.stop is inherently unsafe and has been deprecated since Java 1.2 
> (1998). It's time to terminally deprecate this method so it can be degraded 
> and removed in the future.
> 
> This PR does not propose any changes to the JVM TI StopThread function (or 
> the corresponding JDWP command or JDI method).

Let's this go away! 

-

Marked as reviewed by uschindler (Author).

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[Alan Bateman]

On 30/11/2021 17:13, Alan Snyder wrote:

Although I understand the potential dangers of using Thread.stop, it seems to 
me there are cases where its use is legitimate and valuable.

The examples I am thinking of involve a potentially long running computation 
whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or 
audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to 
modify to insert code to support graceful interruption).



JCiP Ch.7 has some good advice on this topic. In general, it needs the 
task to poll a cancel status or test 
Thread.currentThread().isInterrupted() to check for interrupt. In your 
scenario, with image analysis in native code, then Thread.stop won't 
help as it would need to return from the native code to detect the async 
exception.


-Alan

Re: RFR: 8276141: XPathFactory set/getProperty method [v11]

[Joe Wang]

On Tue, 30 Nov 2021 18:00:38 GMT, Joe Wang  wrote:

>> Add setProperty/getProperty methods to the XPath API so that properties can 
>> be supported in the future.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add a statement to clarify the space of the properties

Hi all, refer to Joe's comment on the CSR, adding a statement to clarify where 
properties may be defined, that is, the method may be used for setting 
properties that may have been defined by the spec or the underlying impl.

-

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

Re: RFR: 8276141: XPathFactory set/getProperty method [v11]

[Joe Wang]

> Add setProperty/getProperty methods to the XPath API so that properties can 
> be supported in the future.

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

  Add a statement to clarify the space of the properties

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6215/files
  - new: https://git.openjdk.java.net/jdk/pull/6215/files/3f1ca154..c791758a

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

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

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[Mandy Chung]

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman  wrote:

> Thread.stop is inherently unsafe and has been deprecated since Java 1.2 
> (1998). It's time to terminally deprecate this method so it can be degraded 
> and removed in the future.
> 
> This PR does not propose any changes to the JVM TI StopThread function (or 
> the corresponding JDWP command or JDI method).

Marked as reviewed by mchung (Reviewer).

-

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

Re: RFR: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

[Iris Clark]

On Tue, 30 Nov 2021 14:28:05 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which fixes a typo in the javadoc 
> of `java.util.zip.ZipEntry.setTime()` method?

Marked as reviewed by iris (Reviewer).

-

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[Alan Snyder]

Although I understand the potential dangers of using Thread.stop, it seems to 
me there are cases where its use is legitimate and valuable.

The examples I am thinking of involve a potentially long running computation 
whose result is no longer needed.
In particular, I am thinking of pure computations such as image analysis or 
audio analysis that do not involve waiting (so that interrupt is not useful)
and probably are implemented using some C library (which is not feasible to 
modify to insert code to support graceful interruption).

Is there some alternative that can be used in such cases?

Perhaps a version of stop() that only works if no locks are held?

  Alan





> On Nov 30, 2021, at 7:51 AM, Roger Riggs  wrote:
> 
> On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman  wrote:
> 
>> Thread.stop is inherently unsafe and has been deprecated since Java 1.2 
>> (1998). It's time to terminally deprecate this method so it can be degraded 
>> and removed in the future.
>> 
>> This PR does not propose any changes to the JVM TI StopThread function (or 
>> the corresponding JDWP command or JDI method).
> 
> Past time for this to go.
> 
> 

Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v13]

[Sean Coffey]

On Thu, 18 Nov 2021 19:09:18 GMT, Ravi Reddy  wrote:

>> Hi all,
>> 
>> Please review this fix for Infinite loop in ZipOutputStream.close().
>> The main issue here is when ever there is an exception during close 
>> operations on GZip we are not setting the deflator to a finished state which 
>> is leading to an infinite loop when we try writing on the same GZip 
>> instance( since we use while(!def.finished()) inside the write operation).
>> 
>> Thanks,
>> Ravi
>
> Ravi Reddy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added ZipException case to handle failure CopyZipFile.java - Only close the 
> deflater incase of IOException not in ZipException case scenario

could you move the test up one directory to java/util/zip ? I don't think it's 
particular to GZIP any longer. Also perhaps a rename to something like 
CloseDeflaterTest etc.

-

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

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

[John Neffenger]

On Tue, 30 Nov 2021 12:00:59 GMT, Andrew Leonard  wrote:

> So what you suggest sounds reasonable, although it would be a "behaviour 
> change" in that whereas now if the date is between 1980->2106 only a xdostime 
> is stored, we would now always be storing the additional "mtime" field ...

No, sorry, that is not at all what I suggested.

I am suggesting to immediately convert the value of the `--date` command-line 
option to an `Instant`. Once you have an `Instant` object, it's just like 
before when you had the value of `SOURCE_DATE_EPOCH` (which is in fact an 
instant on the time line), so you can revert to using your prior revision that 
handled that situation just perfectly!

In fact, you always want to avoid storing the addition "extended timestamp" 
field when possible because it causes a rather surprising increase in the size 
of the archive.

I wanted to send this right away, because you're such a fast coder, and in a 
different time zone, but please give me some time to look over your and 
Jaikiran's comments in more detail this morning. Thanks! 

-

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

Re: RFR: 8277861: Terminally deprecate Thread.stop

[Roger Riggs]

On Tue, 30 Nov 2021 14:52:37 GMT, Alan Bateman  wrote:

> Thread.stop is inherently unsafe and has been deprecated since Java 1.2 
> (1998). It's time to terminally deprecate this method so it can be degraded 
> and removed in the future.
> 
> This PR does not propose any changes to the JVM TI StopThread function (or 
> the corresponding JDWP command or JDI method).

Past time for this to go.

-

Marked as reviewed by rriggs (Reviewer).

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

Re: Adding an @Immutable annotation to Java

[Brian Goetz]

I don’t see how a restricted reference by itself is useful, if you cannot 
depend upon the object not being mutated via other references.

Agree; this may help you do local proofs of correctness, and may conceivably 
help the JIT (though, its pretty smart about figuring this stuff out on its 
own), but the benefit doesn’t rise to the level of the complexity here.

The main value of an @Immutable designation is that it is safe to share a 
reference without additional coordination (both sharing with untrusted code, 
and sharing across threads).  But, this is only useful if the designation is 
(a) true all the way down, and (b) cannot be subverted by unsafe casts.

One needs look no farther than String to realize the near-hopelessness of this 
task.  A String is logically immutable, but its representation includes (a) a 
mutable cache of its hashCode, and (b) a reference to a mutable array of bytes 
or chars.  Even if we were willing to throw (a) overboard, (b) would require 
some sort of @TrustMe, which we could conceivably do for jl.String, but 
couldn’t possibly expose more broadly without driving the value of the 
mechanism to near-zero.  Treat this as an existence proof that mutability is 
just too pervasive to contain, at least for now.

There are efforts underway to chip away at some of the untamed mutability; 
frozen arrays are on the drawing board (addressing (b)), and Valhalla will join 
records in defining _shallowly_ immutable aggregates.  (It could conceivably go 
farther, but not until we can at least bring String into the fold.)  But we’d 
need to make a lot more progress before anyone could consider believing an 
@Immutable designation.  And if it can’t be believed, it doesn’t have enough 
value to put in the language.  (No problem with privately using annotations to 
capture design intent in documentation, but the bar for the language is higher 
than that.)

There are other impediments too; much ink has been spilled on the challenges of 
capturing immutability in a type hierarchy as complex as Collections.  But my 
main point is that while this is something that initially seems desirable, when 
you start pulling on the string, you realize it is not as useful as it 
initially seems in an environment of pervasive mutability.

RFR: 8277861: Terminally deprecate Thread.stop

[Alan Bateman]

Thread.stop is inherently unsafe and has been deprecated since Java 1.2 (1998). 
It's time to terminally deprecate this method so it can be degraded and removed 
in the future.

This PR does not propose any changes to the JVM TI StopThread function (or the 
corresponding JDWP command or JDI method).

-

Commit messages:
 - Initial commit

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

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

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

[Andrew Leonard]

On Mon, 29 Nov 2021 16:22:30 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 incrementally with two additional 
> commits since the last revision:
> 
>  - 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 

Thanks for taking a look Jaikiran,
I concur with the above. I'm currently looking to make the rest of jar/jmod
deterministic as there
are several currentTimeMillis() used... i'll probably update this to be at
least consistent
with the others.
Thanks
Andrew


On Tue, Nov 30, 2021 at 3:02 PM mlbridge[bot] ***@***.***>
wrote:

> *Mailing list message from Jaikiran Pai ***@***.***> on
> core-libs-dev ***@***.***>:*
>
> Hello Andrew,
>
> On 30/11/21 7:32 pm, Andrew Leonard wrote:
>
> On Mon, 29 Nov 2021 16:22:30 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.
>
> However, it looks like this behavior to not set extended mtime within the
> xdostime range has just been changed by a recent PR: https://github.com/
> /pull/5486 /files which has
> changed jartool.Main using zipEntry.setTime(time) to use
> zipEntry.setLastModifiedTime(time), the later sets mtime always regardless
> of xdostime.
>
> When I changed the implementation in sun.tools.jar.Main to use
> setLastModifiedTime(FileTime) instead of the previous setTime(long), I
> hadn't paid close attention to the implementation of these methods in
> java.util.zip.ZipEntry. Now that you brought this up, I had a more
> closer look at their implementation.
>
> So it looks like the implementation of setTime(long) conditionally sets
> the extended timestamp fields in optional extra data of the zip entry.
> That condition checks to see if the time value being set is before 1980
> or after 2099 and if it passes this condition, it goes and sets the
> extended timestamp fields. So in practice, for jar creation/updation in
> sun.tools.jar.Main, the previous code using setTime(long) wouldn't have
> set the extended timestamp fields, because the current year(s) aren't
> before 1980 or after 2099. The implementation of
> java.util.zip.ZipEntry.setLastModifiedTime(FileTime) on the other hand
> has no such conditional checks and (as noted in the javadoc of that
> method) goes ahead and sets the time value in the extended timestamp
> fields of that zip entry.
>
> So yes, the change that went in to
> https://github.com//pull/5486 
> did introduce this subtle
> change in the generated zip/jar entries. Having said that, the
> setTime(long) on java.util.zip.ZipEntry makes no mention of these
> conditional checks. It currently states:
>
>  * Sets the last modification time of the entry.
>  *
>  *  If the entry is output to a ZIP file or ZIP file formatted
>  * output stream the last modification time set by this method will
>  * be stored into the ***@***.*** date and time fields} of the zip file
>  * entry and encoded in standard ***@***.*** MS-DOS date and time format}.
>  * The ***@***.*** java.util.TimeZone#getDefault() default TimeZone} is
>  * used to convert the epoch time to the MS-DOS data and time.
>
> So it doesn't even make a mention of setting extended timestamp fields,
> let along setting them on a particular condition. So I'm unsure whether
> switching to setLastModifiedTime(FileTime) instead of setTime(long) was
> a bad thing.
>
> The implications of https://bugs.openjdk.java.net/browse/JDK-8073497
> might also apply to FileTime unnecessary initialization slowing VM startup,
> however if FileTime is already regularly referenced during startup, then it
> wont.. If this is the case then way forward (1) would be ok...
> @AlanBateman was that an intentional change? @jaikiran
>
> I had a look at that JBS issue now. From what I understand in its
> description and goal, the idea was to prevent initialization of
> java.util.Date utilities very early on during the startup. I had a quick
> look at the code in ZipEntry and how it behaves when it constructs a
> ZipEntry instance out of the zip file data. With the change in
> https://github.com//pull/5486 ,
> the "mtime" (which represents
> extended timestamp field) will be set in the zip file entry, so there
> would be an attempt to create a FileTime out of it. The call that does
> that appears to be "unixTimeToFileTime" 

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

[Andrew Leonard]

On Tue, 30 Nov 2021 08:53:03 GMT, John Neffenger  wrote:

>> @andrew-m-leonard Thank you, Andrew, for your bold solution to Mark's 
>> request -- one that even solves the problem with the current `ZipEntry` API!
>> 
>> Would you be willing to consider a minor change to your implementation?
>> 
>> As I mentioned earlier, if we are going to allow an ISO 8601 zoned date and 
>> time, we need either to **truncate, restrict, or convert** its value. The 
>> current implementation **truncates,** discarding the time zone information. 
>> The better option might be to **convert.** That is, parse the ISO 8601 
>> string with `Instant.parse`, get the seconds since the epoch with 
>> `getEpochSecond`, and then proceed as before. Treat the value of `--date` as 
>> an instant on the time line, just like `SOURCE_DATE_EPOCH`, and always store 
>> its value in UTC.
>> 
>> This also solves the problem of storing dates before 1980 as a local date 
>> and time in UTC while storing those after 1980 as a local date and time with 
>> a discarded time zone. By converting instead of truncating, the value is 
>> always stored as the local date and time in UTC regardless of whether or not 
>> it's within the range of the "MS-DOS date and time" field.
>> 
>> Those of us using the timestamp of the last commit can still get the value 
>> directly from `git`.
>> 
>> For `SOURCE_DATE_EPOCH`, run:
>> 
>> 
>> $ git log -1 --pretty=%ct
>> 1638207366
>> 
>> 
>> For the `--date` option of the `jar` and `jmod` tools, run:
>> 
>> 
>> $ git log -1 --pretty=%cI
>> 2021-11-29T09:36:06-08:00
>> 
>> 
>> Specifically, that `git` ISO 8601 string, and even the sample `date` command 
>> that Mark shows in his comment, are what break the current implementation by 
>> creating archives that depend on the time zone of the build machine again.
>> 
>> By the way, the `jar` utility displays a time zone in the output of its 
>> `--list` and `--verbose` options even when there is no time zone information 
>> in the file. (In other words, it lies, or at least embellishes.) Use the 
>> verbose output of `zipinfo` to see the true values of the timestamps in an 
>> archive.
>
>> Would you be willing to consider a minor change to your implementation?
> 
> Just to be clear, this change should let us postpone any additions to the 
> `ZipEntry` API for another day, if at all, and maybe even get this pull 
> request integrated for JDK 18.

@jgneff John, i'm going to update this PR to use the LocalDateTime setting 
option and restrict --date to 1980->2099, thus ensuring xdostime is always only 
used. This I believe we should be able to get into jdk-18.

-

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

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

[Jaikiran Pai]

Hello Andrew,

On 30/11/21 7:32 pm, Andrew Leonard wrote:

On Mon, 29 Nov 2021 16:22:30 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.



However, it looks like this behavior to not set extended mtime within the 
xdostime range has just been changed by a recent PR: 
https://github.com/openjdk/jdk/pull/5486/files which has changed jartool.Main 
using zipEntry.setTime(time) to use zipEntry.setLastModifiedTime(time), the 
later sets mtime always regardless of xdostime.


When I changed the implementation in sun.tools.jar.Main to use 
setLastModifiedTime(FileTime) instead of the previous setTime(long), I 
hadn't paid close attention to the implementation of these methods in 
java.util.zip.ZipEntry. Now that you brought this up, I had a more 
closer look at their implementation.


So it looks like the implementation of setTime(long) conditionally sets 
the extended timestamp fields in optional extra data of the zip entry. 
That condition checks to see if the time value being set is before 1980 
or after 2099 and if it passes this condition, it goes and sets the 
extended timestamp fields. So in practice, for jar creation/updation in 
sun.tools.jar.Main, the previous code using setTime(long) wouldn't have 
set the extended timestamp fields, because the current year(s) aren't 
before 1980 or after 2099. The implementation of 
java.util.zip.ZipEntry.setLastModifiedTime(FileTime) on the other hand 
has no such conditional checks and (as noted in the javadoc of that 
method) goes ahead and sets the time value in the extended timestamp 
fields of that zip entry.


So yes, the change that went in to 
https://github.com/openjdk/jdk/pull/5486 did introduce this subtle 
change in the generated zip/jar entries. Having said that, the 
setTime(long) on java.util.zip.ZipEntry makes no mention of these 
conditional checks. It currently states:


 * Sets the last modification time of the entry.
 *
 *  If the entry is output to a ZIP file or ZIP file formatted
 * output stream the last modification time set by this method will
 * be stored into the {@code date and time fields} of the zip file
 * entry and encoded in standard {@code MS-DOS date and time format}.
 * The {@link java.util.TimeZone#getDefault() default TimeZone} is
 * used to convert the epoch time to the MS-DOS data and time.

So it doesn't even make a mention of setting extended timestamp fields, 
let along setting them on a particular condition. So I'm unsure whether 
switching to setLastModifiedTime(FileTime) instead of setTime(long) was 
a bad thing.



The implications of https://bugs.openjdk.java.net/browse/JDK-8073497 might also 
apply to FileTime unnecessary initialization slowing VM startup, however if 
FileTime is already regularly referenced during startup, then it wont.. If this 
is the case then way forward (1) would be ok...
@AlanBateman was that an intentional change? @jaikiran


I had a look at that JBS issue now. From what I understand in its 
description and goal, the idea was to prevent initialization of 
java.util.Date utilities very early on during the startup. I had a quick 
look at the code in ZipEntry and how it behaves when it constructs a 
ZipEntry instance out of the zip file data. With the change in 
https://github.com/openjdk/jdk/pull/5486, the "mtime" (which represents 
extended timestamp field) will be set in the zip file entry, so there 
would be an attempt to create a FileTime out of it. The call that does 
that appears to be "unixTimeToFileTime" in ZipEntry and as far as I can 
see it doesn't end up using any java.util.Date classes. Of course, I 
haven't yet looked or experimented to verify and be absolutely sure 
about it, but from a cursory check it doesn't look like this would 
contribute to pulling in java.util.Date utilities.


-Jaikiran

Re: 8276766: Discuss options for deterministic jar/jmod timestamps across timezones

[Andrew Leonard]

Ah, I suspect the getTime() for xdostime performance issue relates to this:
https://bugs.openjdk.java.net/browse/JDK-4981560
which is not relevant any more since dosToJavaTime() is now implemented
differently

On Tue, Nov 30, 2021 at 2:44 PM Andrew Leonard  wrote:

> Thanks Alan,
> Having tried implementing a Zoned option, I agree it does seem to bloat
> ZipEntry a bit.
> As JohnN's suggestion pointed out we could at some point move to always
> setting the extended
> mtime(FileTime), but we have 78years until we really need to worry about
> that! (xdostime covers to 2099)
> It seems a conscious decision to only set xdostime was made for VM startup
> optimization (see https://bugs.openjdk.java.net/browse/JDK-8073497), at
> least until 2099.
>
> I will update the PR based on option (1), which I have a commit which is
> nearly at that point anyway.
> I like your suggestion of restricting the --date option to 1980->2099
>
> regards
> Andrew
>
>
> On Tue, Nov 30, 2021 at 1:13 PM Alan Bateman 
> wrote:
>
>> On 29/11/2021 19:25, Andrew Leonard wrote:
>>
>> *Problem:*
>> PR https://github.com/openjdk/jdk/pull/6481
>> addresses the main problems with deterministic timestamping of Zip entries,
>> with
>> the introduction of a new --date  option for jar & jmod.
>> However, the ZipEntry timestamp is constructed from a combination of an
>> MS-DOS timestamp
>> and a FileTime(seconds since epoch). MS-DOS timestamp is used between
>> 1980->2106, FileTime is
>> used outside that range.
>> The problem arises in deterministically setting the ZipEntry times within
>> JVMs
>> with different default system time-zones. This occurs because
>> ZipEntry.setTime(long millisSinceEpoch) or
>> setTimeLocal(LocalDateTime) are unaware of time-zones. So when setTime has
>> to calculate the msdostime from
>> millisSinceEpoch, it has to query the system time-zone to work out what the
>> "date-time" is. Similarly, for
>> setTimeLocal(LocalDateTime), if the LocalDateTime is outside 1980-2106
>> range, then it has to create a
>> FileTime, hence again it has to use the system time-zone to work out how
>> many seconds since epoch
>> the LocalDateTime is.
>>
>> *Solutions proposed so far:*
>> 1. Take the --date  value and create a LocalDateTime
>> eg.1982-12-09T14:02:06, and call
>> ZipEntry.setTimeLocal(with LocalDateTime"1982-12-09T14:02:06"). For the
>> same input and across time-zones
>> the ZipEntry is deterministic when within the valid MS-DOS time range
>> (1980->2106), because no extended
>> FileTime is created.
>> Pros:
>> - No API changes, uses existing ZipEntry.setTimeLoca(LocalDateTime)
>> Cons:
>> - Is only determinstic between years 1980->2106, outside this range a
>> FileTime is generated using the system
>> time-zone.
>>
>> 2. Add new "Zoned" set/get methods to ZipEntry, so that ZipEntry does not
>> have to defer to the system time-zone.
>> By adding set/getTimeZoned(ZonedDateTime),
>> set/getLastModifiedTimeZoned(FileTime, ZoneId) methods to ZipEntry
>> the ZipEntry MS-DOS time and extended FileTime can be set precisely and
>> deterministically to the user supplied
>> date-time.
>> Pros:
>> - ZipEntry is always deterministically created for the same input --date
>> , across different JVM
>> time-zones, for all time ranges.
>> Cons:
>> - An API addition is required to ZipEntry to add new "Zoned" set/get time
>> methods.
>>
>> Thoughts, comments, or alternatives most welcome please?
>>
>> ZipEntry already defines 3 methods to set the last modification time so I
>> think we have to be cautious about adding additional methods. If we
>> eventually do add a method to set the date-time with time zone then I think
>> it will require broader work in the javadoc to help developers choose the
>> right method to use.
>>
>> I don't think we should get hung up being able to set (via a CLI option)
>> the time stamps to the 1970-01-01 epoch, it's just not interesting. For
>> reproducible builds it should sufficient to set the timestamp to a fixed
>> DOS date/time.
>>
>> So for the above options then I think #1 isn't too bad. The tools could
>> limit the range supported by the --date option to avoid needing to use the
>> modification time in the extended field data.
>>
>> -Alan.
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>

Re: 8276766: Discuss options for deterministic jar/jmod timestamps across timezones

[Andrew Leonard]

Thanks Alan,
Having tried implementing a Zoned option, I agree it does seem to bloat
ZipEntry a bit.
As JohnN's suggestion pointed out we could at some point move to always
setting the extended
mtime(FileTime), but we have 78years until we really need to worry about
that! (xdostime covers to 2099)
It seems a conscious decision to only set xdostime was made for VM startup
optimization (see https://bugs.openjdk.java.net/browse/JDK-8073497), at
least until 2099.

I will update the PR based on option (1), which I have a commit which is
nearly at that point anyway.
I like your suggestion of restricting the --date option to 1980->2099

regards
Andrew


On Tue, Nov 30, 2021 at 1:13 PM Alan Bateman 
wrote:

> On 29/11/2021 19:25, Andrew Leonard wrote:
>
> *Problem:*
> PR https://github.com/openjdk/jdk/pull/6481
> addresses the main problems with deterministic timestamping of Zip entries,
> with
> the introduction of a new --date  option for jar & jmod.
> However, the ZipEntry timestamp is constructed from a combination of an
> MS-DOS timestamp
> and a FileTime(seconds since epoch). MS-DOS timestamp is used between
> 1980->2106, FileTime is
> used outside that range.
> The problem arises in deterministically setting the ZipEntry times within
> JVMs
> with different default system time-zones. This occurs because
> ZipEntry.setTime(long millisSinceEpoch) or
> setTimeLocal(LocalDateTime) are unaware of time-zones. So when setTime has
> to calculate the msdostime from
> millisSinceEpoch, it has to query the system time-zone to work out what the
> "date-time" is. Similarly, for
> setTimeLocal(LocalDateTime), if the LocalDateTime is outside 1980-2106
> range, then it has to create a
> FileTime, hence again it has to use the system time-zone to work out how
> many seconds since epoch
> the LocalDateTime is.
>
> *Solutions proposed so far:*
> 1. Take the --date  value and create a LocalDateTime
> eg.1982-12-09T14:02:06, and call
> ZipEntry.setTimeLocal(with LocalDateTime"1982-12-09T14:02:06"). For the
> same input and across time-zones
> the ZipEntry is deterministic when within the valid MS-DOS time range
> (1980->2106), because no extended
> FileTime is created.
> Pros:
> - No API changes, uses existing ZipEntry.setTimeLoca(LocalDateTime)
> Cons:
> - Is only determinstic between years 1980->2106, outside this range a
> FileTime is generated using the system
> time-zone.
>
> 2. Add new "Zoned" set/get methods to ZipEntry, so that ZipEntry does not
> have to defer to the system time-zone.
> By adding set/getTimeZoned(ZonedDateTime),
> set/getLastModifiedTimeZoned(FileTime, ZoneId) methods to ZipEntry
> the ZipEntry MS-DOS time and extended FileTime can be set precisely and
> deterministically to the user supplied
> date-time.
> Pros:
> - ZipEntry is always deterministically created for the same input --date
> , across different JVM
> time-zones, for all time ranges.
> Cons:
> - An API addition is required to ZipEntry to add new "Zoned" set/get time
> methods.
>
> Thoughts, comments, or alternatives most welcome please?
>
> ZipEntry already defines 3 methods to set the last modification time so I
> think we have to be cautious about adding additional methods. If we
> eventually do add a method to set the date-time with time zone then I think
> it will require broader work in the javadoc to help developers choose the
> right method to use.
>
> I don't think we should get hung up being able to set (via a CLI option)
> the time stamps to the 1970-01-01 epoch, it's just not interesting. For
> reproducible builds it should sufficient to set the timestamp to a fixed
> DOS date/time.
>
> So for the above options then I think #1 isn't too bad. The tools could
> limit the range supported by the --date option to avoid needing to use the
> modification time in the extended field data.
>
> -Alan.
>
>
>
>
>
>
>
>
>
>
>
>

RFR: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

[Jaikiran Pai]

Can I please get a review for this change which fixes a typo in the javadoc of 
`java.util.zip.ZipEntry.setTime()` method?

-

Commit messages:
 - 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

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

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

Re: RFR: 8277986: Typo in javadoc of java.util.zip.ZipEntry#setTime

[Alan Bateman]

On Tue, 30 Nov 2021 14:28:05 GMT, Jaikiran Pai  wrote:

> Can I please get a review for this change which fixes a typo in the javadoc 
> of `java.util.zip.ZipEntry.setTime()` method?

Marked as reviewed by alanb (Reviewer).

-

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

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

[Andrew Leonard]

On Mon, 29 Nov 2021 16:22:30 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 incrementally with two additional 
> commits since the last revision:
> 
>  - 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 

However, it looks like this behavior to not set extended mtime within the 
xdostime range has just been changed by a recent PR: 
https://github.com/openjdk/jdk/pull/5486/files which has changed jartool.Main 
using zipEntry.setTime(time) to use zipEntry.setLastModifiedTime(time), the 
later sets mtime always regardless of xdostime.
The implications of https://bugs.openjdk.java.net/browse/JDK-8073497 might also 
apply to FileTime unnecessary initialization slowing VM startup, however if 
FileTime is already regularly referenced during startup, then it wont.. If this 
is the case then way forward (1) would be ok...
@AlanBateman was that an intentional change? @jaikiran

-

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

Re: RFR: 8277924: Small tweaks to foreign function and memory API [v2]

[Maurizio Cimadamore]

On Mon, 29 Nov 2021 18:32:30 GMT, Maurizio Cimadamore  
wrote:

>> Following integration of the second incubator of the foreign function and 
>> memory API [1], we detected few divergences between the contents of the jdk 
>> repo and the panama repo:
>> 
>> * the name of some of the `FunctionDescriptor` wither methods is different 
>> (e.g. `withAppendedLayoutArguments` vs. `appendLayoutArguments`), as it has 
>> been simplified and improved following a change that was not incorporated in 
>> [1].
>> 
>> * TestUpcall does not execute all the test combinations, because of an issue 
>> in the jtreg header (also fixed in the panama repo)
>> 
>> * Addressing some feedback, we would like to bring back alignment to 
>> JAVA_INT layout constants (and related constants). 
>> 
>> Javadoc: 
>> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/javadoc/jdk/incubator/foreign/package-summary.html
>> Specdiff: 
>> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/spec_diff/overview-summary.html
>> 
>> [1] - #5907
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update 
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ValueLayout.java
>   
>   Co-authored-by: Jorn Vernee 

I've uploaded a new iteration, which drops the changes to value layout 
constants. The rationale behind this move is explained below:

https://mail.openjdk.java.net/pipermail/panama-dev/2021-November/015852.html

Javadoc: 
http://cr.openjdk.java.net/~mcimadamore/8277924/v2/javadoc/jdk/incubator/foreign/package-summary.html
Specdiff: 
http://cr.openjdk.java.net/~mcimadamore/8277924/v2/spec_diff/overview-summary.html

-

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

Re: RFR: 8277924: Small tweaks to foreign function and memory API [v3]

[Maurizio Cimadamore]

> Following integration of the second incubator of the foreign function and 
> memory API [1], we detected few divergences between the contents of the jdk 
> repo and the panama repo:
> 
> * the name of some of the `FunctionDescriptor` wither methods is different 
> (e.g. `withAppendedLayoutArguments` vs. `appendLayoutArguments`), as it has 
> been simplified and improved following a change that was not incorporated in 
> [1].
> 
> * TestUpcall does not execute all the test combinations, because of an issue 
> in the jtreg header (also fixed in the panama repo)
> 
> * Addressing some feedback, we would like to bring back alignment to JAVA_INT 
> layout constants (and related constants). 
> 
> Javadoc: 
> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/javadoc/jdk/incubator/foreign/package-summary.html
> Specdiff: 
> http://cr.openjdk.java.net/~mcimadamore/8277924/v1/spec_diff/overview-summary.html
> 
> [1] - #5907

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

 - Drop changes to alignment of layout constants
 - Simplify logic to access package-private method in ValueLayout

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6589/files
  - new: https://git.openjdk.java.net/jdk/pull/6589/files/54b89f30..e2dfb83b

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

  Stats: 255 lines in 6 files changed: 0 ins; 159 del; 96 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6589.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6589/head:pull/6589

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

Re: 8276766: Discuss options for deterministic jar/jmod timestamps across timezones

[Alan Bateman]

On 29/11/2021 19:25, Andrew Leonard wrote:

*Problem:*
PRhttps://github.com/openjdk/jdk/pull/6481
addresses the main problems with deterministic timestamping of Zip entries,
with
the introduction of a new --date  option for jar & jmod.
However, the ZipEntry timestamp is constructed from a combination of an
MS-DOS timestamp
and a FileTime(seconds since epoch). MS-DOS timestamp is used between
1980->2106, FileTime is
used outside that range.
The problem arises in deterministically setting the ZipEntry times within
JVMs
with different default system time-zones. This occurs because
ZipEntry.setTime(long millisSinceEpoch) or
setTimeLocal(LocalDateTime) are unaware of time-zones. So when setTime has
to calculate the msdostime from
millisSinceEpoch, it has to query the system time-zone to work out what the
"date-time" is. Similarly, for
setTimeLocal(LocalDateTime), if the LocalDateTime is outside 1980-2106
range, then it has to create a
FileTime, hence again it has to use the system time-zone to work out how
many seconds since epoch
the LocalDateTime is.

*Solutions proposed so far:*
1. Take the --date  value and create a LocalDateTime
eg.1982-12-09T14:02:06, and call
ZipEntry.setTimeLocal(with LocalDateTime"1982-12-09T14:02:06"). For the
same input and across time-zones
the ZipEntry is deterministic when within the valid MS-DOS time range
(1980->2106), because no extended
FileTime is created.
Pros:
- No API changes, uses existing ZipEntry.setTimeLoca(LocalDateTime)
Cons:
- Is only determinstic between years 1980->2106, outside this range a
FileTime is generated using the system
time-zone.

2. Add new "Zoned" set/get methods to ZipEntry, so that ZipEntry does not
have to defer to the system time-zone.
By adding set/getTimeZoned(ZonedDateTime),
set/getLastModifiedTimeZoned(FileTime, ZoneId) methods to ZipEntry
the ZipEntry MS-DOS time and extended FileTime can be set precisely and
deterministically to the user supplied
date-time.
Pros:
- ZipEntry is always deterministically created for the same input --date
, across different JVM
time-zones, for all time ranges.
Cons:
- An API addition is required to ZipEntry to add new "Zoned" set/get time
methods.

Thoughts, comments, or alternatives most welcome please?
ZipEntry already defines 3 methods to set the last modification time so 
I think we have to be cautious about adding additional methods. If we 
eventually do add a method to set the date-time with time zone then I 
think it will require broader work in the javadoc to help developers 
choose the right method to use.


I don't think we should get hung up being able to set (via a CLI option) 
the time stamps to the 1970-01-01epoch, it's just not interesting. For 
reproducible builds it should sufficient to set the timestamp to a fixed 
DOS date/time.


So for the above options then I think #1 isn't too bad. The tools could 
limit the range supported by the --date option to avoid needing to use 
the modification time in the extended field data.


-Alan.

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

[Andrew Leonard]

On Mon, 29 Nov 2021 16:22:30 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 incrementally with two additional 
> commits since the last revision:
> 
>  - 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 

I see maybe 2 ways forward:
1) We either prove always setting an mtime(FileTime) has no impact in JVM 
startup, and is an acceptable behavior change
2) We implement the original fix in 2 stages:
a) jdk-18: setTimeLocal() change, which provides deterministic behavior over 
years 1980-2106 (which is in keeping with the theme of JDK-8073497 !)
b) jdk-19+: provide ZipEntry Zoned time setting support

-

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

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

[Andrew Leonard]

On Tue, 30 Nov 2021 08:53:03 GMT, John Neffenger  wrote:

>> @andrew-m-leonard Thank you, Andrew, for your bold solution to Mark's 
>> request -- one that even solves the problem with the current `ZipEntry` API!
>> 
>> Would you be willing to consider a minor change to your implementation?
>> 
>> As I mentioned earlier, if we are going to allow an ISO 8601 zoned date and 
>> time, we need either to **truncate, restrict, or convert** its value. The 
>> current implementation **truncates,** discarding the time zone information. 
>> The better option might be to **convert.** That is, parse the ISO 8601 
>> string with `Instant.parse`, get the seconds since the epoch with 
>> `getEpochSecond`, and then proceed as before. Treat the value of `--date` as 
>> an instant on the time line, just like `SOURCE_DATE_EPOCH`, and always store 
>> its value in UTC.
>> 
>> This also solves the problem of storing dates before 1980 as a local date 
>> and time in UTC while storing those after 1980 as a local date and time with 
>> a discarded time zone. By converting instead of truncating, the value is 
>> always stored as the local date and time in UTC regardless of whether or not 
>> it's within the range of the "MS-DOS date and time" field.
>> 
>> Those of us using the timestamp of the last commit can still get the value 
>> directly from `git`.
>> 
>> For `SOURCE_DATE_EPOCH`, run:
>> 
>> 
>> $ git log -1 --pretty=%ct
>> 1638207366
>> 
>> 
>> For the `--date` option of the `jar` and `jmod` tools, run:
>> 
>> 
>> $ git log -1 --pretty=%cI
>> 2021-11-29T09:36:06-08:00
>> 
>> 
>> Specifically, that `git` ISO 8601 string, and even the sample `date` command 
>> that Mark shows in his comment, are what break the current implementation by 
>> creating archives that depend on the time zone of the build machine again.
>> 
>> By the way, the `jar` utility displays a time zone in the output of its 
>> `--list` and `--verbose` options even when there is no time zone information 
>> in the file. (In other words, it lies, or at least embellishes.) Use the 
>> verbose output of `zipinfo` to see the true values of the timestamps in an 
>> archive.
>
>> Would you be willing to consider a minor change to your implementation?
> 
> Just to be clear, this change should let us postpone any additions to the 
> `ZipEntry` API for another day, if at all, and maybe even get this pull 
> request integrated for JDK 18.

@jgneff Hi John, many thanks for the suggestions

So what you suggest sounds reasonable, although it would be a "behaviour 
change" in that whereas now if the date is between 1980->2106 only a xdostime 
is stored, we would now always be storing the additional "mtime" field which is 
a FileTime persisted as an int("seconds from epoch") in the zip file. I was 
trying to not change the binary semantics, but I actually think your suggestion 
makes sense.

I've done a bit of digging as to why the extended mtime is only stored outside 
the xdos date range, and found this bug: 
https://bugs.openjdk.java.net/browse/JDK-8073497
Which basically I think changed ZipEntry so it did not construct Date() objects 
from ZipEntries loaded during JVM startup, which it "implies" (although I can't 
see the evidence) improves startup performance.
There is an interesting comment here: 
https://github.com/openjdk/jdk/blame/739769c8fc4b496f08a92225a12d07414537b6c0/src/java.base/share/classes/java/util/zip/ZipEntry.java#L83

 * This establish an approximate high-bound value for DOS times in
 * milliseconds since epoch, used to enable an efficient but
 * sufficient bounds check to avoid generating extended last modified
 * time entries.
 *
 * Calculating the exact number is locale dependent, would require loading
 * TimeZone data eagerly, and would make little practical sense. Since DOS
 * times theoretically go to 2107 - with compatibility not guaranteed
 * after 2099 - setting this to a time that is before but near 2099
 * should be sufficient.

So if we were to always set the extended mtime field, then FileTime.from(long) 
would potentially cause the impact JDK-8073497 was trying to avoid...?
However, with this bug we seem to have not thought about deterministic 
behavior..."Calculating the exact number is locale dependent, would require 
loading TimeZone data eagerly, and would make little practical sense"

Thoughts?

@AlanBateman @LanceAndersen fyi

-

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

Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v2]

[Roman Kennke]

On Mon, 15 Nov 2021 21:35:06 GMT, Roman Kennke  wrote:

>> The caches in ObjectStreamClass basically map WeakReference to 
>> SoftReference, where the ObjectStreamClass also 
>> references the same Class. That means that the cache entry, and thus the 
>> class and its class-loader, will not get reclaimed, unless the GC determines 
>> that memory pressure is very high.
>> 
>> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
>> all its classes alive much longer than necessary: as soon as a ClassLoader 
>> (and all its classes) become unreachable, there is no point in retaining the 
>> stuff in OSC's caches.
>> 
>> The proposed change is to use WeakReference instead of SoftReference for the 
>> values in caches.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [x] tier3
>>  - [ ] tier4
>
> Roman Kennke has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Use ForceGC instead of System.gc()
>  - Convert test to testng

> A patch is worth a thousand words. Here's what I meant when I said this could 
> be elegantly solved with ClassValue:
> 
> [plevart@6e16e5e](https://github.com/plevart/jdk/commit/6e16e5e526c7f3d868b16543f2f3418c751068e4)
> 
> Note this is not tested. Just an idea.

Very nice!
I've merged your change, it passes the testcase, and I've also run tier1 
successfully. I'm testing it more. Do you want to take over? It's mostly your 
change now, anyway (except for the testcase). Or do you want me to finish it?

-

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

Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v3]

[Roman Kennke]

> The caches in ObjectStreamClass basically map WeakReference to 
> SoftReference, where the ObjectStreamClass also references 
> the same Class. That means that the cache entry, and thus the class and its 
> class-loader, will not get reclaimed, unless the GC determines that memory 
> pressure is very high.
> 
> However, this seems bogus, because that unnecessarily keeps ClassLoaders and 
> all its classes alive much longer than necessary: as soon as a ClassLoader 
> (and all its classes) become unreachable, there is no point in retaining the 
> stuff in OSC's caches.
> 
> The proposed change is to use WeakReference instead of SoftReference for the 
> values in caches.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2
>  - [x] tier3
>  - [ ] tier4

Roman Kennke 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 six additional commits since the 
last revision:

 - Merge remote-tracking branch 'jdk-plevart/JDK-8277072-peter' into JDK-8277072
 - Use ClassValue to solve JDK-8277072
 - Use ForceGC instead of System.gc()
 - Convert test to testng
 - Fix indentation of new testcase
 - 8277072: ObjectStreamClass caches keep ClassLoaders alive

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6375/files
  - new: https://git.openjdk.java.net/jdk/pull/6375/files/6f099c9c..2ed745b3

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

  Stats: 943784 lines in 2433 files changed: 495164 ins; 434236 del; 14384 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6375.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6375/head:pull/6375

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

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

[John Neffenger]

On Tue, 30 Nov 2021 08:31:54 GMT, John Neffenger  wrote:

> Would you be willing to consider a minor change to your implementation?

Just to be clear, this change should let us postpone any additions to the 
`ZipEntry` API for another day, if at all, and maybe even get this pull request 
integrated for JDK 18.

-

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

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

[John Neffenger]

On Mon, 29 Nov 2021 19:27:57 GMT, Andrew Leonard  wrote:

>>> @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 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.
>> 
>> I agree with Alan. We are too close to RDP 1 to consider this type of change 
>> for JDK 18 as we need more time to discuss, update the CSR, test (and we 
>> will need additional tests for this), and give it time to bake. IMHO this 
>> will need to go into JDK 19 assuming we move forward with changes similar to 
>> the latest commit
> 
> Thanks @LanceAndersen , @AlanBateman, i've just posted a discussion thread 
> here: 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/084150.html

@andrew-m-leonard Thank you, Andrew, for your bold solution to Mark's request 
-- one that even solves the problem with the current `ZipEntry` API!

Would you be willing to consider a minor change to your implementation?

As I mentioned earlier, if we are going to allow an ISO 8601 zoned date and 
time, we need either to **truncate, restrict, or convert** its value. The 
current implementation **truncates,** discarding the time zone information. The 
better option might be to **convert.** That is, parse the ISO 8601 string with 
`Instant.parse`, get the seconds since the epoch with `getEpochSecond`, and 
then proceed as before. Treat the value of `--date` as an instant on the time 
line, just like `SOURCE_DATE_EPOCH`, and always store its value in UTC.

This also solves the problem of storing dates before 1980 as a local date and 
time in UTC while storing those after 1980 as a local date and time with a 
discarded time zone. By converting instead of truncating, the value is always 
stored as the local date and time in UTC regardless of whether or not it's 
within the range of the "MS-DOS date and time" field.

Those of us using the timestamp of the last commit can still get the value 
directly from `git`.

For `SOURCE_DATE_EPOCH`, run:


$ git log -1 --pretty=%ct
1638207366


For the `--date` option of the `jar` and `jmod` tools, run:


$ git log -1 --pretty=%cI
2021-11-29T09:36:06-08:00


Specifically, that `git` ISO 8601 string, and even the sample `date` command 
that Mark shows in his comment, are what break the current implementation by 
creating archives that depend on the time zone of the build machine again.

By the way, the `jar` utility displays a time zone in the output of its 
`--list` and `--verbose` options even when there is no time zone information in 
the file. (In other words, it lies, or at least embellishes.) Use the verbose 
output of `zipinfo` to see the true values of the timestamps in an archive.

-

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