Re: RFR: 8188055: (ref) Add Reference::refersTo predicate [v6]

[David Holmes]

On Wed, 21 Oct 2020 02:28:30 GMT, Kim Barrett  wrote:

>> Finally returning to this review that was started in April 2020.  I've
>> recast it as a github PR.  I think the security concern raised by Gil
>> has been adequately answered.
>> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-April/029203.html
>> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-July/030401.html
>> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-August/030677.html
>> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-September/030793.html
>> 
>> Please review a new function: java.lang.ref.Reference.refersTo.
>> 
>> This function is needed to test the referent of a Reference object without
>> artificially extending the lifetime of the referent object, as may happen
>> when calling Reference.get.  Some garbage collectors require extending the
>> lifetime of a weak referent when accessed, in order to maintain collector
>> invariants.  Lifetime extension may occur with any collector when the
>> Reference is a SoftReference, as calling get indicates recent access.  This
>> new function also allows testing the referent of a PhantomReference, which
>> can't be accessed by calling get.
>> 
>> The new function uses native methods whose implementations are in the VM so
>> they can use the Access API.  It is the intent that these methods will be
>> intrinsified by optimizing compilers like C2 or graal, but that hasn't been
>> implemented yet.  Bear that in mind before rushing off to change existing
>> uses of Reference.get.
>> 
>> There are two native methods involved, one in Reference and an override in
>> PhantomReference, both package private in java.lang.ref. The reason for this
>> split is to simplify the intrinsification. This is a change from the version
>> from April 2020; that version had a single native method in Reference,
>> implemented using the ON_UNKNOWN_OOP_REF Access reference strength category.
>> However, adding support for that category in the compilers adds significant
>> implementation effort and complexity. Splitting avoids that complexity.
>> 
>> Testing:
>> mach5 tier1
>> Locally (linux-x64) verified the new test passes with various garbage 
>> collectors.
>
> Kim Barrett has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   improve wording in refersTo javadoc

Update looks good. Need to reflect the change in the CSR.

Thanks.
David

-

Marked as reviewed by dholmes (Reviewer).

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

Re: RFR: 8188055: (ref) Add Reference::refersTo predicate [v6]

[Kim Barrett]

> Finally returning to this review that was started in April 2020.  I've
> recast it as a github PR.  I think the security concern raised by Gil
> has been adequately answered.
> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-April/029203.html
> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-July/030401.html
> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-August/030677.html
> https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2020-September/030793.html
> 
> Please review a new function: java.lang.ref.Reference.refersTo.
> 
> This function is needed to test the referent of a Reference object without
> artificially extending the lifetime of the referent object, as may happen
> when calling Reference.get.  Some garbage collectors require extending the
> lifetime of a weak referent when accessed, in order to maintain collector
> invariants.  Lifetime extension may occur with any collector when the
> Reference is a SoftReference, as calling get indicates recent access.  This
> new function also allows testing the referent of a PhantomReference, which
> can't be accessed by calling get.
> 
> The new function uses native methods whose implementations are in the VM so
> they can use the Access API.  It is the intent that these methods will be
> intrinsified by optimizing compilers like C2 or graal, but that hasn't been
> implemented yet.  Bear that in mind before rushing off to change existing
> uses of Reference.get.
> 
> There are two native methods involved, one in Reference and an override in
> PhantomReference, both package private in java.lang.ref. The reason for this
> split is to simplify the intrinsification. This is a change from the version
> from April 2020; that version had a single native method in Reference,
> implemented using the ON_UNKNOWN_OOP_REF Access reference strength category.
> However, adding support for that category in the compilers adds significant
> implementation effort and complexity. Splitting avoids that complexity.
> 
> Testing:
> mach5 tier1
> Locally (linux-x64) verified the new test passes with various garbage 
> collectors.

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

  improve wording in refersTo javadoc

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/498/files
  - new: https://git.openjdk.java.net/jdk/pull/498/files/ab4e519b..3a15b6a9

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

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

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

Re: RFR: 8188055: (ref) Add Reference::refersTo predicate [v5]

[Kim Barrett]

On Tue, 20 Oct 2020 16:35:02 GMT, Mandy Chung  wrote:

