Re: RFR: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently [v2]

[Naoto Sato]

> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6598/files
  - new: https://git.openjdk.java.net/jdk/pull/6598/files/8fee6741..ecbedb09

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

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

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

Re: RFR: 8190748: java/text/Format/DateFormat/DateFormatTest.java and NonGregorianFormatTest fail intermittently [v2]

[Naoto Sato]

On Mon, 29 Nov 2021 21:09:47 GMT, Lance Andersen  wrote:

>> 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
>
> test/jdk/java/text/Format/DateFormat/DateFormatTest.java line 349:
> 
>> 347: var defZone = ZoneId.systemDefault();
>> 348: if 
>> (defZone.getRules().getTransition(date1.toInstant().atZone(defZone).toLocalDateTime())
>>  != null) {
>> 349: logln("At the offset transition. Round trip test skipped.");
> 
> Should the message be more inlined with the test name vs indicate Round trip 
> test is skipped?

I changed the test not to skip even at the overlap, by explicitly giving the 
zone pattern. Unfortunately, this cannot be applied to the other location in 
`NonGregorianFormatTest`, because the argument is a bare `DateFormat`.

-

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

Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v10]

[iaroslavski]

> Sorting:
> 
> - adopt radix sort for sequential and parallel sorts on int/long/float/double 
> arrays (almost random and length > 6K)
> - fix tryMergeRuns() to better handle case when the last run is a single 
> element
> - minor javadoc and comment changes
> 
> Testing:
> - add new data inputs in tests for sorting
> - add min/max/infinity values to float/double testing
> - add tests for radix sort

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

  JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort)
  
  * Updated javadoc
  * Optimized insertion sort threshold
  * Refactored parallel sorting section
  * Improved step for pivot candidates
  * Changed condition for Radix sort

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3938/files
  - new: https://git.openjdk.java.net/jdk/pull/3938/files/4baa9a39..41b92f67

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3938&range=09
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3938&range=08-09

  Stats: 128 lines in 2 files changed: 14 ins; 48 del; 66 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3938.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3938/head:pull/3938

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

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

[Lance Andersen]

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.

test/jdk/java/text/Format/DateFormat/DateFormatTest.java line 349:

> 347: var defZone = ZoneId.systemDefault();
> 348: if 
> (defZone.getRules().getTransition(date1.toInstant().atZone(defZone).toLocalDateTime())
>  != null) {
> 349: logln("At the offset transition. Round trip test skipped.");

Should the message be more inlined with the test name vs indicate Round trip 
test is skipped?

-

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

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

[Joe Wang]

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.

Marked as reviewed by joehw (Reviewer).

-

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

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

[Roger Riggs]

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.

Marked as reviewed by rriggs (Reviewer).

-

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

RFR: 8277957: Add test group for IPv6 exclusive testing

[Ivan Šipka]

Adding test group for IPv6 exclusive testing.

-

Commit messages:
 - 8277957: add IPv6 test group

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

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

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

[Iris Clark]

On Mon, 29 Nov 2021 18:38:35 GMT, Joe Darcy  wrote:

>> 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 incrementally with one additional 
> commit since the last revision:
> 
>   Update symbol information for JDK 18 b25.

Marked as reviewed by iris (Reviewer).

-

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

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

[Andrew Leonard]

Given an API-change is not realistic at this point in jdk-18, would it
make more sense to implement solution (1), and consider (2) for jdk-19+.. ?

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

[Andrew Leonard]

On Mon, 29 Nov 2021 17:07:52 GMT, Alan Bateman  wrote:

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

> > > @AlanBateman 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

-

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

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

[Andrew Leonard]

*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?

Thanks
Andrew

Re: Adding an @Immutable annotation to Java

[Alan Snyder]

> On Nov 29, 2021, at 10:42 AM, Simon Roberts  
> wrote:
> 
> I will say that as Java provides more and more features modeled on more
> functional languages, I'm finding more and more people struggling with the
> inherently mutable nature of Java, and that I strongly believe that
> something that helps bridge the gap is likely to be crucial to the
> language's continued improvement and growth. Clearly it can't turn into
> Haskell, nor should it, but without saying what, may I add my voice to
> those looking for this kind of feature.
> 

I agree with this sentiment.

> One thing that I haven't seen discussed is the fact that Java has had the
> "const" keyword since the beginning, but it's never been implemented. That
> raises the question of whether it's more practical (in terms of bridging
> styles) for methods to guarantee that they don't change some object (i.e.
> implement const) or for a data type to be declared in a way that it cannot
> be mutated (i.e. the @Immutable discussion). Given the difficulties raised
> already in attempts to create truly immutable lists, or perhaps read-only
> lists, and merge them in the existing library APIs, perhaps it's worth
> considering attacking the problem from the other side? Perhaps there's a
> way to declare a particular reference as "only to be used with const /
> non-mutating operations". Might the combination of those two features be
> worth consideration?

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.
Swift has solved this issue (as far as I can tell), but it has a fundamentally 
different semantics.

