Re: RFR: 8255964: Add all details to jstack log in jtreg timeout handler [v2]

[Igor Ignatyev]

On Fri, 6 Nov 2020 20:25:13 GMT, Nils Eliasson  wrote:

>> This patch adds jcmd Thread.print to the jtreg timeout handler.
>> 
>> Please review.
>
> Nils Eliasson has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add extended printing to jstack

Marked as reviewed by iignatyev (Reviewer).

-

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

Re: RFR: 8247781: Day periods support [v9]

[Stephen Colebourne]

On Thu, 5 Nov 2020 23:24:19 GMT, Stephen Colebourne  
wrote:

>> I implemented what you suggested here in the latest PR, but that would be a 
>> behavioral change which requires a CSR, as "AM" would be resolved to 06:00 
>> which was not before. Do you think it would be acceptable? If so, I will 
>> reopen the CSR and describe the change. (In fact some TCK failed with this 
>> impl)
>
> I find the whole "half way between the start and end" behaviour of day 
> periods odd anyway, but if it is to be supported then it should be consistent 
> as you've implemented. 
> 
> Another option I should have thought of earlier would be to simply not 
> support the "half way between the start and end" behaviour of LDML in either 
> dayPeriod or AM/PM. But since LDML defines it, you've implemented it, and it 
> isn't overly harmful I think its OK to leave it in.
> 
> Would it be possible for STRICT mode to not have the "half way between the 
> start and end" behaviour?

I've had a look tonight, but found two more problems!

The comments at the start of `resolveTimeLenient()` indicate that setting the 
midpoint in `resolveTimeFields()` is a bad idea - it needs to be done inside 
`resolveTimeLenient()`. (This ensures user-defined fields can resolve to 
ChronoFields IIRC). Thus, the day period resolving would have to be split 
between the two methods. How important is the midpoint logic? Could it just be 
dropped?

Secondly, we need to ensure that "24:00 midnight" (using HOUR_OF_DAY only) 
correctly parses to the end of day midnight, not the start of day or an 
exception. This is related to the problem above. 

I _think_ (but haven't confirmed everything yet) that the only thing that 
should be in `resolveTimeFields()` is the resolution of HOUR_OF_AMPM + 
dayPeriod to HOUR_OF_DAY (with `dayPeriod` then being set to `null`). 
Everything else should go in `resolveTimeLenient()` - the midpoint logic in the 
first if block (where time fields are defaulted), and the validation logic at 
the end of the method.

-

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

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v15]

[Coleen Phillimore]

On Thu, 5 Nov 2020 21:26:16 GMT, Maurizio Cimadamore  
wrote:

>> This patch contains the changes associated with the first incubation round 
>> of the foreign linker access API incubation
>> (see JEP 389 [1]). This work is meant to sit on top of the foreign memory 
>> access support (see JEP 393 [2] and associated pull request [3]).
>> 
>> The main goal of this API is to provide a way to call native functions from 
>> Java code without the need of intermediate JNI glue code. In order to do 
>> this, native calls are modeled through the MethodHandle API. I suggest 
>> reading the writeup [4] I put together few weeks ago, which illustrates what 
>> the foreign linker support is, and how it should be used by clients.
>> 
>> Disclaimer: the pull request mechanism isn't great at managing *dependent* 
>> reviews. For this reasons, I'm attaching a webrev which contains only the 
>> differences between this PR and the memory access PR. I will be periodically 
>> uploading new webrevs, as new iterations come out, to try and make the life 
>> of reviewers as simple as possible.
>> 
>> A big thank to Jorn Vernee and Vladimir Ivanov - they are the main 
>> architects of all the hotspot changes you see here, and without their help, 
>> the foreign linker support wouldn't be what it is today. As usual, a big 
>> thank to Paul Sandoz, who provided many insights (often by trying the bits 
>> first hand).
>> 
>> Thanks
>> Maurizio
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/webrev
>> 
>> Javadoc:
>> 
>> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/javadoc/jdk/incubator/foreign/package-summary.html
>> 
>> Specdiff (relative to [3]):
>> 
>> http://cr.openjdk.java.net/~mcimadamore/8254231_v1/specdiff_delta/overview-summary.html
>> 
>> CSR:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8254232
>> 
>> 
>> 
>> ### API Changes
>> 
>> The API changes are actually rather slim:
>> 
>> * `LibraryLookup`
>>   * This class allows clients to lookup symbols in native libraries; the 
>> interface is fairly simple; you can load a library by name, or absolute 
>> path, and then lookup symbols on that library.
>> * `FunctionDescriptor`
>>   * This is an abstraction that is very similar, in spirit, to `MethodType`; 
>> it is, at its core, an aggregate of memory layouts for the function 
>> arguments/return type. A function descriptor is used to describe the 
>> signature of a native function.
>> * `CLinker`
>>   * This is the real star of the show. A `CLinker` has two main methods: 
>> `downcallHandle` and `upcallStub`; the first takes a native symbol (as 
>> obtained from `LibraryLookup`), a `MethodType` and a `FunctionDescriptor` 
>> and returns a `MethodHandle` instance which can be used to call the target 
>> native symbol. The second takes an existing method handle, and a 
>> `FunctionDescriptor` and returns a new `MemorySegment` corresponding to a 
>> code stub allocated by the VM which acts as a trampoline from native code to 
>> the user-provided method handle. This is very useful for implementing 
>> upcalls.
>>* This class also contains the various layout constants that should be 
>> used by clients when describing native signatures (e.g. `C_LONG` and 
>> friends); these layouts contain additional ABI classfication information (in 
>> the form of layout attributes) which is used by the runtime to *infer* how 
>> Java arguments should be shuffled for the native call to take place.
>>   * Finally, this class provides some helper functions e.g. so that clients 
>> can convert Java strings into C strings and back.
>> * `NativeScope`
>>   * This is an helper class which allows clients to group together logically 
>> related allocations; that is, rather than allocating separate memory 
>> segments using separate *try-with-resource* constructs, a `NativeScope` 
>> allows clients to use a _single_ block, and allocate all the required 
>> segments there. This is not only an usability boost, but also a performance 
>> boost, since not all allocation requests will be turned into `malloc` calls.
>> * `MemorySegment`
>>   * Only one method added here - namely `handoff(NativeScope)` which allows 
>> a segment to be transferred onto an existing native scope.
>> 
>> ### Safety
>> 
>> The foreign linker API is intrinsically unsafe; many things can go wrong 
>> when requesting a native method handle. For instance, the description of the 
>> native signature might be wrong (e.g. have too many arguments) - and the 
>> runtime has, in the general case, no way to detect such mismatches. For 
>> these reasons, obtaining a `CLinker` instance is a *restricted* operation, 
>> which can be enabled by specifying the usual JDK property 
>> `-Dforeign.restricted=permit` (as it's the case for other restricted method 
>> in the foreign memory API).
>> 
>> ### Implementation changes
>> 
>> The Java changes associated with `LibraryLookup` are relative 
>> straightforward; the only interesting thing to note here is that library 