>>> @kimbarrett your reworded text is okay. I think "if it initially had some 
>>> other referent value" can be dropped.
>>> 
>>> For a `Reference` constructed with a `null` referent, we can clarify in the 
>>> spec that such reference object will never
>>> get cleared and enqueued. I suggest to file a separate issue to follow up.
>> 
>> I don't think that clause can be dropped, because of explicit clearing (by 
>> clear() or enqueue()) rather than by the
>> GC.  If the reference was constructed with a null referent, 
>> ref.refersTo(null) cannot tell whether ref.clear() has been
>> called.
>
>> Mandy's comment implied that references with a null referent never get 
>> enqueued. Otherwise when would they get
>> enqueued? There would be nothing to trigger it.
> 
> Sorry I should have been clearer.What I try to say is that 
> `Reference(null)`
> will never be cleared and enqueued by GC since its referent is `null`.
> 
> Kim is right that `Reference(null)` can be explicitly cleared and enqueued
> via `Reference::enqueue`.`Reference::clear` on such an "empty" reference
> object is essentially a no-op.Whoever creates an "empty" reference would
> not intend to be cleared.
> 
>> > But the more we discuss this the more I think allowing an initial null 
>> > referent was a mistake in the first place. :(
>> 
>> I agree, but here we are. Very hard to know what the compatibility impact of 
>> changing that would be.
> 
> There are existing use cases depending on `Reference(null)` for example as a 
> special
> instance be an empty reference or the head of a doubly-linked list of 
> references.
> This was discussed two years ago [1].
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2018-July/054325.html

David, Mandy, and I discussed the wording in refersTo javadoc and reached a 
consensus that is reflected in 3a15b6a.

-

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

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

[Maurizio Cimadamore]

> 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
> loading does _not_ depend on class loaders, so `LibraryLookup` is not subject 
> to the same 

Re: RFR: 8188055: (ref) Add Reference::refersTo predicate [v5]

[Mandy Chung]

On Tue, 20 Oct 2020 05:22:57 GMT, Kim Barrett  wrote:

>> @kimbarrett  your reworded text is okay.  I think "if it initially had some 
>> other referent value" can be dropped.
>> 
>> For a `Reference` constructed with a `null` referent, we can clarify in the 
>> spec that such reference object will never
>> get cleared and enqueued.  I suggest to file a separate issue to follow up.
>
>> @kimbarrett your reworded text is okay. I think "if it initially had some 
>> other referent value" can be dropped.
>> 
>> For a `Reference` constructed with a `null` referent, we can clarify in the 
>> spec that such reference object will never
>> get cleared and enqueued. I suggest to file a separate issue to follow up.
> 
> I don't think that clause can be dropped, because of explicit clearing (by 
> clear() or enqueue()) rather than by the
> GC.  If the reference was constructed with a null referent, 
> ref.refersTo(null) cannot tell whether ref.clear() has been
> called.

> Mandy's comment implied that references with a null referent never get 
> enqueued. Otherwise when would they get
> enqueued? There would be nothing to trigger it.

Sorry I should have been clearer.What I try to say is that `Reference(null)`
will never be cleared and enqueued by GC since its referent is `null`.
Kim is right that `Reference(null)` can be explicitly cleared and enqueued
via `Reference::enqueue`.`Reference::clear` on such an "empty" reference
object is essentially a no-op.