I think the solution is to abandon the goal of fully integrating immutable 
collections into the existing collections API.
Yes, that creates API bloat, but it may be worth it.
I’ve done that in my own programming and have no regrets.

There are precedents: NIO Paths vs Files, Streams vs Iterators, Modules vs 
Classpaths.
To be fair, I don’t use Paths or Streams or Modules, because the payoff seems 
small.
The payoff for immutable collections is much larger, in my opinion (less buggy 
code).

  Alan

Re: RFR: JDK-8277375: jdeps errors on a class path with a file path with no permission

[Mandy Chung]

On Wed, 24 Nov 2021 12:32:32 GMT, Michael Hall  wrote:

> Would there be any need to scan class path at all? That would mean a module 
> would have a class path dependency wouldn't it?

One reason of scanning the class path is to detect any split packages and emit 
warnings.   

> Yes, I shouldn't of included -cp at all but shouldn't keeps ignore it?

In general, a module should not depend on any class on the classpath.   
However, it's possible that there is any missing dependency i.e. can't be found 
from its required modules.   One could use `-cp` to add additional libraries 
for a quick analysis.

I got your point though.   Scanning class path is not strictly necessary to 
analyze dependencies and it can be done lazily.   Finding all packages in the 
unnamed module is just the current implementation.  I'll consider and look into 
making it lazy.

-

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

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

[Lance Andersen]

On Mon, 29 Nov 2021 17:07:52 GMT, Alan Bateman  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.

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

-

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

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

[Jorn Vernee]

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 

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ValueLayout.java 
line 619:

> 617:  */
> 618: public static final OfDouble JAVA_DOUBLE = new 
> OfDouble(ByteOrder.nativeOrder())
> 619: .withBitAlignment(ADDRESS_SIZE_BITS);

The code here is still using `ADDRESS_SIZE_BITS` (same for JAVA_LONG), instead 
of `64` as the javadoc says. Not sure that is intended?

-

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

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

[Naoto Sato]

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

-

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

Changes: https://git.openjdk.java.net/jdk/pull/6598/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6598&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8190748
  Stats: 21 lines in 2 files changed: 14 ins; 2 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6598.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6598/head:pull/6598

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

Re: Adding an @Immutable annotation to Java

[Simon Roberts]

I will say that as Java provides more and more features modeled on more
functional languages, I'm finding more and more people struggling with the
inherently mutable nature of Java, and that I strongly believe that
something that helps bridge the gap is likely to be crucial to the
language's continued improvement and growth. Clearly it can't turn into
Haskell, nor should it, but without saying what, may I add my voice to
those looking for this kind of feature.

One thing that I haven't seen discussed is the fact that Java has had the
"const" keyword since the beginning, but it's never been implemented. That
raises the question of whether it's more practical (in terms of bridging
styles) for methods to guarantee that they don't change some object (i.e.
implement const) or for a data type to be declared in a way that it cannot
be mutated (i.e. the @Immutable discussion). Given the difficulties raised
already in attempts to create truly immutable lists, or perhaps read-only
lists, and merge them in the existing library APIs, perhaps it's worth
considering attacking the problem from the other side? Perhaps there's a
way to declare a particular reference as "only to be used with const /
non-mutating operations". Might the combination of those two features be
worth consideration?