Re: RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v15]

[Coleen Phillimore]

On Fri, 6 Nov 2020 21:47:42 GMT, Coleen Phillimore  wrote:

>> Maurizio Cimadamore has updated the pull request with a new target base due 
>> to a merge or a rebase. The pull request now contains 64 commits:
>> 
>>  - Merge branch '8254162' into 8254231_linker
>>  - Fix post-merge issues caused by 8219014
>>  - Merge branch 'master' into 8254162
>>  - Addess remaining feedback from @AlanBateman and @mrserb
>>  - Address comments from @AlanBateman
>>  - Fix typo in upcall helper for aarch64
>>  - Merge branch '8254162' into 8254231_linker
>>  - Merge branch 'master' into 8254162
>>  - Fix issues with derived buffers and IO operations
>>  - More 32-bit fixes for TestLayouts
>>  - ... and 54 more: 
>> https://git.openjdk.java.net/jdk/compare/a50fdd54...b38afb3f
>
> src/hotspot/cpu/aarch64/universalUpcallHandler_aarch64.cpp line 81:
> 
>> 79: #endif
>> 80: 
>> 81:   Method* method = k->lookup_method(mname_sym, mdesc_sym);
> 
> This "method" appears unused.

This should be moved into javaClasses or common code.  resolve_or_null only 
resolves the class, it doesn't also call the initializer for the class so you 
shouldn't be able to call a static method on the class.

-

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

Re: RFR: 8247781: Day periods support [v9]

[Naoto Sato]

> Hi,
> 
> Please review the changes for the subject issue. This is to enhance the 
> java.time package to support day periods, such as "in the morning", defined 
> in CLDR. It will add a new pattern character 'B' and its supporting builder 
> method. The motivation and its spec are in this CSR:
> 
> https://bugs.openjdk.java.net/browse/JDK-8254629
> 
> Naoto

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

  Fixed a comment.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/938/files
  - new: https://git.openjdk.java.net/jdk/pull/938/files/8054ce6b..3a9ea64a

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

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

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

RFR: 8233332: Need to create exploded tests covering all forms of modules

[Alexey Semenyuk]

822: Need to create exploded tests covering all forms of modules

-

Commit messages:
 - 822: Need to create exploded tests covering all forms of modules
 - 822: Need to create exploded tests covering all forms of modules

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

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

Re: RFR: 8255989: Remove explicitly unascribed authorship from Java source files

[Iris Clark]

On Fri, 6 Nov 2020 20:11:24 GMT, Pavel Rappo  wrote:

> This PR proposes to remove
> 1. JavaDoc `@author` tags with unclear semantics: `@author 
> unascribed|unattributed|unknown`
> 2. A couple of astray Form Feed (a.k.a. FF, `\f`,  `0xC`, or `^L`) characters

Marked as reviewed by iris (Reviewer).

-

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

Re: RFR: 8255989: Remove explicitly unascribed authorship from Java source files

[Mandy Chung]

On Fri, 6 Nov 2020 20:11:24 GMT, Pavel Rappo  wrote:

> This PR proposes to remove
> 1. JavaDoc `@author` tags with unclear semantics: `@author 
> unascribed|unattributed|unknown`
> 2. A couple of astray Form Feed (a.k.a. FF, `\f`,  `0xC`, or `^L`) characters

Marked as reviewed by mchung (Reviewer).

-

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

Re: RFR: 8255989: Remove explicitly unascribed authorship from Java source files

[Mark Reinhold]

On Fri, 6 Nov 2020 20:11:24 GMT, Pavel Rappo  wrote:

> This PR proposes to remove
> 1. JavaDoc `@author` tags with unclear semantics: `@author 
> unascribed|unattributed|unknown`
> 2. A couple of astray Form Feed (a.k.a. FF, `\f`,  `0xC`, or `^L`) characters

Marked as reviewed by mr (Lead).

-

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

Re: RFR: 8247781: Day periods support [v7]

[Naoto Sato]