> > But the more we discuss this the more I think allowing an initial null 
> > referent was a mistake in the first place. :(
> 
> I agree, but here we are. Very hard to know what the compatibility impact of 
> changing that would be.

There are existing use cases depending on `Reference(null)` for example as a 
special
instance be an empty reference or the head of a doubly-linked list of 
references.
This was discussed two years ago [1].

[1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2018-July/054325.html

-

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

Re: RFR: 8254982: (tz) Upgrade time-zone data to tzdata2020c [v2]

[Naoto Sato]

On Tue, 20 Oct 2020 11:39:36 GMT, Kiran Sidhartha Ravikumar 
 wrote:

>> Hi Guys,
>> 
>> Please review the integration of tzdata2020c to JDK.
>> 
>> Details regarding the change can be viewed at - 
>> https://mm.icann.org/pipermail/tz-announce/2020-October/60.html
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8254982
>> 
>> Along with it, there is a test fix for 
>> https://bugs.openjdk.java.net/browse/JDK-8254865, With tzdata2020b, the test
>> fails for the mentioned zones expecting "PST" but JDK has Zone names for 
>> "MST" for JRE locale provider. Since the
>> purpose of the test is to get any GMT-08:00 time zone, I have excluded those 
>> zones from the test.  Please let me know
>> if the changes are good to push.
>> Thanks,
>> Kiran
>
> Kiran Sidhartha Ravikumar has updated the pull request with a new target base 
> due to a merge or a rebase. The
> incremental webrev excludes the unrelated changes brought in by the 
> merge/rebase. The pull request contains three
> additional commits since the last revision:
>  - 8254982: Adding comments for removal of zones in BasicDateTime.java
>  - Merge remote-tracking branch 'origin/master' into JDK-8254982
>  - 8254982: (tz) Upgrade time-zone data to tzdata2020c

Looks good. Thank you for the changes.

-

Marked as reviewed by naoto (Reviewer).

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

Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v3]

[Bernhard Urban-Forster]

> Use r18 as allocatable register on Linux only.
> 
> A bootstrap works now (it has been crashing before due to r18 being 
> allocated):
> $
> ./windows-aarch64-server-fastdebug/bin/java.exe 
> -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI 
> -version
> Bootstrapping JVMCI. in 17990 ms (compiled 
> 3330 methods)
> openjdk version "16-internal" 2021-03-16
> OpenJDK Runtime Environment (fastdebug build 
> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk)
> OpenJDK 64-Bit Server VM (fastdebug build 
> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode)
> 
> Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well.

Bernhard Urban-Forster 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 five additional commits
since the last revision:

 - add missing precompiled.hpp include
 - Merge remote-tracking branch 'upstream/master' into 
8254827-enable-jvmci-win-aarch64
 - rename argument to canUsePlatformRegister
 - comment for platformRegister
 - 8254827: JVMCI: Enable it for Windows+AArch64
   
   Use r18 as allocatable register on Linux only.
   
   A bootstrap works now (it has been crashing before due to r18 being 
allocated):
   ```console
   $
   ./windows-aarch64-server-fastdebug/bin/java.exe 
-XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI 
-version
   Bootstrapping JVMCI. in 17990 ms (compiled 
3330 methods)
   openjdk version "16-internal" 2021-03-16
   OpenJDK Runtime Environment (fastdebug build 
16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk)
   OpenJDK 64-Bit Server VM (fastdebug build 
16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode)
   ```
   
   Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/685/files
  - new: https://git.openjdk.java.net/jdk/pull/685/files/28dcf572..7e6cb739

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

  Stats: 29566 lines in 423 files changed: 18920 ins; 8788 del; 1858 mod
  Patch: https://git.openjdk.java.net/jdk/pull/685.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/685/head:pull/685

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

Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v2]

[Bernhard Urban-Forster]

On Mon, 19 Oct 2020 11:03:46 GMT, Magnus Ihse Bursie  wrote:

>> Bernhard Urban-Forster has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - rename argument to canUsePlatformRegister
>>  - comment for platformRegister
>
> Build changes look good, but you'll need a review on the hotspot part as well.

Thank you for your comments @dougxc 

-

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

Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64 [v2]

[Bernhard Urban-Forster]

> Use r18 as allocatable register on Linux only.
> 
> A bootstrap works now (it has been crashing before due to r18 being 
> allocated):
> $
> ./windows-aarch64-server-fastdebug/bin/java.exe 
> -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI 
> -version
> Bootstrapping JVMCI. in 17990 ms (compiled 
> 3330 methods)
> openjdk version "16-internal" 2021-03-16
> OpenJDK Runtime Environment (fastdebug build 
> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk)
> OpenJDK 64-Bit Server VM (fastdebug build 
> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode)
> 
> Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well.

Bernhard Urban-Forster has updated the pull request incrementally with two 
additional commits since the last revision:

 - rename argument to canUsePlatformRegister
 - comment for platformRegister

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/685/files
  - new: https://git.openjdk.java.net/jdk/pull/685/files/593dfdd6..28dcf572

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

  Stats: 18 lines in 2 files changed: 9 ins; 3 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/685.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/685/head:pull/685

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

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

[Maurizio Cimadamore]

On Tue, 20 Oct 2020 12:15:23 GMT, Jan Lahoda  wrote:

>> 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 with a new target base due to a merge 
> or a rebase. The pull request now
> contains 35 commits:
>  - Merge branch 'JDK-8250768-dev' of https://github.com/lahodaj/jdk into 
> JDK-8250768
>  - More fixing tests.
>  - Fixing tests.
>  - Using unique sections for preview warning sections, as suggested.
>  - Merge branch 'master' into JDK-8250768
>  - Reflecting review comments.
>  - Fixing tests.
>  - Various cleanup.
>  - The Preview taglet is not needed anymore.
>  - There is not jdk.internal package anymore
>  - ... and 25 more: 
> https://git.openjdk.java.net/jdk/compare/98ec4a67...be1d8651