Cheers,
Simon


On Mon, Nov 29, 2021 at 11:06 AM Alberto Otero Rodríguez <
albest...@hotmail.com> wrote:

> I have created myself a project with a possible implementation of
> Immutable Collections using the source code of Java 17.
>
> https://github.com/Aliuken/JavaImmutableCollections
> [
> https://opengraph.githubassets.com/b1aeb5022614cc86e9ca7b0effe8a6298ce1ba62fee0b97021af3ff40e505cea/Aliuken/JavaImmutableCollections
> ]
> GitHub - Aliuken/JavaImmutableCollections<
> https://github.com/Aliuken/JavaImmutableCollections>
> Contribute to Aliuken/JavaImmutableCollections development by creating an
> account on GitHub.
> github.com
>
> 
> De: Alberto Otero Rodríguez 
> Enviado: jueves, 25 de noviembre de 2021 9:39
> Para: Justin Dekeyser 
> Cc: core-libs-dev@openjdk.java.net 
> Asunto: RE: Adding an @Immutable annotation to Java
>
> I have not thought about that. I'm not a Java expert.
>
> I just throwed the idea precisely to avoid "fake immutability", because a
> programmer could think one record is immutable simply by being a record,
> while this is false.
>
> There are probably lots of problems that need to be taken in consideration
> (like inheritance). But I just throw the idea because I think it would be a
> desirable feature.
>
> I would be grateful if some expert could deepen in possible problems and
> their solutions.
>
> Regards,
>
> Alberto Otero Rodríguez.
> 
> De: Justin Dekeyser 
> Enviado: jueves, 25 de noviembre de 2021 9:27
> Para: Alberto Otero Rodríguez 
> Cc: core-libs-dev@openjdk.java.net 
> Asunto: Re: Adding an @Immutable annotation to Java
>
> Hello,
>
> Quick question, out of curiosity: how would it behave with respect to
> inheritance? Can a @Immutable class inherit from an non immutable one?
> if no: that's a bit annoying, no? (fake immutability)
> Can @Immutable class be subclassed to a non @Immutable one? if no:
> that's a bit annoying too, no? (downcasting)
>
> Since Object is the super class of everything, it sounds like a
> problem. What have you thought about to handle this concern?
>
> Regards,
>
> Justin Dekeyser
>
> On Thu, Nov 25, 2021 at 9:08 AM Alberto Otero Rodríguez
>  wrote:
> >
> > Hi, I was thinking that it could be interesting adding an @Immutable
> annotation to Java. It would be a marker annotation for the compiler
> (similar to @FunctionalInterface) in order to throw an error if the
> annotated class/record has a component that is not @Immutable.
> >
> > This means that all existing immutable objects (like the
> primitive-wrapping objects and String) should be annotated with @Immutable
> and the programmer could, for example, annotate a new record object with
> @Immutable only if all its fields are annotated with @Immutable.
> >
> > What do you think?
> >
> > Regards,
> >
> > Alberto Otero Rodríguez.
>


-- 
Simon Roberts
(303) 249 3613

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

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

  Update symbol information for JDK 18 b25.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6493/files
  - new: https://git.openjdk.java.net/jdk/pull/6493/files/5c36a2ef..53e46dec

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6493&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6493&range=03-04

  Stats: 85 lines in 5 files changed: 62 ins; 22 del; 1 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: 8277924: Small tweaks to foreign function and memory API [v2]

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

  Update 
src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ValueLayout.java
  
  Co-authored-by: Jorn Vernee 

-

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

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

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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: 8277924: Small tweaks to foreign function and memory API [v2]

[Maurizio Cimadamore]

On Mon, 29 Nov 2021 13:26:11 GMT, Jorn Vernee  wrote:

>> 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 
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/FunctionDescriptor.java
>  line 134:
> 
>> 132:  * layout array of this function descriptor.
>> 133:  * @param index the index at which to insert the arguments
>> 134:  * @param addedLayouts the argument layouts to append.
> 
> Suggestion:
> 
>  * @param addedLayouts the argument layouts to insert.

Fixed.

-

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

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

[Maurizio Cimadamore]

On Mon, 29 Nov 2021 11:22:45 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

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java
 line 1337:

> 1335:  */
> 1336: @ForceInline
> 1337: default char getAtIndex(ValueLayout.OfChar layout, long index) {

We need to thread lightly here. The alignment check is not free, 
performance-wise, because of [1]. So, if we can detect that the var handle will 
always be accessed at offsets that are multiple of the alignment, we can use 
the more optimized "aligned" var handle, which will skip the alignment check. 
Eventually this special casing will go away when [1] is fixed.

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

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MemoryAddressImpl.java
 line 273:

> 271: public char getAtIndex(ValueLayout.OfChar layout, long index) {
> 272: Reflection.ensureNativeAccess(Reflection.getCallerClass());
> 273: if (layout.byteAlignment() <= layout.byteSize()) {

Here we also have to workaround [1]. Since we don't want to slice the 
everything segment (as that would create an additional instance), the only 
option is to use the var handle directly, but to use the "aligned" version if 
we detect that the offset is always aligned. One caveat is that we need to 
manually check the base address for alignment, since this check will be omitted 
otherwise. Again, this logic will go away when [1] is fixed.

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

-

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

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

[Jorn Vernee]

On Mon, 29 Nov 2021 11:22:45 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

Marked as reviewed by jvernee (Reviewer).

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

> 132:  * layout array of this function descriptor.
> 133:  * @param index the index at which to insert the arguments
> 134:  * @param addedLayouts the argument layouts to append.

Suggestion:

 * @param addedLayouts the argument layouts to insert.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ValueLayout.java 
line 205:

> 203: 
> 204: static final int ALIGNED_POS = 0;
> 205: static final int UNALIGNED_POS = 1;

Suggestion:

private static final int ALIGNED_POS = 0;
private static final int UNALIGNED_POS = 1;

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/Utils.java line 
179:

> 177: 
> 178: @Stable
> 179: public static LayoutAccess LAYOUT_ACCESS;

Clever :) Maybe some comment here about what this is doing would be nice (I had 
to look at it for a while).
Suggestion:

public static LayoutAccess LAYOUT_ACCESS; // shared secret


Alternatively, I think you could do this with a MethodHandle acquired through 
`Lookup.privateLookupIn(lookup(), ValueLayout.class).findVirtual(...)`.

-

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

RFR: 8277924: Small tweaks to foreign function and memory API

[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

-

Commit messages:
 - Further tweak to javadoc of layout constants
 - Wrong alignment constraints in ValueLayout constants javadoc
 - Tweak javadoc
 - Fix jtreg header on TestUpcall
 - Tweak API names for FunctionDescriptor methods
 - * Tweak value layout constants alignment to reflect VM alignment

Changes: https://git.openjdk.java.net/jdk/pull/6589/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6589&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8277924
  Stats: 343 lines in 14 files changed: 206 ins; 5 del; 132 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: Adding an @Immutable annotation to Java

[Alberto Otero Rodríguez]

I have created myself a project with a possible implementation of Immutable 
Collections using the source code of Java 17.

https://github.com/Aliuken/JavaImmutableCollections
[https://opengraph.githubassets.com/b1aeb5022614cc86e9ca7b0effe8a6298ce1ba62fee0b97021af3ff40e505cea/Aliuken/JavaImmutableCollections]
GitHub - 
Aliuken/JavaImmutableCollections
Contribute to Aliuken/JavaImmutableCollections development by creating an 
account on GitHub.
github.com


De: Alberto Otero Rodríguez 
Enviado: jueves, 25 de noviembre de 2021 9:39
Para: Justin Dekeyser 
Cc: core-libs-dev@openjdk.java.net 
Asunto: RE: Adding an @Immutable annotation to Java

I have not thought about that. I'm not a Java expert.

I just throwed the idea precisely to avoid "fake immutability", because a 
programmer could think one record is immutable simply by being a record, while 
this is false.

There are probably lots of problems that need to be taken in consideration 
(like inheritance). But I just throw the idea because I think it would be a 
desirable feature.

I would be grateful if some expert could deepen in possible problems and their 
solutions.

Regards,

Alberto Otero Rodríguez.

De: Justin Dekeyser 
Enviado: jueves, 25 de noviembre de 2021 9:27
Para: Alberto Otero Rodríguez 
Cc: core-libs-dev@openjdk.java.net 
Asunto: Re: Adding an @Immutable annotation to Java

Hello,

Quick question, out of curiosity: how would it behave with respect to
inheritance? Can a @Immutable class inherit from an non immutable one?
if no: that's a bit annoying, no? (fake immutability)
Can @Immutable class be subclassed to a non @Immutable one? if no:
that's a bit annoying too, no? (downcasting)

Since Object is the super class of everything, it sounds like a
problem. What have you thought about to handle this concern?

Regards,

Justin Dekeyser

On Thu, Nov 25, 2021 at 9:08 AM Alberto Otero Rodríguez
 wrote:
>
> Hi, I was thinking that it could be interesting adding an @Immutable 
> annotation to Java. It would be a marker annotation for the compiler (similar 
> to @FunctionalInterface) in order to throw an error if the annotated 
> class/record has a component that is not @Immutable.
>
> This means that all existing immutable objects (like the primitive-wrapping 
> objects and String) should be annotated with @Immutable and the programmer 
> could, for example, annotate a new record object with @Immutable only if all 
> its fields are annotated with @Immutable.
>
> What do you think?
>
> Regards,
>
> Alberto Otero Rodríguez.

Re: RFR: 8177819: DateTimeFormatterBuilder zone parsing should recognise DST [v3]

[Naoto Sato]

On Thu, 25 Nov 2021 08:26:56 GMT, Stephen Colebourne  
wrote:

>> Naoto Sato has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Replaced integer literals.
>>  - Refined wording #2
>
> src/java.base/share/classes/java/time/format/DateTimeFormatter.java line 508:
> 
>> 506:  * of {@link ChronoLocalDateTime#atZone(ZoneId)}. If the {@code ZoneId} 
>> was
>> 507:  * parsed from the zone name that does not indicate daylight saving 
>> time, then
>> 508:  * the standard offset will be used at the local time-line overlap.
> 
> As written, I would read it as being the generic zone name that gets altered, 
> rather than a zone name that explicitly indicates "winter" time. maybe:
> 
> "If the {@code ZoneId} was parsed from a zone name that indicates whether 
> daylight saving time in in operation or not, then that fact will be used to 
> select the correct offset at the local time-line overlap."

Modified as suggested.

> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 4906:
> 
>> 4904: private static class LENIENT extends CI {
>> 4905: 
>> 4906: private LENIENT(String k, String v, int t, PrefixTree 
>> child) {
> 
> Is class `LENIENT` actually in use?

Removed.

-

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

Re: RFR: 8177819: DateTimeFormatterBuilder zone parsing should recognise DST [v4]

[Naoto Sato]

> This fix intends to honor the type (std/dst/generic) of parsed zone name for 
> selecting the offset at the overlap. Corresponding CSR has also been drafted.

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

  Refined wording. Removed `LENIENT` parser. Added tests for CI.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6527/files
  - new: https://git.openjdk.java.net/jdk/pull/6527/files/89c894ae..2be302f3

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

  Stats: 97 lines in 3 files changed: 12 ins; 83 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6527.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6527/head:pull/6527

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

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

[John Neffenger]

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 

This is just a quick comment on the epoch date and time string. I'm testing 
your updates on the JavaFX builds now and will follow up with the results and 
more comments later today. Thanks, Andrew.

src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 88:

> 86: date {0} is not a valid ISO 8601 date and time
> 87: error.date.before.epoch=\
> 88: date {0} is before Epoch 1970-01-01T00:00:00

The epoch in Java is a [zoned date and time][1] (1970-01-01T00:00:00Z).

[1]: 
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/Instant.html#EPOCH

-

Changes requested by jgneff (no project role).

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

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

[Alan Bateman]

On Mon, 29 Nov 2021 16:18:44 GMT, Alan Bateman  wrote:

>> 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 
>
> @andrew-m-leonard I see you've add several APIs to ZipEntry in this PR. I 
> think this part will require discussion as it's not immediately clear that we 
> should over burden the ZipEntry API as proposed.

> @AlanBateman yes, see above comment, thanks

This is a significant change to the ZipEntry API that will require discussion 
and agreement. Can you start a discussion on core-libs-dev about the topic? You 
could start with a summary of the problem and the API and non-API options that 
have been explored.

-

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 Mon, 29 Nov 2021 16:18:44 GMT, Alan Bateman  wrote:

>> 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 
>
> @andrew-m-leonard I see you've add several APIs to ZipEntry in this PR. I 
> think this part will require discussion as it's not immediately clear that we 
> should over burden the ZipEntry API as proposed.

@AlanBateman yes, see above comment, thanks

-

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

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

[Andrew Leonard]

On Thu, 25 Nov 2021 19:21:36 GMT, John Neffenger  wrote:

>>> A user who’s not familiar with the lingo of [reproducible 
>>> builds](https://reproducible-builds.org/docs/source-date-epoch/) will be 
>>> mystified by an option named `--source-date`. A user who just wants to set 
>>> the timestamp of new entries won’t be looking for an option whose name 
>>> includes “source.”
>>> 
>>> Please consider providing a more general option, say `--date`, which takes 
>>> an [ISO 8601 date/time 
>>> string](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/format/DateTimeFormatter.html#ISO_DATE_TIME).
>>>  That would solve the general problem while also satisfying the requirement 
>>> for reproducible builds. In the build it’s easy enough to convert the 
>>> seconds-since epoch value of `SOURCE_DATE_EPOCH` to an ISO 8601 string 
>>> (`date -d @$SOURCE_DATE_EPOCH --iso-8601=seconds`).
>> 
>> Thanks @mbreinhold , good point, i'll update to use --date=
>
>> Please consider providing a more general option, say `--date`, which takes 
>> an [ISO 8601 date/time 
>> string](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/format/DateTimeFormatter.html#ISO_DATE_TIME).
> 
> The `jar` and `jmod` tools will need to truncate, restrict, or convert the 
> time zone and fractional seconds permitted by the ISO 8601 date and time 
> format.
> 
> The only way I found to store a timestamp using the methods of 
> `java.util.zip.ZipEntry` that was independent of the system's time zone was 
> by passing a local date and time in UTC to `setTimeLocal(LocalDateTime)`, 
> truncated to seconds.
> 
> You can store a zoned date and time indirectly (in seconds) by adding an 
> extra extended timestamp field to each entry with 
> `setLastModifiedTime(FileTime)`. Unfortunately, that method also stores the 
> "MS-DOS date and time" as the local date and time in the default time zone of 
> the JVM (the time zone of the build machine). Furthermore, the extra field 
> added to each entry increases the size of the archive.
> 
> The beauty of the `SOURCE_DATE_EPOCH` value is that it avoids any confusion 
> in its interpretation while providing a straightforward solution to the only 
> users ever known to need it.

@jgneff @LanceAndersen As John has pointed out there is a fundamental issue 
with the current ZipEntry API, in that for the conversion to/from 
(setTime()/getTime()) MS-DOS dostime it has to infer the time-zone from the 
current JVM default, so it can then workout the millis since epoch. Using 
set/getLocalDateTime() is only a half solution, that will only work as long as 
there isn't an extended FileTime being stored in the ZipEntry, which will occur 
for years  <1980 or >2106.

My updated PR proposes an extension to the ZipEntry API (and thus the CSR) to 
handle Zoned times for set/get, with the addition of the following API 
extensions to ZipEntry:
void setTimeZoned(ZonedDateTime)
ZonedDateTime getTimeZoned(ZoneId)
ZipEntry setLastModifiedTimeZoned(FileTime, ZoneId)
FileTime getLastModifiedTimeZoned(ZoneId)

Your thoughts are appreciated please?
Thanks

-

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

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

[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/4deb80ea..c7928bfe

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=07
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=06-07

  Stats: 50 lines in 3 files changed: 40 ins; 1 del; 9 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: 8276766: Enable jar and jmod to produce deterministic timestamped content [v9]

[Alan Bateman]

On Mon, 29 Nov 2021 16:19:01 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 

@andrew-m-leonard I see you've add several APIs to ZipEntry in this PR. I think 
this part will require discussion as it's not immediately clear that we should 
over burden the ZipEntry API as proposed.

-

Changes requested by alanb (Reviewer).

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

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

[Andrew Leonard]

On Fri, 26 Nov 2021 16:24:31 GMT, Lance Andersen  wrote:

>> @LanceAndersen java File times can't be before the epoch, but having a test 
>> before dostime 1980 would be useful
>
>> 
> 
> The change to sun/tools/jar/GNUStyleOptions.java does not prevent a negative 
> value which can be set via ZipEntry similar to:
> 
> public void testOfEpochSecond() {
> var ze = new ZipEntry("test");
> for(var i =  0; i < 100; i++) {
> var time = LocalDateTime.ofEpochSecond(-i, 0, ZoneOffset.UTC);
> ze.setTimeLocal(time);
> System.out.printf(
> "time= %s, Zip Entry time= %s%n", time, 
> ze.getTimeLocal());
> }
> }
> 
> If the intent is to not support dates prior to the Epoch then GNUStyleOptions 
> should throw an Exception in this case.

Added more tests, and negative check

-

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

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

[Andrew Leonard]

On Fri, 26 Nov 2021 16:25:43 GMT, Lance Andersen  wrote:

>> For files with large number of entries, alot of LocalDateTime Objects are 
>> being created here. Would there be an efficiency gain by converting 
>> sourceDate variable to be of type LocalDateTime and simply pass it to the 
>> s.setTimeLocal call here. The LocalDateTime Object could be constructed at 
>> init time (may be null) and can be static etc. 
>> 
>> Same for jmod code ?
>
>> 
> 
> I think that is a reasonable suggestion

The ZonedDateTime is now resolved during parsing of the options.

-

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

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

[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 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 

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6481/files
  - new: https://git.openjdk.java.net/jdk/pull/6481/files/c7928bfe..6206c1ee

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=07-08

  Stats: 4 lines in 2 files changed: 0 ins; 0 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: 8276766: Enable jar and jmod to produce deterministic timestamped content [v7]

[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 eight 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 
 - 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 

-

Changes: https://git.openjdk.java.net/jdk/pull/6481/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=06
  Stats: 544 lines in 11 files changed: 515 ins; 0 del; 29 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: 8277072: ObjectStreamClass caches keep ClassLoaders alive [v2]

[Peter Levart]

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:

https://github.com/plevart/jdk/commit/6e16e5e526c7f3d868b16543f2f3418c751068e4

Note this is not tested. Just an idea.

-

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

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

[Peter Levart]

On Mon, 29 Nov 2021 13:47:19 GMT, Roman Kennke  wrote:

> I don't quite understand this: If the Class object is still reachable by the 
> app,
1. a weak reference would not get cleared and 
2. the Class's ClassLoader would not get unloaded.

...but the ObjectStreamClass instance could still get GC-ed, because it is held 
in the map using WeakReference. The fact that associated 
Class is still reachable does not mean that the ObjectStreamClass instance is!

-

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

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

> While it is true that when the Class object used in a weak key is not 
> reachable any more by the app, it is not sensible to hold on to the value any 
> longer so in that respect SoftReference is to "storng" of a weakness. But 
> while the Class object is still reachable by the app, the app expects to 
> obtain the ObjectStreamClass (the value) from the cache at least most of the 
> time. If you change the SoftReference into WeakReference, the 
> ObjectStreamClass might get GC-ed even while in the middle of stream 
> deserialization.

I don't quite understand this: If the Class object is still reachable by the 
app, 1. a weak reference would not get cleared and 2. the Class's ClassLoader 
would not get unloaded. Conversely, if it's not reachable by the app anymore, 
then the *key* in the cache would get cleared, and we would not find the 
ObjectStreamClass anyway. Except that the OSC holds onto the Class object by a 
SoftReference, so it would effectively prevent getting cleared (and get 
unloaded).

> ObjectStream class pre-dates java.lang.invoke (MethodHandles), so it uses its 
> own implementation of weak caching. But since MethodHandles, there is a class 
> called ClassValue that would solve these problem with more elegance, because 
> it ties the lifetime of the value (ObjectStreamClass) to the lifetime of the 
> Class key (Class object has a strong reference to the associated value) while 
> the Class key is only Weakly referenced.

Hmm, sounds nice. Do you think that would work in the context of OSC?

-

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

Re: RFR: 8277072: ObjectStreamClass caches keep ClassLoaders alive

[Peter Levart]

On Mon, 15 Nov 2021 19:10:17 GMT, Roman Kennke  wrote:

> > If the intent of this change is to alter the lifetimes of the objects in 
> > question in a meaningful way, I recommend a CSR for the behavioral 
> > compatibility impact.
> 
> It would be hard for application code to observe this change: before the 
> change, a ClassLoader and its classes could be lingering in the cache longer 
> than necessary, even if otherwise not reachable. With the change, they would 
> be reclaimed as soon as they become unreachable. This could only be observed, 
> if application code holds onto ClassLoader or Class instances via Weak or 
> PhantomReference, and even then I am not sure if that qualifies as 
> 'meaningful'.

Hi Roman,

What your patch changes is the following:

ConcurrentHashMap>, SoftReference>

into:

ConcurrentHashMap>, WeakReference>


While it is true that when the Class object used in a weak key is not 
reachable any more by the app, it is not sensible to hold on to the value any 
longer so in that respect SoftReference is to "storng" of a weakness. 
But while the Class object is still reachable by the app, the app expects to 
obtain the ObjectStreamClass (the value) from the cache at least most of the 
time. If you change the SoftReference into 
WeakReference, the ObjectStreamClass might get GC-ed even 
while in the middle of stream deserialization.

ObjectStream class pre-dates java.lang.invoke (MethodHandles), so it uses its 
own implementation of weak caching. But since MethodHandles, there is a class 
called ClassValue that would solve these problem with more elegance, because 
it ties the lifetime of the value (ObjectStreamClass) to the lifetime of the 
Class key (Class object has a strong reference to the associated value) 
while the Class key is only Weakly referenced.

-

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

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

Ping? Can I please get another review? Thanks!

-

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

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

[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/3768810f..7f24db78

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=04-05

  Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 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: 8276766: Enable jar and jmod to produce deterministic timestamped content [v5]

[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/6d372436..3768810f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6481&range=03-04

  Stats: 359 lines in 12 files changed: 280 ins; 14 del; 65 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: 8003417: WeakHashMap$HashIterator removes wrong entry

[Jaikiran Pai]

On Sat, 20 Nov 2021 10:08:41 GMT, Jaikiran Pai  wrote:

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

Any reviews for this change, please?

-

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

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

[Сергей Цыпанов]

> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6575/files
  - new: https://git.openjdk.java.net/jdk/pull/6575/files/8fa8242a..6744a562

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

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

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

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

[Сергей Цыпанов]

On Sat, 27 Nov 2021 22:50:55 GMT, Michael Bien  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8277868: Use Integer.signum() in BasicTableUI
>
> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java line 
> 249:
> 
>> 247: 
>> 248: private static int sign(int num) {
>> 249: return Integer.compare(num, 0);
> 
> => Integer.signum(num)

Done!

-

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