On Thu, 5 Nov 2020 23:49:25 GMT, Stephen Colebourne  
wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fixed typo/grammatical error.
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 5055:
> 
>> 5053: @Override
>> 5054: public boolean format(DateTimePrintContext context, 
>> StringBuilder buf) {
>> 5055: Long value = context.getValue(MINUTE_OF_DAY);
> 
> This does not match the spec: " During formatting, the day period is obtained 
> from {@code HOUR_OF_DAY}, and optionally {@code MINUTE_OF_HOUR} if exist"
> 
> It is possible and legal to create a Temporal that returns `HOUR_OF_DAY` and 
> `MINUTE_OF_HOUR` but not `MINUTE_OF_DAY`. As such, this method must be 
> changed to follow the spec.
> 
> -
> 
> In addition, it is possible for `HOUR_OF_DAY` and `MINUTE_OF_HOUR` to be 
> outside their normal bounds. The right behaviour would be to combine the two 
> fields within this method, and then use mod to get the value into the range 0 
> to 1440 before calling `dayPeriod.include`. (While the fall back behaviour 
> below does cover this, it would be better to do what I propose here.)
> 
> An example of this is a `TransportTime` class where the day runs from 03:00 
> to 27:00 each day (because trains run after midnight for no extra cost to the 
> passenger, and it is more convenient for the operator to treat the date that 
> way). A `TransportTime` of 26:30 should still resolve to "night1" rather than 
> fall back to "am".

Fixed.

-

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

Re: RFR: 8247781: Day periods support [v8]

[Naoto Sato]

> Hi,
> 
> Please review the changes for the subject issue. This is to enhance the 
> java.time package to support day periods, such as "in the morning", defined 
> in CLDR. It will add a new pattern character 'B' and its supporting builder 
> method. The motivation and its spec are in this CSR:
> 
> https://bugs.openjdk.java.net/browse/JDK-8254629
> 
> Naoto

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

  Addressed the following comments:
  - https://github.com/openjdk/jdk/pull/938#discussion_r518431077
  - https://github.com/openjdk/jdk/pull/938#discussion_r518616570
  - https://github.com/openjdk/jdk/pull/938#discussion_r518439782

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/938/files
  - new: https://git.openjdk.java.net/jdk/pull/938/files/b0649899..8054ce6b

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

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

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

Re: RFR: 8247781: Day periods support [v7]

[Naoto Sato]

On Fri, 6 Nov 2020 09:12:38 GMT, Stephen Colebourne  
wrote:

>> Did you mean in STRICT mode, HOUR_OF_AMPM should default to 0, and to 6 in 
>> SMART/LENIENT modes?
>
> No. I mean that when resolving AMPM/dayPeriod in strict mode, and there is no 
> HOUR_OF_DAY or HOUR_OF_AMPM, then do not resolve using "half way between"(ie. 
> fail). This will produce a result where `LocalTime` cannot be obtained.
> 
> var f = 
> DateTimeFormatter.ofPattern("B").withResolverStyle(ResolverStyle.STRICT);
> var t = LocalTime.from("at night", f);
> would throw an exception in STRICT mode (whereas SMART or LENIENT would 
> return a `LocalTime`). Same with pattern "a".

Changed to throw an exception in STRICT mode.

-

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

Re: RFR: 8255989: Remove explicitly unascribed authorship from Java source files

[Claes Redestad]

On Fri, 6 Nov 2020 20:11:24 GMT, Pavel Rappo  wrote:

> This PR proposes to remove
> 1. JavaDoc `@author` tags with unclear semantics: `@author 
> unascribed|unattributed|unknown`
> 2. A couple of astray Form Feed (a.k.a. FF, `\f`,  `0xC`, or `^L`) characters

A removed @author tag is a good @author tag

-

Marked as reviewed by redestad (Reviewer).

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

Re: RFR: 8255964: Add jcmd Thread.print to jtreg timeout handler [v2]

[Nils Eliasson]

> This patch adds jcmd Thread.print to the jtreg timeout handler.
> 
> Please review.

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

  add extended printing to jstack

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1080/files
  - new: https://git.openjdk.java.net/jdk/pull/1080/files/2451660c..2ca47884

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

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

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

RFR: 8255989: Remove explicitly unascribed authorship from Java source files

[Pavel Rappo]

This PR proposes to remove
1. JavaDoc `@author` tags with unclear semantics: `@author 
unascribed|unattributed|unknown`
2. A couple of astray Form Feed (a.k.a. FF, `\f`,  `0xC`, or `^L`) characters

-

Commit messages:
 - Initial commit

Changes: https://git.openjdk.java.net/jdk/pull/1100/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1100=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8255989
  Stats: 140 lines in 77 files changed: 0 ins; 80 del; 60 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1100.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1100/head:pull/1100

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

Re: RFR: 8180352: Add Stream.toList() method

[Martin Desruisseaux]

Le 06/11/2020 à 19:00, Alan Snyder a écrit :
(…snip…) But a question that deserves ongoing review is whether Java 
should support immutable collections using a separate type hierarchy 
(…snip…).


Maybe an alternative way (admittedly more difficult) to have immutable 
collections without introducing new interfaces could be to take 
inspiration from the C++ "const" keyword? Especially since in Java, 
"const" is already a reserved keyword, just not used yet.


I realize that it would be a difficult task: how to handle private 
fields that are just caches in a const object; where to add the "const" 
keyword in existing JDK API; how to make such change in a backward 
compatible way (e.g. when a legacy code overrides a "const" method). I 
do not have any expectation. But given the inconvenient of alternatives, 
I wonder if there is some research about the long-term feasibility of a 
"const" semantic in Java? If it was the case, then maybe it would be 
better to not add immutable collection interfaces in Java for now.


    Regards

        Martin

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v11]