javac changes look good

-

Marked as reviewed by mcimadamore (Reviewer).

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

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

[Jan Lahoda]

On Fri, 16 Oct 2020 16:47:25 GMT, Erik Joelsson  wrote:

>> Jan Lahoda has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now
>> contains 35 commits:
>>  - Merge branch 'JDK-8250768-dev' of https://github.com/lahodaj/jdk into 
>> JDK-8250768
>>  - More fixing tests.
>>  - Fixing tests.
>>  - Using unique sections for preview warning sections, as suggested.
>>  - Merge branch 'master' into JDK-8250768
>>  - Reflecting review comments.
>>  - Fixing tests.
>>  - Various cleanup.
>>  - The Preview taglet is not needed anymore.
>>  - There is not jdk.internal package anymore
>>  - ... and 25 more: 
>> https://git.openjdk.java.net/jdk/compare/98ec4a67...be1d8651
>
> Build changes look good.

I've updated the patch based on the comments: mostly updating the anchors in 
the javadoc, but also removing the updates
to JavacParser, which are only loosely related to the primary focus of this 
patch, and should possibly be done
separately.

-

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

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

[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 with a new target base due to a merge 
or a rebase. The pull request now
contains 35 commits:

 - Merge branch 'JDK-8250768-dev' of https://github.com/lahodaj/jdk into 
JDK-8250768
 - More fixing tests.
 - Fixing tests.
 - Using unique sections for preview warning sections, as suggested.
 - Merge branch 'master' into JDK-8250768
 - Reflecting review comments.
 - Fixing tests.
 - Various cleanup.
 - The Preview taglet is not needed anymore.
 - There is not jdk.internal package anymore
 - ... and 25 more: https://git.openjdk.java.net/jdk/compare/98ec4a67...be1d8651

-

Changes: https://git.openjdk.java.net/jdk/pull/703/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=703=01
  Stats: 3206 lines in 156 files changed: 2515 ins; 409 del; 282 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: 8254982: (tz) Upgrade time-zone data to tzdata2020c [v2]

[Kiran Sidhartha Ravikumar]

> Hi Guys,
> 
> Please review the integration of tzdata2020c to JDK.
> 
> Details regarding the change can be viewed at - 
> https://mm.icann.org/pipermail/tz-announce/2020-October/60.html
> Bug: https://bugs.openjdk.java.net/browse/JDK-8254982
> 
> Along with it, there is a test fix for 
> https://bugs.openjdk.java.net/browse/JDK-8254865, With tzdata2020b, the test
> fails for the mentioned zones expecting "PST" but JDK has Zone names for 
> "MST" for JRE locale provider. Since the
> purpose of the test is to get any GMT-08:00 time zone, I have excluded those 
> zones from the test.  Please let me know
> if the changes are good to push.
> Thanks,
> Kiran

Kiran Sidhartha Ravikumar has updated the pull request with a new target base 
due to a merge or a rebase. The
incremental webrev excludes the unrelated changes brought in by the 
merge/rebase. The pull request contains three
additional commits since the last revision:

 - 8254982: Adding comments for removal of zones in BasicDateTime.java
 - Merge remote-tracking branch 'origin/master' into JDK-8254982
 - 8254982: (tz) Upgrade time-zone data to tzdata2020c

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/747/files
  - new: https://git.openjdk.java.net/jdk/pull/747/files/7bd51537..7434663a

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

  Stats: 26902 lines in 293 files changed: 17268 ins; 8054 del; 1580 mod
  Patch: https://git.openjdk.java.net/jdk/pull/747.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/747/head:pull/747

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

Re: RFR: 8254982: (tz) Upgrade time-zone data to tzdata2020c [v2]

[Kiran Sidhartha Ravikumar]

On Mon, 19 Oct 2020 19:38:57 GMT, Naoto Sato  wrote:

>> Kiran Sidhartha Ravikumar has updated the pull request with a new target 
>> base due to a merge or a rebase. The
>> incremental webrev excludes the unrelated changes brought in by the 
>> merge/rebase. The pull request contains three
>> additional commits since the last revision:
>>  - 8254982: Adding comments for removal of zones in BasicDateTime.java
>>  - Merge remote-tracking branch 'origin/master' into JDK-8254982
>>  - 8254982: (tz) Upgrade time-zone data to tzdata2020c
>
> test/jdk/java/util/Formatter/BasicDateTime.java line 1695:
> 
>> 1693: list.remove("America/Dawson");
>> 1694: list.remove("America/WhiteHorse");
>> 1695: list.remove("Canada/Yukon");
> 
> I'd explicitly mention why these time zones are removed from the test.

Thanks, Naoto, I have added the information.

-

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

Re: RFR: 8254827: JVMCI: Enable it for Windows+AArch64

[Doug Simon]

On Thu, 15 Oct 2020 15:00:47 GMT, Bernhard Urban-Forster  
wrote:

> Use r18 as allocatable register on Linux only.
> 
> A bootstrap works now (it has been crashing before due to r18 being 
> allocated):
> $
> ./windows-aarch64-server-fastdebug/bin/java.exe 
> -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler -XX:+BootstrapJVMCI 
> -version
> Bootstrapping JVMCI. in 17990 ms (compiled 
> 3330 methods)
> openjdk version "16-internal" 2021-03-16
> OpenJDK Runtime Environment (fastdebug build 
> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk)
> OpenJDK 64-Bit Server VM (fastdebug build 
> 16-internal+0-adhoc.NORTHAMERICAbeurba.openjdk-jdk, mixed mode)
> 
> Jtreg tests `test/hotspot/jtreg/compiler/jvmci` are passing as well.

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java
line 126:

> 124: public static final Register metaspaceMethodRegister = r12;
> 125:
> 126: public static final Register platformRegister = r18;

There should be a comment here as "platform register" is rather ambiguous.

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java
line 133:

> 131: private static final RegisterArray reservedRegisters = new 
> RegisterArray(rscratch1, rscratch2, threadRegister,
> fp, lr, r31, zr, sp); 132:
> 133: private static RegisterArray initAllocatable(Architecture arch, 
> boolean reserveForHeapBase, boolean linuxOs) {

Instead of `linuxOs`, `canUsePlatformRegister` is a better name. The logic of 
which OS does what belongs more in
AArch64HotSpotJVMCIBackendFactory.

-

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

Re: JDK14 cross-compile to arm64 fails with linking error

[Andrew Haley]

Hi,

On 19/10/2020 03:53, Choe, Jiwon wrote:

> I'm trying to cross-compile JDK 14 to target aarch64-linux-gnu, from a
> 64-bit x86 Linux (Ubuntu 18.04).
> 
> I followed the steps described in the documentation (
> https://hg.openjdk.java.net/jdk/jdk14/raw-file/tip/doc/building.html#creating-and-using-sysroots-with-qemu-deboostrap),
> but when I try to make the target image, I get the following error:
> 
> /usr/lib/gcc-cross/aarch64-linux-gnu/7/../../../../aarch64-linux-gnu/bin/ld:
> /opt/sysroot/usr/lib/aarch64-linux-gnu/libm.a(e_pow.o): relocation
> R_AARCH64_ADR_PREL_PG_HI21 against symbol `__stack_chk_guard@@GLIBC_2.17'
> which may bind externally can not be used when making a shared object;
> recompile with -fPIC
> /usr/lib/gcc-cross/aarch64-linux-gnu/7/../../../../aarch64-linux-gnu/bin/ld:
> /opt/sysroot/usr/lib/aarch64-linux-gnu/libm.a(e_pow.o)(.text+0xdc):

^ This looks wrong. You should be linking with /usr/whatever/libm.so, not
/usr/whatever/libm.a. Is libm.so missing from that directory?

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Integrated: 8254282: Add Linux x86_32 builds to submit workflow

[Aleksey Shipilev]

On Mon, 12 Oct 2020 12:46:57 GMT, Aleksey Shipilev  wrote:

> Building x86_32 usually exposes 32/64 cleanliness problems early. Since 
> 32-bit builds break often, it might make sense
> to add them to submit workflow. The x86_32 might not be widely deployed by 
> itself, but 32/64 bit cleanliness detects
> bugs that manifest on ARM32 early.

This pull request has now been integrated.

Changeset: 5d1397fa
Author:Aleksey Shipilev 
URL:   https://git.openjdk.java.net/jdk/commit/5d1397fa
Stats: 99 lines in 1 file changed: 99 ins; 0 del; 0 mod

8254282: Add Linux x86_32 builds to submit workflow

Reviewed-by: erikj, rwestberg

-

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