[Jan Lahoda]

> This is an update to javac and javadoc, to introduce support for Preview 
> APIs, and generally improve javac and javadoc behavior to more closely adhere 
> to JEP 12.
> 
> The notable changes are:
> 
>  * adding support for Preview APIs (javac until now supported primarily only 
> preview language features, and APIs associated with preview language 
> features). This includes:
>  * the @PreviewFeature annotation has boolean attribute "reflective", 
> which should be true for reflective Preview APIs, false otherwise. This 
> replaces the existing "essentialAPI" attribute with roughly inverted meaning.
>  * the preview warnings for preview APIs are auto-suppressed as described 
> in the JEP 12. E.g. the module that declares the preview API is free to use 
> it without warnings
>  * improving error/warning messages. Please see [1] for a list of 
> cases/examples.
>  * class files are only marked as preview if they are using a preview 
> feature. [1] also shows if a class file is marked as preview or not.
>  * the PreviewFeature annotation has been moved to jdk.internal.javac package 
> (originally was in the jdk.internal package).
>  * Preview API in JDK's javadoc no longer needs to specify @preview tag in 
> the source files. javadoc will auto-generate a note for @PreviewFeature 
> elements, see e.g. [2] and [3] (non-reflective and reflective API, 
> respectively). A summary of preview elements is also provided [4]. Existing 
> uses of @preview have been updated/removed.
>  * non-JDK javadoc is also enhanced to auto-generate preview notes for uses 
> of Preview elements, and for declaring elements using preview language 
> features [5].
>  
>  Please also see the CSR [6] for more information.
>  
>  [1] http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html
>  [2] 
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html
>  [3] 
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html
>  [4] 
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html
>  [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/
>  [6] https://bugs.openjdk.java.net/browse/JDK-8250769

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

  Fixing navigator for the PREVIEW page.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/703/files
  - new: https://git.openjdk.java.net/jdk/pull/703/files/097ae3c1..61264fe4

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

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

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

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]

[Jan Lahoda]

On Fri, 6 Nov 2020 15:32:16 GMT, Jonathan Gibbons  wrote:

>> FWIW, a javadoc generated with the current version of the patch:
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/index.html
>> 
>> And a specdiff comparing it to the javadoc built from the corresponding 
>> master:
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.specdiff.01/overview-summary.html
>
> The page and comments are generally good, but there is a bug that needs 
> to be fixed.
> 
> After clicking on the PREVIEW link in the top bar to go to the Preview 
> page, the word DEPRECATED is highlighted in the top navbar instead of 
> PREVIEW.
> 
> -- Jon
> 
> 
> On 11/5/20 10:13 AM, mlbridge[bot] wrote:
>>
>> /Mailing list message from Alex Buckley 
>>  on kulla-dev 
>> :/
>>
>> On 11/5/2020 4:45 AM, Jan Lahoda wrote:
>>
>> FWIW, a javadoc generated with the current version of the patch:
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/index.html
>>
>> Allow me to draw people's attention to the PREVIEW link in the banner of
>> the generated javadoc. It shows all the preview APIs in the release on
>> one page. This is very helpful for understanding the surface area of a
>> preview feature.
>>
>> For example, with Sealed Classes being the only preview feature likely
>> to target JDK 16, the PREVIEW page shows that the feature's API is
>> solely about reflection. It's clear that Sealed Classes do not
>> introduce, say, a java.lang.Sealed class analogous to the
>> java.lang.Record class introduced by Records in JDK 14/15 (and which
>> would have appeared on the PREVIEW page had it existed then).
>>
>> Alex
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub 
>> ,
>>  
>> or unsubscribe 
>> .
>>

Thanks for noticing Chris and @jonathan-gibbons. Fixed in 61264fe, updated 
javadoc:
http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.02/api/preview-list.html

-

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

Re: RFR: 8180352: Add Stream.toList() method

[Alan Snyder]

>> This discussion of unmodifiable lists brings me back to the thought that
>> there would be good client-side reasons for inserting an UnmodifiableList
>> interface as a parent of LIst

> On Nov 6, 2020, at 1:14 AM, Remi Forax  > wrote:
> 
> 
> This question is asked at least every six months since 1998
> https://docs.oracle.com/javase/8/docs/technotes/guides/collections/designfaq.html#a1
>  
> 

The question that Simon asked is not exactly the question that is answered in 
this link.

The question of whether List should be a subtype of (some kind of) 
ImmutableList is answered in
Stuart’s stack overflow answer (https://stackoverflow.com/a/57926310/1441122 
). The answer
is that it should not.

The question answered in the link is basically a straw man: why not capture 
every conceivable
semantic distinction in the collections type system. And the answer is, not 
surprisingly, that
there would be way too many types.

But a question that deserves ongoing review is whether Java should support 
immutable collections
using a separate type hierarchy. In other words, Immutable List would not be a 
subtype of List
and List would not be a subtype of Immutable List. The linked answer says:
"Adding this support to the type hierarchy requires four more interfaces.” Four 
additional
interfaces sounds like a small cost for a significant benefit. As Java evolves 
to better support
functional programming styles, immutable collections seems like an obvious next 
step.

Much could be written, and probably has been, about the disadvantages of basing 
the
collections framework on mutable collections. Let me just remark that in 
addition to the
already mentioned UnsupportedOperationException, there is also the more subtle
ConcurrentModificationException, both of which would be absent in fully 
immutable collections.

The lack of subtyping between List and ImmutableList is not terrible. Arrays 
coexist with
Lists with no subtyping relationship. java.io.File coexists with 
java.nio.filePath with no
subtyping relationship. It means that explicit conversions are required between 
List and
ImmutableList. Extra copying may be needed, but there are tricks for reducing 
copying, and the
need for defensive copying is removed.

  Alan

Re: RFR: 8180352: Add Stream.toList() method [v2]

[Stuart Marks]

> This change introduces a new terminal operation on Stream. This looks like a 
> convenience method for Stream.collect(Collectors.toList()) or 
> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this 
> method directly on Stream enables it to do what can't easily by done by a 
> Collector. In particular, it allows the stream to deposit results directly 
> into a destination array (even in parallel) and have this array be wrapped in 
> an unmodifiable List without copying.
> 
> In the past we've kept most things from the Collections Framework as 
> implementations of Collector, not directly on Stream, whereas only 
> fundamental things (like toArray) appear directly on Stream. This is true of 
> most Collections, but it does seem that List is special. It can be a thin 
> wrapper around an array; it can handle generics better than arrays; and 
> unlike an array, it can be made unmodifiable (shallowly immutable); and it 
> can be value-based. See John Rose's comments in the bug report:
> 
> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
> 
> This operation is null-tolerant, which matches the rest of Streams. This 
> isn't specified, though; a general statement about null handling in Streams 
> is probably warranted at some point.
> 
> Finally, this method is indeed quite convenient (if the caller can deal with 
> what this operation returns), as collecting into a List is the most common 
> stream terminal operation.

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

  Merge ListNNullsAllowed into ListN. Update spec for Stream.toList.
  Add nulls-allowed empty list. Simplify indexOf/lastIndexOf.
  Reduce tests, add contains(null) tests.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1026/files
  - new: https://git.openjdk.java.net/jdk/pull/1026/files/3e05564d..cf849755

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

  Stats: 181 lines in 3 files changed: 16 ins; 117 del; 48 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1026.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1026/head:pull/1026

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

Re: RFR: 8254354: Add a withInvokeExactBehavior() VarHandle combinator [v13]

[Jorn Vernee]

> Hi,
> 
> This patch adds an asExact() combinator to VarHandle, that will return a new 
> VarHandle that performs exact type checks, similar to 
> MethodHandle::invokeExact, to help developers catch inexact VarHandle usage, 
> which can lead to performance degradation.
> 
> This is implemented using a boolean flag in VarForm. If the flag is set, the 
> exact type of the invocation is checked against the exact type in the 
> VarForm. If there is a mismatch, a WrongMethodTypeException is thrown.
> 
> Other than that, there is also an asGeneric() combinator added that does the 
> inverse operation (thanks to Rémi for the suggestion). I've also added The 
> `@Hidden` annotation to the VarHandleGuards methods, as well as a 
> type-checking helper method called from the generic invocation lambda form, 
> so that the stack trace we get points at the location where the VarHandle is 
> being used.
> 
> Thanks,
> Jorn
> 
> CSR link: https://bugs.openjdk.java.net/browse/JDK-8255375

Jorn Vernee 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 17 additional commits since the 
last revision:

 - Merge branch 'master' into Exact_VarHandle
 - Address review comments:
   - Fix typo
   - add specification about return value of hasInvokeExactBehaviour
 - s/an arity/and arity
 - behaviour -> behavior in javadoc as well
 - - behaviour -> behaviour
   - Return same VarHandle instance when instance is already exact/non-exact
 - Update Javadoc, and rename asExact and asGeneric to withInvokeExactBehaviour 
and withInvokeBehaviour
 - Merge branch 'master' into Exact_VarHandle
 - Use AccessType ordinal in guard checks instead of the AccessMode ordinal
 - Update accessModeType to use the AccessType ordinal directly.
 - Add benchmarks
 - ... and 7 more: https://git.openjdk.java.net/jdk/compare/b6a085d4...ea7c920c

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/843/files
  - new: https://git.openjdk.java.net/jdk/pull/843/files/e92bd30f..ea7c920c

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

  Stats: 15154 lines in 825 files changed: 8417 ins; 4076 del; 2661 mod
  Patch: https://git.openjdk.java.net/jdk/pull/843.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/843/head:pull/843

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

Re: RFR: 8254354: Add a withInvokeExactBehavior() VarHandle combinator [v9]

[Jorn Vernee]

On Fri, 30 Oct 2020 15:23:28 GMT, Paul Sandoz  wrote:

>> Jorn Vernee 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 12 additional 
>> commits since the last revision:
>> 
>>  - Update Javadoc, and rename asExact and asGeneric to 
>> withInvokeExactBehaviour and withInvokeBehaviour
>>  - Merge branch 'master' into Exact_VarHandle
>>  - Use AccessType ordinal in guard checks instead of the AccessMode ordinal
>>  - Update accessModeType to use the AccessType ordinal directly.
>>  - Add benchmarks
>>  - - Update javadoc
>>- Make isExact() public
>>  - Fixes failing tests, and enable verifier on Exact test
>>  - Fix whitespace
>>  - Comment out VarHandleGuards generator code
>>  - Makes exactness a property of a VarHandle, not a VarForm, since the 
>> latter are shared. Use handle.accessModeType to get the exact type of the 
>> VarHandle.
>>  - ... and 2 more: 
>> https://git.openjdk.java.net/jdk/compare/84ce7081...3c707bc7
>
> Looks good. There is just one difference between the spec and implementation. 
> The spec states: 
>> If this VarHandle already has invoke{-exact} behaviour this VarHandle is 
>> returned.
> 
> I prefer this behaviour, but feel free to update the spec if you like e.g. If 
> ... already has XXX then a new VH with exactly the same behaviour as this VH 
> is returned.
> 
> I just realized i used "behaviour", the english spelling. We should use the 
> US spelling, "behavior", which is more commonly used throughout the JDK 
> documentation.

@ChrisHegarty Thanks for the comments. I've implemented them.

-

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

Re: RFR: 8255964: Add jcmd Thread.print to jtreg timeout handler

[Nils Eliasson]

On Fri, 6 Nov 2020 15:53:35 GMT, Igor Ignatyev  wrote:

>> This patch adds jcmd Thread.print to the jtreg timeout handler.
>> 
>> Please review.
>
> Hi Nils,
> 
> It looks alright, but could you please elaborate on why we need it when there 
> is already `jstack` action?
> 
> — Igor

They have the same output - I didn't realize that. But it would be nice to add 
the extended output - so I am updating the PR.

-

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

Integrated: 8255969: Improve java/io/BufferedInputStream/LargeCopyWithMark.java using jtreg tags

[Brian Burkhalter]

On Thu, 5 Nov 2020 19:50:14 GMT, Brian Burkhalter  wrote:

> Please review this change which would replace the use of a system property 
> and a child process with updated jtreg tags.

This pull request has now been integrated.

Changeset: 727a69f5
Author:Brian Burkhalter 
URL:   https://git.openjdk.java.net/jdk/commit/727a69f5
Stats: 45 lines in 1 file changed: 1 ins; 24 del; 20 mod

8255969: Improve java/io/BufferedInputStream/LargeCopyWithMark.java using jtreg 
tags

Reviewed-by: naoto

-

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

Integrated: JDK-8254920: Application launched with jpackage produced .exe crashes JVM

[Andy Herrick]

On Thu, 29 Oct 2020 17:32:24 GMT, Andy Herrick  wrote:

> JVM

This pull request has now been integrated.

Changeset: 952abea4
Author:Andy Herrick 
URL:   https://git.openjdk.java.net/jdk/commit/952abea4
Stats: 115 lines in 4 files changed: 110 ins; 0 del; 5 mod

8254920: Application launched with jpackage produced .exe crashes JVM

Reviewed-by: asemenyuk, almatvee, kizune

-

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

Re: RFR: 8255964: Add jcmd Thread.print to jtreg timeout handler

[Igor Ignatyev]

On Thu, 5 Nov 2020 17:09:58 GMT, Nils Eliasson  wrote:

> This patch adds jcmd Thread.print to the jtreg timeout handler.
> 
> Please review.

Hi Nils,

It looks alright, but could you please elaborate on why we need it when there 
is already `jstack` action?

— Igor

-

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

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]

[Jonathan Gibbons]

On Thu, 5 Nov 2020 12:43:03 GMT, Jan Lahoda  wrote:

>> Thanks @jonathan-gibbons for your comments! I've tried to update the code 
>> based on them, mostly in 
>> https://github.com/lahodaj/jdk/commit/743f516c660b577035cdda4510a0bb97937fd9b2
>>  and 
>> https://github.com/lahodaj/jdk/commit/e4b02827998fc2e8f19f983aabfb3d720b03d111
>> 
>> A big chunk of the update is generalization of the deprecated and preview 
>> list builders and writers into a "summary" list builder and writer. These 
>> should also now handle records. For record components, those are a little 
>> tricky, as (AFAIK) can't currently have deprecation/preview-ness for them 
>> (and hence there is no good way to test any support for record components in 
>> these). But the summary build and writer are looking for record components 
>> and will fail in case a record component is sent into them.
>
> FWIW, a javadoc generated with the current version of the patch:
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/index.html
> 
> And a specdiff comparing it to the javadoc built from the corresponding 
> master:
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.specdiff.01/overview-summary.html

The page and comments are generally good, but there is a bug that needs 
to be fixed.

After clicking on the PREVIEW link in the top bar to go to the Preview 
page, the word DEPRECATED is highlighted in the top navbar instead of 
PREVIEW.

-- Jon


On 11/5/20 10:13 AM, mlbridge[bot] wrote:
>
> /Mailing list message from Alex Buckley 
>  on kulla-dev 
> :/
>
> On 11/5/2020 4:45 AM, Jan Lahoda wrote:
>
> FWIW, a javadoc generated with the current version of the patch:
> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/index.html
>
> Allow me to draw people's attention to the PREVIEW link in the banner of
> the generated javadoc. It shows all the preview APIs in the release on
> one page. This is very helpful for understanding the surface area of a
> preview feature.
>
> For example, with Sealed Classes being the only preview feature likely
> to target JDK 16, the PREVIEW page shows that the feature's API is
> solely about reflection. It's clear that Sealed Classes do not
> introduce, say, a java.lang.Sealed class analogous to the
> java.lang.Record class introduced by Records in JDK 14/15 (and which
> would have appeared on the PREVIEW page had it existed then).
>
> Alex
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub 
> ,
>  
> or unsubscribe 
> .
>

-

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

Re: RFR: JDK-8250768: javac should be adapted to changes in JEP 12 [v6]

[Chris Hegarty]

> On 5 Nov 2020, at 18:11, Alex Buckley  wrote:
> 
> On 11/5/2020 4:45 AM, Jan Lahoda wrote:
>> FWIW, a javadoc generated with the current version of the patch:
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/index.html
> 
> Allow me to draw people's attention to the PREVIEW link in the banner of the 
> generated javadoc. It shows all the preview APIs in the release on one page. 
> This is very helpful for understanding the surface area of a preview feature.
> 
> For example, with Sealed Classes being the only preview feature likely to 
> target JDK 16, the PREVIEW page shows that the feature's API is solely about 
> reflection. It's clear that Sealed Classes do not introduce, say, a 
> java.lang.Sealed class analogous to the java.lang.Record class introduced by 
> Records in JDK 14/15 (and which would have appeared on the PREVIEW page had 
> it existed then).

Very cool! ( I didn’t notice this until you pointed it out ;-) )

http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.01/api/preview-list.html
 


There appears to be a very minor bug; when I click on the PREVIEW link in the 
banner, the page that lists the preview API points loads, but the banner does 
not “highlight” that PREVIEW is “selected”, but rather it “highlights” 
DEPRECATED.

-Chris.

RFR: 8255982: Extend BasicJMapTest to test with different GC Heap

[Lin Zang]

The implementation of jmap tool depends on the implementation of object 
iteration by different GC heap.
This patch extend the BasicJMapTest to cover differet GC Heap.

-

Commit messages:
 - 8255982: Extend BasicJMapTest to test with different GC Heap

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

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

Re: RFR: 8247781: Day periods support [v7]

[Stephen Colebourne]

On Fri, 6 Nov 2020 03:00:52 GMT, Naoto Sato  wrote:

>> test/jdk/java/time/tck/java/time/format/TCKDateTimeParseResolver.java line 
>> 858:
>> 
>>> 856: return new Object[][]{
>>> 857: {STRICT, 0, LocalTime.of(6, 0), 0},
>>> 858: {STRICT, 1, LocalTime.of(18, 0), 1},
>> 
>> As mentioned in my other comment, this seems odd in STRICT mode.
>
> Did you mean in STRICT mode, HOUR_OF_AMPM should default to 0, and to 6 in 
> SMART/LENIENT modes?

No. I mean that when resolving AMPM/dayPeriod in strict mode, and there is no 
HOUR_OF_DAY or HOUR_OF_AMPM, then do not resolve using "half way between"(ie. 
fail). This will produce a result where `LocalTime` cannot be obtained.

var f = 
DateTimeFormatter.ofPattern("B").withResolverStyle(ResolverStyle.STRICT);
var t = LocalTime.from("at night", f);
would throw an exception in STRICT mode (whereas SMART or LENIENT would return 
a `LocalTime`). Same with pattern "a".

-

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

Re: RFR: 8180352: Add Stream.toList() method

[Remi Forax]

- Mail original -
> De: "Simon Roberts" 
> À: "core-libs-dev" 
> Envoyé: Jeudi 5 Novembre 2020 18:40:44
> Objet: Re: RFR: 8180352: Add Stream.toList() method

> At the risk of a can of worms, or at least of raising something that has
> long since been discussed and rejected...
> 
> This discussion of unmodifiable lists brings me back to the thought that
> there would be good client-side reasons for inserting an UnmodifiableList
> interface as a parent of LIst, not least because all our unmodifiable
> variants from the Collections.unmodifiableList proxy onward fail the Liskov
> substitution test for actually "being contract-fulfilling Lists".
> 
> Is this something that has been considered and rejected? (If so, is it easy
> to point me at the discussion, as I expect I'd find it fascinating). Is it
> worth considering, might it have some merit, or merely horrible
> unforeseen-by-me consequences to the implementation?

This question is asked at least every six months since 1998
https://docs.oracle.com/javase/8/docs/technotes/guides/collections/designfaq.html#a1

> 
> Cheers,
> Simon

regards,
Rémi Forax

> 
> 
> 
> On Thu, Nov 5, 2020 at 10:30 AM Stuart Marks 
> wrote:
> 
>> On Wed, 4 Nov 2020 23:03:02 GMT, Paŭlo Ebermann > 645859+ep...@openjdk.org> wrote:
>>
>> >> This change introduces a new terminal operation on Stream. This looks
>> like a convenience method for Stream.collect(Collectors.toList()) or
>> Stream.collect(Collectors.toUnmodifiableList()), but it's not. Having this
>> method directly on Stream enables it to do what can't easily by done by a
>> Collector. In particular, it allows the stream to deposit results directly
>> into a destination array (even in parallel) and have this array be wrapped
>> in an unmodifiable List without copying.
>> >>
>> >> In the past we've kept most things from the Collections Framework as
>> implementations of Collector, not directly on Stream, whereas only
>> fundamental things (like toArray) appear directly on Stream. This is true
>> of most Collections, but it does seem that List is special. It can be a
>> thin wrapper around an array; it can handle generics better than arrays;
>> and unlike an array, it can be made unmodifiable (shallowly immutable); and
>> it can be value-based. See John Rose's comments in the bug report:
>> >>
>> >>
>> https://bugs.openjdk.java.net/browse/JDK-8180352?focusedCommentId=14133065=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14133065
>> >>
>> >> This operation is null-tolerant, which matches the rest of Streams.
>> This isn't specified, though; a general statement about null handling in
>> Streams is probably warranted at some point.
>> >>
>> >> Finally, this method is indeed quite convenient (if the caller can deal
>> with what this operation returns), as collecting into a List is the most
>> common stream terminal operation.
>> >
>> > src/java.base/share/classes/java/util/ImmutableCollections.java line 199:
>> >
>> >> 197:  * safely reused as the List's internal storage, avoiding a
>> defensive copy. Declared
>> >> 198:  * with Object... instead of E... as the parameter type so
>> that varargs calls don't
>> >> 199:  * accidentally create an array of type other than Object[].
>> >
>> > Why would that be a problem? If the resulting list is immutable, then
>> the actual array type doesn't really matter, right?
>>
>> It's an implementation invariant that the internal array be Object[].
>> Having it be something other than Object[] can lead to subtle bugs. See
>> [JDK-6260652](https://bugs.openjdk.java.net/browse/JDK-6260652) for
>> example.
>>
>> -
>>
>> PR: https://git.openjdk.java.net/jdk/pull/1026
>>
> 
> 
> --
> Simon Roberts
> (303) 249 3613

Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod…

[Joel Borggrén-Franck]

On Fri, 6 Nov 2020 00:24:14 GMT, Hui Shi  wrote:

>> If we are changing NativeMethodAccessorImpl.invoke then we should probably 
>> do NativeConstructorAccessorImpl.newInstance at the same time. Also 
>> wondering if we should, while in the area, add "return acc.invoke(obj, 
>> args)" after setting the delegate so that it invokes the newly generated 
>> accessor.
>> 
>> Are there resource or other cases that you have observed where 
>> generateMethod fails and then succeeds in a subsequent call?
>> 
>> @cl4es Do you know of any startup tests that might be sensitive to the eager 
>> creating of a VarHandle?
>> 
>> I agree with @shipilev to test before the CAS.
>
>> If we are changing NativeMethodAccessorImpl.invoke then we should probably 
>> do NativeConstructorAccessorImpl.newInstance at the same time. 
> 
> Yes, NativeConstructorAccessorImpl should also apply this change.
> 
>> Also wondering if we should, while in the area, add "return acc.invoke(obj, 
>> args)" after setting the delegate so that it invokes the newly generated 
>> accessor.
>> 
> 
> Agree,  I see no harm to invoke generated method when it is aviable. 
> Should leave this to another patch?
> 
>> Are there resource or other cases that you have observed where 
>> generateMethod fails and then succeeds in a subsequent call?
>> 
> I have not seen exception/error happen in generate method yet.  But in case 
> it fails in some ways, try - catch - reset is added to make sure behavior is 
> same before/after with this change.
> 
>> @cl4es Do you know of any startup tests that might be sensitive to the eager 
>> creating of a VarHandle?
>> 
>> I agree with @shipilev to test before the CAS.
> 
> @AlanBateman 
> Thanks for you comments!

Are there any benchmarks to compare this accessor with the previous version in 
the presumably common case where there is no or very little contention? Edit to 
clarify: it is stated as "trivial" is this also measured somewhere?

-

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

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v2]

[Roland Westrelin]

On Thu, 5 Nov 2020 23:59:43 GMT, Dean Long  wrote:

> C2 changes look good, except for new code block in 
> inline_preconditions_checkIndex could use a comment.

Thanks for the review. I added a comment for this block and some other comments 
for the rest of this method.

-

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

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v3]

[Roland Westrelin]

> This change add 3 new methods in Objects:
> 
> public static long checkIndex(long index, long length)
> public static long checkFromToIndex(long fromIndex, long toIndex, long length)
> public static long checkFromIndexSize(long fromIndex, long size, long length)
> 
> This mirrors the int utility methods that were added by JDK-8135248
> with the same motivations.
> 
> As is the case with the int checkIndex(), the long checkIndex() method
> is JIT compiled as an intrinsic. It allows the JIT to compile
> checkIndex to an unsigned comparison and properly recognize it as
> a range check that then becomes a candidate for the existing range check
> optimizations. This has proven to be important for panama's
> MemorySegment API and a prototype of this change (with some extra c2
> improvements) showed that panama micro benchmark results improve
> significantly.
> 
> This change includes:
> 
> - the API change
> - the C2 intrinsic
> - tests for the API and the C2 intrinsic
> 
> This is a joint work with Paul who reviewed and reworked the API change
> and filled the CSR.

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

  intrinsic comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1003/files
  - new: https://git.openjdk.java.net/jdk/pull/1003/files/b47184ac..aaacd328

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

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

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

Re: RFR: 8255150: Add utility methods to check long indexes and ranges [v2]

[Roland Westrelin]

On Fri, 6 Nov 2020 04:25:40 GMT, Dean Long  wrote:

>> src/hotspot/share/opto/library_call.cpp line 1015:
>> 
>>> 1013:   Deoptimization::Action_make_not_entrant);
>>> 1014:   }
>>> 1015: 
>> 
>> A comment here explaining what the code below is doing would be helpful.
>
> This code wasn't here before, so I'm guessing it's needed for T_LONG.  For 
> T_INT is it just wasted work?

Code in IdealLoopTree::is_range_check_if() uses this check:
  if (range->Opcode() != Op_LoadRange && !iff->is_RangeCheck()) {
const TypeInt* tint = phase->_igvn.type(range)->isa_int();
if (tint == NULL || tint->empty() || tint->_lo < 0) {
  // Allow predication on positive values that aren't LoadRanges.
  // This allows optimization of loops where the length of the
  // array is a known value and doesn't need to be loaded back
  // from the array.
  return false;
that is it assumes that everything that's on the right hand size of the a 
RangeCheck test is positive. I think it's cleaner and less dangerous to 
explicitly cast the length to >= 0 when the intrinsic is built. In a subsequent 
patch I intend to drop the && !iff->is_RangeCheck() check above.

-

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