Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v5]

[Hui Shi]

On Thu, 12 Nov 2020 12:23:36 GMT, Alan Bateman  wrote:

>> @AlanBateman 
>> 
>>> What is the reason for using an int? I remember there was a suggestion for 
>>> three states but two states seems okay so curious why it was changed from 
>>> boolean to int.
>> 
>> shipilev suggested not to use sub-word CAS, so change compareAndSetBoolean 
>> to compareAndSetInt. Change field "generated" from boolean to int doesn't 
>> increase object size (no extra byte/boolean to fold into same word).
>> 
>>> The restoring to 0 in the event of failure should probably be a 
>>> volatile-write. Might be clearer to declare it as a volatile.
>> 
>> "generated" field  is read once and CAS once in this method. It doesn't 
>> likely cached and no visibiliy order required.
>
> okay, although I think it would be clearer for maintainers to declare it as 
> volatile because it is set with CAS.

@AlanBateman 

volatile attribute is added. As in common case, it only read and CAS at most 
once. And I see no regression on my testcase.

-

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

Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v6]

[Hui Shi]

> …AccessorImpl object
> 
> We met real problem when using protobuf with option optimized for code size, 
> detail in JBS https://bugs.openjdk.java.net/browse/JDK-8255883
> 
> Optimize solution is adding a new boolean field to detect concurrent method 
> accessor generation in same NativeMethodAccessorImpl object, only one thread 
> is allowed to generate accessor, other threads still invoke in jni way until 
> parent's delegator is updated from NativeMethodAccessorImpl  to generated 
> accessor.
> 
> In common case, extra overhead is an atomic operation, compared with method 
> accessor generate, this cost is trivial.

Hui Shi has refreshed the contents of this pull request, and previous commits 
have been removed. The incremental views will show differences compared to the 
previous content of the PR. The pull request contains one new commit since the 
last revision:

  8255883: Avoid multiple GeneratedAccessor for same 
NativeMethod/ConstructorAccessorImpl object

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1070/files
  - new: https://git.openjdk.java.net/jdk/pull/1070/files/ec1531b2..85c8b96a

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

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

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

Re: RFR: 8255947: [macos] Signed macOS jpackage app doesn't filter spurious '-psn' argument [v2]

[Alexander Matveev]

> This is regression from JDK-8242302 and for some reason removing -psn 
> argument code was removed during refactoring. Fixed be adding removing -psn 
> argument back. Also, test was added to test this functionality.

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

  8255947: [macos] Signed macOS jpackage app doesn't filter spurious '-psn' 
argument [v2]

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1154/files
  - new: https://git.openjdk.java.net/jdk/pull/1154/files/e07fec6f..7df8d225

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

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

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

Integrated: 8256063: Module::getPackages on an unnamed module may return packages that are in a named module

[Mandy Chung]

On Wed, 11 Nov 2020 23:02:49 GMT, Mandy Chung  wrote:

> If `Module::getPackages` is invoked on the unnamed module of a class loader
> then the set of packages returned will include the packages of any named 
> modules
> that are defined to that class loader.  This is a spec and implementation bug.
> 
> `Module::getPackages` is changed to return the packages that are defined in
> the unnamed module. 
> 
> CSR: https://bugs.openjdk.java.net/browse/JDK-8256242

This pull request has now been integrated.

Changeset: dff26a48
Author:Mandy Chung 
URL:   https://git.openjdk.java.net/jdk/commit/dff26a48
Stats: 123 lines in 4 files changed: 118 ins; 1 del; 4 mod

8256063: Module::getPackages on an unnamed module may return packages that are 
in a named module

Reviewed-by: alanb, chegar

-

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

Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs

[Kevin Rushforth]

On Mon, 9 Nov 2020 13:56:33 GMT, Andy Herrick  wrote:

> JDK-8189198: Add "forRemoval = true" to Applet APIs

@andyherrick can you enter the `/csr needed` command? I would, but it needs to 
be done by either the author of the PR or a Reviewer.

-

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

Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v2]

[Alexander Matveev]

On Thu, 12 Nov 2020 20:48:13 GMT, Andy Herrick  wrote:

>> JDK-8189198: Add "forRemoval = true" to Applet APIs
>
> Andy Herrick has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains six additional 
> commits since the last revision:
> 
>  - JDK-8189198: Add "forRemoval = true" to Applet APIs
>  - Merge branch 'master' into JDK-8189198
>  - Merge branch 'master' into JDK-8189198
>  - JDK-8189198: Add "forRemoval = true" to Applet APIs
>  - JDK-8189198: Add "forRemoval = true" to Applet APIs
>  - JDK-8189198: Add "forRemoval = true" to Applet APIs

Marked as reviewed by almatvee (Committer).

-

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

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

[Naoto Sato]

On Thu, 12 Nov 2020 21:49:16 GMT, Stephen Colebourne  
wrote:

>> 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_r522185469
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r522187931
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r522203757
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r522211444
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r522244221
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r522262379
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r522266836
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 5063:
> 
>> 5061: }
>> 5062: Long moh = context.getValue(MINUTE_OF_HOUR);
>> 5063: long value = (hod * 60 + (moh != null ? moh : 0)) % 1_440;
> 
> `long value = Math.floorMod(hod, 24) * 60 + (moh != null ? Math.floorMod(moh, 
> 60) : 0);`
> 
> and remove the next three lines

Thanks, Stephen. I made the changes based on your comments.

-

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

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

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

  Re-worded the spec of appendDayPeriodText, refactored calculation of 
minute-of-day.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/938/files
  - new: https://git.openjdk.java.net/jdk/pull/938/files/570b4582..1aa3134f

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

  Stats: 18 lines in 1 file changed: 0 ins; 3 del; 15 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: 8230501: Class data support for hidden classes [v2]

[Mandy Chung]

> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live 
> objects
> be shared between a hidden class and other classes.  A hidden class can load
> these live objects as dynamically-computed constants via this API.
> 
> Specdiff
> http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html
> 
> With this class data support and hidden classes, 
> `sun.misc.Unsafe::defineAnonymousClass`
> will be deprecated for removal.  Existing libraries should replace their
> calls to `sun.misc.Unsafe::defineAnonymousClass` with 
> `Lookup::defineHiddenClass`
> or `Lookup::defineHiddenClassWithClassData`.
> 
> This patch also updates the implementation of lambda meta factory and
> `MemoryAccessVarHandleGenerator` to use class data.   No performance 
> difference
> observed in the jdk.incubator.foreign microbenchmarks.   A side note: 
> `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of 
> JDK-8254162 but it helps validating the class data support.
> 
> Background
> --
> 
> This is an enhancement following up JEP 371: Hidden Classes w.r.t.
> "Constant-pool patching" in the "Risks and Assumption" section.
> 
> A VM-anonymous class can be defined with its constant-pool entries already
> resolved to concrete values. This allows critical constants to be shared
> between a VM-anonymous class and the language runtime that defines it, and
> between multiple VM-anonymous classes. For example, a language runtime will
> often have `MethodHandle` objects in its address space that would be useful
> to newly-defined VM-anonymous classes. Instead of the runtime serializing
> the objects to constant-pool entries in VM-anonymous classes and then
> generating bytecode in those classes to laboriously `ldc` the entries,
> the runtime can simply supply `Unsafe::defineAnonymousClass` with references
> to its live objects. The relevant constant-pool entries in the newly-defined
> VM-anonymous class are pre-linked to those objects, improving performance
> and reducing footprint. In addition, this allows VM-anonymous classes to
> refer to each other: Constant-pool entries in a class file are based on names.
> They thus cannot refer to nameless VM-anonymous classes. A language runtime 
> can,
> however, easily track the live Class objects for its VM-anonymous classes and
> supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new 
> class's
> constant pool entries to other VM-anonymous classes.
> 
> This extends the hidden classes to allow live objects to be injected
> in a hidden class and loaded them via condy.
> 
> Details
> ---
> 
> A new `Lookup::defineHiddenClassWithClassData` API takes additional
> `classData` argument compared to `Lookup::defineHiddenClass`.
> Class data can be method handles, lookup objects, arbitrary user objects
> or collections of all of the above.
> 
> This method behaves as if calling `Lookup::defineHiddenClass` to define
> a hidden class with a private static unnamed field that is initialized
> with `classData` at the first instruction of the class initializer.
> 
> `MethodHandles::classData(Lookup lookup, String name, Class type)`
> is a bootstrap method to load the class data of the given lookup's lookup 
> class.
> The hidden class will be initialized when `classData` method is called if
> the hidden class has not been initialized.
> 
> For a class data containing more than one single element, libraries can
> create their convenience method to load a single live object via condy.
> We can reconsider if such a convenience method is needed in the future.
> 
> Frameworks sometimes want to dynamically create a hidden class (HC) and add it
> it the lookup class nest and have HC to carry secrets hidden from that nest.
> In this case, frameworks should not to use private static finals (in the HCs
> they spin) to hold secrets because a nestmate of HC may obtain access to
> such a private static final and observe the framework's secret.  It should use
> condy.  In addition, we need to differentiate if a lookup object is created 
> from
> the original lookup class or created from teleporting e.g. `Lookup::in`
> and `MethodHandles::privateLookupIn`.
> 
> This proposes to add a new `ORIGINAL` bit that is only set if the lookup
> object is created by `MethodHandles::lookup` or by bootstrap method 
> invocation.
> The operations only apply to a Lookup object with original access are:
>- create method handles for caller-sensitve methods
>- obtain class data associated with the lookup class
> 
> No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which
> ignores the ORIGINAL bit.
> 
> 
> Compatibility Risks
> ---
> 
> `Lookup::lookupModes` includes a new `ORIGINAL` bit.  Most lookup operations
> ignore this original bit except creating method handles for caller-sensitive 
> methods
> that expects the lookup from the original lookup class.  Existing code  
> compares
> the return value of 

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

[Stephen Colebourne]

On Thu, 12 Nov 2020 20:03:14 GMT, Naoto Sato  wrote:

>> 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_r522185469
>   - https://github.com/openjdk/jdk/pull/938#discussion_r522187931
>   - https://github.com/openjdk/jdk/pull/938#discussion_r522203757
>   - https://github.com/openjdk/jdk/pull/938#discussion_r522211444
>   - https://github.com/openjdk/jdk/pull/938#discussion_r522244221
>   - https://github.com/openjdk/jdk/pull/938#discussion_r522262379
>   - https://github.com/openjdk/jdk/pull/938#discussion_r522266836

Approved with one overflow to fix.

The spec could do with some rewording too. It might be better to explicitly 
mention the "resolving phase" with the three parts:

> The day period is combined with other fields to make a `LocalTime` in the 
> resolving phase. If the `HOUR_OF_AMPM` field is present, it is combined with 
> the day period to make `HOUR_OF_DAY` taking into account any `MINUTE_OF_HOUR` 
> value. If `HOUR_OF_DAY` is present, it is validated against the day period 
> taking into account any `MINUTE_OF_HOUR` value. If a day period is present 
> without `HOUR_OF_DAY`, `MINUTE_OF_HOUR`, `SECOND_OF_MINUTE` and 
> `NANO_OF_SECOND` then the midpoint of the day period is set as the time.

Note that the above is incomplete, and it doesn't describe STRICT/LENIENT, so 
the actual words will be more complex,

src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 
5063:

> 5061: }
> 5062: Long moh = context.getValue(MINUTE_OF_HOUR);
> 5063: long value = (hod * 60 + (moh != null ? moh : 0)) % 1_440;

`long value = Math.floorMod(hod, 24) * 60 + (moh != null ? Math.floorMod(moh, 
60) : 0);`

and remove the next three lines

-

Marked as reviewed by scolebourne (Author).

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

Re: RFR: JDK-8189198: Add "forRemoval = true" to Applet APIs [v2]

[Andy Herrick]

> JDK-8189198: Add "forRemoval = true" to Applet APIs

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

 - JDK-8189198: Add "forRemoval = true" to Applet APIs
 - Merge branch 'master' into JDK-8189198
 - Merge branch 'master' into JDK-8189198
 - JDK-8189198: Add "forRemoval = true" to Applet APIs
 - JDK-8189198: Add "forRemoval = true" to Applet APIs
 - JDK-8189198: Add "forRemoval = true" to Applet APIs

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1127/files
  - new: https://git.openjdk.java.net/jdk/pull/1127/files/a74d..d9850cd8

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

  Stats: 7753 lines in 89 files changed: 4891 ins; 1603 del; 1259 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1127.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1127/head:pull/1127

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

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

[Naoto Sato]

On Thu, 12 Nov 2020 15:21:52 GMT, Roger Riggs  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Added a test case for user defined temporal field resolution with day 
>> period.
>
> test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java 
> line 877:
> 
>> 875: try {
>> 876: dtf.parse("0 at night");
>> 877: throw new RuntimeException("DateTimeParseException should 
>> be thrown");
> 
> Testng has `Assert.fail(message)` for this case.

Fixed.

-

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

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

[Naoto Sato]

On Thu, 12 Nov 2020 15:41:56 GMT, Stephen Colebourne  
wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Added a test case for user defined temporal field resolution with day 
>> period.
>
> src/java.base/share/classes/java/time/format/Parsed.java line 550:
> 
>> 548: long midpoint = dayPeriod.mid();
>> 549: fieldValues.put(HOUR_OF_DAY, midpoint / 60);
>> 550: fieldValues.put(MINUTE_OF_HOUR, midpoint % 60);
> 
> Set `dayPeriod = null` here, as it has been successfully used.

Fixed.

> src/java.base/share/classes/java/time/format/Parsed.java line 502:
> 
>> 500: HOUR_OF_AMPM.checkValidValue(hoap);
>> 501: }
>> 502: if (dayPeriod.includes((hoap % 24 + 12) * 60)) {
> 
> Pretty sure the 24 should be 12.
> 
> Also, it should use `Math.floorMod()` to handle `HOUR_OF_AMPM = -1` (which is 
> 11 o'clock).
> 
> Also, this doesn't take into account minutes. So if the day period rule is 
> afternoon1 to 18:30 and evening1 after 18:30, and the input is `HOUR_OF_AMPM 
> = 6, MINUTE_OF_HOUR = 45`, this will fail to resolve,
> 
> Something like this should work (no need for `updateCheckDayPeriodConflict`):
> 
> long hoap = fieldValues.remove(HOUR_OF_AMPM);
> if (resolverStyle == ResolverStyle.LENIENT) {
>   HOUR_OF_AMPM.checkValidValue(hoap);
> }
> Long mohObj = fieldValues.get(MINUTE_OF_HOUR);
> long moh = mohObj != null ? Math.floorMod(mohObj, 60) : 0;
> long excessHours = dayPeriod.includes(Math.floorMod(hoap, 12) + 12 * 60 + 
> moh ? 12 : 0;
> long hod = Math.addExact(hoap, excessHours);
> updateCheckConflict(HOUR_OF_AMPM, HOUR_OF_DAY, hod);
> dayPeriod = null;

Fixed.

> src/java.base/share/classes/java/time/format/Parsed.java line 546:
> 
>> 544: 
>> 545: // Set the hour-of-day, if not exist and not in STRICT, to 
>> the mid point of the day period or am/pm.
>> 546: if (!fieldValues.containsKey(HOUR_OF_DAY) && resolverStyle 
>> != ResolverStyle.STRICT) {
> 
> Since this logic replaces both `HOUR_OF_DAY` and `MINUTE_OF_HOUR` I think it 
> should only be invoked if both do not exist. Beyond that, it doesn't really 
> make sense to do this logic if second/sub-second are present. Thus, it needs 
> to check all four fields and can then directly set the time.
> 
> if (!fieldValues.containsKey(HOUR_OF_DAY) && 
> !fieldValues.containsKey(MINUTE_OF_HOUR) && 
> !fieldValues.containsKey(SECOND_OF_MINUTE) && 
> !fieldValues.containsKey(NANO_OF_SECOND) && 
> resolverStyle != ResolverStyle.STRICT) {
> 
>   if (dayPeriod != null) {
> long midpoint = dayPeriod.mid();
> resolveTime(midpoint / 60, midpoint % 60, 0, 0);
> dayPeriod = null;
>   } else if (fieldValues.containsKey(AMPM_OF_DAY)) {
> long ap = fieldValues.remove(AMPM_OF_DAY);
> if (resolverStyle == ResolverStyle.LENIENT) {
>   resolveTime(Math.addExact(Math.multiplyExact(ap, 12), 6), 0, 0, 0);
> } else {  // SMART
>   AMPM_OF_DAY.checkValidValue(ap);
>   resolveTime(ap * 12 + 6, 0, 0, 0);
> }

Fixed.

> src/java.base/share/classes/java/time/format/Parsed.java line 568:
> 
>> 566: if (dayPeriod != null) {
>> 567: // Check whether the hod is within the day period
>> 568: updateCheckDayPeriodConflict(HOUR_OF_DAY, hod);
> 
> With the other changes, this is the only use of this method and it can 
> therefore be simplified (no need to check for conflicts, just whether it 
> matches the day period).

Fixed.

> test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java 
> line 778:
> 
>> 776: {"h B", ResolverStyle.SMART, "59 in the morning", 11},
>> 777: 
>> 778: {"H B", ResolverStyle.LENIENT, "47 at night", 23},
> 
> This test should be split in two - smart (fails) and lenient (succeeds). The 
> lenient tests should ensure that 
> `p.query(DateTimeFormatter.parsedExcessDays())` returns the expected number 
> of excess days. 
> 
> I'd also add a test for a negative value such as "-2 at night"

Fixed.

-

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

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

[Naoto Sato]

On Thu, 12 Nov 2020 15:18:44 GMT, Roger Riggs  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Clarified 24:00 for "midnight" type in the spec. Some clean up.
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 1483:
> 
>> 1481:  * exception is thrown and the day period is ignored.
>> 1482:  * 
>> 1483:  * "midnight" type allows both "00:00" as the start-of-day and 
>> "24:00" as the
> 
> Editorial:  Add "The " at the beginning of the sentence.

Fixed.

-

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

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

[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_r522185469
  - https://github.com/openjdk/jdk/pull/938#discussion_r522187931
  - https://github.com/openjdk/jdk/pull/938#discussion_r522203757
  - https://github.com/openjdk/jdk/pull/938#discussion_r522211444
  - https://github.com/openjdk/jdk/pull/938#discussion_r522244221
  - https://github.com/openjdk/jdk/pull/938#discussion_r522262379
  - https://github.com/openjdk/jdk/pull/938#discussion_r522266836

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/938/files
  - new: https://git.openjdk.java.net/jdk/pull/938/files/a960804f..570b4582

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

  Stats: 75 lines in 3 files changed: 23 ins; 23 del; 29 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: 8230501: Class data support for hidden classes

[Mandy Chung]

On Thu, 12 Nov 2020 14:18:17 GMT, Jorn Vernee  wrote:

>> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live 
>> objects
>> be shared between a hidden class and other classes.  A hidden class can load
>> these live objects as dynamically-computed constants via this API.
>> 
>> Specdiff
>> http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html
>> 
>> With this class data support and hidden classes, 
>> `sun.misc.Unsafe::defineAnonymousClass`
>> will be deprecated for removal.  Existing libraries should replace their
>> calls to `sun.misc.Unsafe::defineAnonymousClass` with 
>> `Lookup::defineHiddenClass`
>> or `Lookup::defineHiddenClassWithClassData`.
>> 
>> This patch also updates the implementation of lambda meta factory and
>> `MemoryAccessVarHandleGenerator` to use class data.   No performance 
>> difference
>> observed in the jdk.incubator.foreign microbenchmarks.   A side note: 
>> `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of 
>> JDK-8254162 but it helps validating the class data support.
>> 
>> Background
>> --
>> 
>> This is an enhancement following up JEP 371: Hidden Classes w.r.t.
>> "Constant-pool patching" in the "Risks and Assumption" section.
>> 
>> A VM-anonymous class can be defined with its constant-pool entries already
>> resolved to concrete values. This allows critical constants to be shared
>> between a VM-anonymous class and the language runtime that defines it, and
>> between multiple VM-anonymous classes. For example, a language runtime will
>> often have `MethodHandle` objects in its address space that would be useful
>> to newly-defined VM-anonymous classes. Instead of the runtime serializing
>> the objects to constant-pool entries in VM-anonymous classes and then
>> generating bytecode in those classes to laboriously `ldc` the entries,
>> the runtime can simply supply `Unsafe::defineAnonymousClass` with references
>> to its live objects. The relevant constant-pool entries in the newly-defined
>> VM-anonymous class are pre-linked to those objects, improving performance
>> and reducing footprint. In addition, this allows VM-anonymous classes to
>> refer to each other: Constant-pool entries in a class file are based on 
>> names.
>> They thus cannot refer to nameless VM-anonymous classes. A language runtime 
>> can,
>> however, easily track the live Class objects for its VM-anonymous classes and
>> supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new 
>> class's
>> constant pool entries to other VM-anonymous classes.
>> 
>> This extends the hidden classes to allow live objects to be injected
>> in a hidden class and loaded them via condy.
>> 
>> Details
>> ---
>> 
>> A new `Lookup::defineHiddenClassWithClassData` API takes additional
>> `classData` argument compared to `Lookup::defineHiddenClass`.
>> Class data can be method handles, lookup objects, arbitrary user objects
>> or collections of all of the above.
>> 
>> This method behaves as if calling `Lookup::defineHiddenClass` to define
>> a hidden class with a private static unnamed field that is initialized
>> with `classData` at the first instruction of the class initializer.
>> 
>> `MethodHandles::classData(Lookup lookup, String name, Class type)`
>> is a bootstrap method to load the class data of the given lookup's lookup 
>> class.
>> The hidden class will be initialized when `classData` method is called if
>> the hidden class has not been initialized.
>> 
>> For a class data containing more than one single element, libraries can
>> create their convenience method to load a single live object via condy.
>> We can reconsider if such a convenience method is needed in the future.
>> 
>> Frameworks sometimes want to dynamically create a hidden class (HC) and add 
>> it
>> it the lookup class nest and have HC to carry secrets hidden from that nest.
>> In this case, frameworks should not to use private static finals (in the HCs
>> they spin) to hold secrets because a nestmate of HC may obtain access to
>> such a private static final and observe the framework's secret.  It should 
>> use
>> condy.  In addition, we need to differentiate if a lookup object is created 
>> from
>> the original lookup class or created from teleporting e.g. `Lookup::in`
>> and `MethodHandles::privateLookupIn`.
>> 
>> This proposes to add a new `ORIGINAL` bit that is only set if the lookup
>> object is created by `MethodHandles::lookup` or by bootstrap method 
>> invocation.
>> The operations only apply to a Lookup object with original access are:
>>- create method handles for caller-sensitve methods
>>- obtain class data associated with the lookup class
>> 
>> No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which
>> ignores the ORIGINAL bit.
>> 
>> 
>> Compatibility Risks
>> ---
>> 
>> `Lookup::lookupModes` includes a new `ORIGINAL` bit.  Most lookup operations
>> ignore this original bit except 

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

[Maurizio Cimadamore]

On Wed, 11 Nov 2020 14:18:33 GMT, Vladimir Ivanov  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with 10 
>> additional commits since the last revision:
>> 
>>  - Merge pull request #7 from JornVernee/Additional_Review_Comments
>>
>>Additional review comments
>>  - Revert System.java changes
>>  - Set copyright year for added files to 2020
>>  - Check result of AttachCurrentThread
>>  - Sort includes alphabetically
>>  - Relax ret_addr_offset() assert
>>  - Extra space after if
>>  - remove excessive asserts in ProgrammableInvoker::invoke_native
>>  - Remove os::is_MP() check
>>  - remove blank line in thread.hpp
>
> I made a pass over hotspot code. Overall, it looks good. Some comments follow.

I've just merged against master - which now contains the foreign memory API 
changes that this JEP depends on. I believe reviewing the changes should now be 
easier, as only the relevant changes should be presented in the "File Changed" 
tab.

-

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

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

[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 restrictions which apply to JNI library loading (e.g. same 
> library 

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

[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 restrictions which apply to JNI library loading (e.g. same 
> library 

Re: RFR: 8255947: [macos] Signed macOS jpackage app doesn't filter spurious '-psn' argument

[Alexey Semenyuk]

On Wed, 11 Nov 2020 04:28:02 GMT, Alexander Matveev  
wrote:

> This is regression from JDK-8242302 and for some reason removing -psn 
> argument code was removed during refactoring. Fixed be adding removing -psn 
> argument back. Also, test was added to test this functionality.

src/jdk.jpackage/macosx/native/common/MacSysInfo.cpp line 90:

> 88: }
> 89: result.push_back(argv[i]);
> 90: }

I'd suggest to use C++ here:
Suggestion:

const tstring psnArgPrefix = "-psn_";
for (int i = progNameMode == ExcludeProgramName ? 1 : 0; i < argc; i++) {
const tstring arg = argv[i];
if (!tstrings::startsWith(arg, psnArgPrefix)) {
result.push_back(arg);
}
}

-

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

Integrated: 8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1

[Roger Riggs]

On Thu, 12 Nov 2020 15:48:33 GMT, Roger Riggs  wrote:

> TestNG 7.1 changed/corrected the way that @BeforeGroups are selected at 
> runtime.
> The test was depending on @BeforeGroups to initialize common security policy 
> for a number of tests.
> The tests are modified to individually setup the needed security manager and 
> policy.

This pull request has now been integrated.

Changeset: b5a9c92b
Author:Roger Riggs 
URL:   https://git.openjdk.java.net/jdk/commit/b5a9c92b
Stats: 19 lines in 1 file changed: 9 ins; 1 del; 9 mod

8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1

Reviewed-by: lancea, iignatyev

-

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

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

[Stephen Colebourne]

On Tue, 10 Nov 2020 19:52:14 GMT, Naoto Sato  wrote:

>> 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:
> 
>   Added a test case for user defined temporal field resolution with day 
> period.

src/java.base/share/classes/java/time/format/Parsed.java line 550:

> 548: long midpoint = dayPeriod.mid();
> 549: fieldValues.put(HOUR_OF_DAY, midpoint / 60);
> 550: fieldValues.put(MINUTE_OF_HOUR, midpoint % 60);

Set `dayPeriod = null` here, as it has been successfully used.

src/java.base/share/classes/java/time/format/Parsed.java line 502:

> 500: HOUR_OF_AMPM.checkValidValue(hoap);
> 501: }
> 502: if (dayPeriod.includes((hoap % 24 + 12) * 60)) {

Pretty sure the 24 should be 12.

Also, it should use `Math.floorMod()` to handle `HOUR_OF_AMPM = -1` (which is 
11 o'clock).

Also, this doesn't take into account minutes. So if the day period rule is 
afternoon1 to 18:30 and evening1 after 18:30, and the input is `HOUR_OF_AMPM = 
6, MINUTE_OF_HOUR = 45`, this will fail to resolve,

Something like this should work (no need for `updateCheckDayPeriodConflict`):

long hoap = fieldValues.remove(HOUR_OF_AMPM);
if (resolverStyle == ResolverStyle.LENIENT) {
  HOUR_OF_AMPM.checkValidValue(hoap);
}
Long mohObj = fieldValues.get(MINUTE_OF_HOUR);
long moh = mohObj != null ? Math.floorMod(mohObj, 60) : 0;
long excessHours = dayPeriod.includes(Math.floorMod(hoap, 12) + 12 * 60 + 
moh ? 12 : 0;
long hod = Math.addExact(hoap, excessHours);
updateCheckConflict(HOUR_OF_AMPM, HOUR_OF_DAY, hod);
dayPeriod = null;

src/java.base/share/classes/java/time/format/Parsed.java line 546:

> 544: 
> 545: // Set the hour-of-day, if not exist and not in STRICT, to 
> the mid point of the day period or am/pm.
> 546: if (!fieldValues.containsKey(HOUR_OF_DAY) && resolverStyle 
> != ResolverStyle.STRICT) {

Since this logic replaces both `HOUR_OF_DAY` and `MINUTE_OF_HOUR` I think it 
should only be invoked if both do not exist. Beyond that, it doesn't really 
make sense to do this logic if second/sub-second are present. Thus, it needs to 
check all four fields and can then directly set the time.

if (!fieldValues.containsKey(HOUR_OF_DAY) && 
!fieldValues.containsKey(MINUTE_OF_HOUR) && 
!fieldValues.containsKey(SECOND_OF_MINUTE) && 
!fieldValues.containsKey(NANO_OF_SECOND) && 
resolverStyle != ResolverStyle.STRICT) {

  if (dayPeriod != null) {
long midpoint = dayPeriod.mid();
resolveTime(midpoint / 60, midpoint % 60, 0, 0);
dayPeriod = null;
  } else if (fieldValues.containsKey(AMPM_OF_DAY)) {
long ap = fieldValues.remove(AMPM_OF_DAY);
if (resolverStyle == ResolverStyle.LENIENT) {
  resolveTime(Math.addExact(Math.multiplyExact(ap, 12), 6), 0, 0, 0);
} else {  // SMART
  AMPM_OF_DAY.checkValidValue(ap);
  resolveTime(ap * 12 + 6, 0, 0, 0);
}

src/java.base/share/classes/java/time/format/Parsed.java line 568:

> 566: if (dayPeriod != null) {
> 567: // Check whether the hod is within the day period
> 568: updateCheckDayPeriodConflict(HOUR_OF_DAY, hod);

With the other changes, this is the only use of this method and it can 
therefore be simplified (no need to check for conflicts, just whether it 
matches the day period).

test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java line 
778:

> 776: {"h B", ResolverStyle.SMART, "59 in the morning", 11},
> 777: 
> 778: {"H B", ResolverStyle.LENIENT, "47 at night", 23},

This test should be split in two - smart (fails) and lenient (succeeds). The 
lenient tests should ensure that 
`p.query(DateTimeFormatter.parsedExcessDays())` returns the expected number of 
excess days. 

I'd also add a test for a negative value such as "-2 at night"

-

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

Re: RFR: 8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1

[Igor Ignatyev]

On Thu, 12 Nov 2020 15:48:33 GMT, Roger Riggs  wrote:

> TestNG 7.1 changed/corrected the way that @BeforeGroups are selected at 
> runtime.
> The test was depending on @BeforeGroups to initialize common security policy 
> for a number of tests.
> The tests are modified to individually setup the needed security manager and 
> policy.

Marked as reviewed by iignatyev (Reviewer).

-

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

Integrated: 8254162: Implementation of Foreign-Memory Access API (Third Incubator)

[Maurizio Cimadamore]

On Wed, 7 Oct 2020 17:13:22 GMT, Maurizio Cimadamore  
wrote:

> This patch contains the changes associated with the third incubation round of 
> the foreign memory access API incubation  (see JEP 393 [1]). This iteration 
> focus on improving the usability of the API in 3 main ways:
> 
> * first, by providing a way to obtain truly *shared* segments, which can be 
> accessed and closed concurrently from multiple threads
> * second, by providing a way to register a memory segment against a 
> `Cleaner`, so as to have some (optional) guarantee that the memory will be 
> deallocated, eventually
> * third, by not requiring users to dive deep into var handles when they first 
> pick up the API; a new `MemoryAccess` class has been added, which defines 
> several useful dereference routines; these are really just thin wrappers 
> around memory access var handles, but they make the barrier of entry for 
> using this API somewhat lower.
> 
> A big conceptual shift that comes with this API refresh is that the role of 
> `MemorySegment` and `MemoryAddress` is not the same as it used to be; it used 
> to be the case that a memory address could (sometimes, not always) have a 
> back link to the memory segment which originated it; additionally, memory 
> access var handles used `MemoryAddress` as a basic unit of dereference.
> 
> This has all changed as per this API refresh;  now a `MemoryAddress` is just 
> a dumb carrier which wraps a pair of object/long addressing coordinates; 
> `MemorySegment` has become the star of the show, as far as dereferencing 
> memory is concerned. You cannot dereference memory if you don't have a 
> segment. This improves usability in a number of ways - first, it is a lot 
> easier to wrap native addresses (`long`, essentially) into a `MemoryAddress`; 
> secondly, it is crystal clear what a client has to do in order to dereference 
> memory: if a client has a segment, it can use that; otherwise, if the client 
> only has an address, it will have to create a segment *unsafely* (this can be 
> done by calling `MemoryAddress::asSegmentRestricted`).
> 
> A list of the API, implementation and test changes is provided below. If  you 
> have any questions, or need more detailed explanations, I (and the  rest of 
> the Panama team) will be happy to point at existing discussions,  and/or to 
> provide the feedback required. 
> 
> A big thank to Erik Osterlund, Vladimir Ivanov and David Holmes, without whom 
> the work on shared memory segment would not have been possible; also I'd like 
> to thank Paul Sandoz, whose insights on API design have been very helpful in 
> this journey.
> 
> Thanks 
> Maurizio 
> 
> Javadoc: 
> 
> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/javadoc/jdk/incubator/foreign/package-summary.html
> 
> Specdiff: 
> 
> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/specdiff/jdk/incubator/foreign/package-summary.html
> 
> CSR: 
> 
> https://bugs.openjdk.java.net/browse/JDK-8254163
> 
> 
> 
> ### API Changes
> 
> * `MemorySegment`
>   * drop factory for restricted segment (this has been moved to 
> `MemoryAddress`, see below)
>   * added a no-arg factory for a native restricted segment representing 
> entire native heap
>   * rename `withOwnerThread` to `handoff`
>   * add new `share` method, to create shared segments
>   * add new `registerCleaner` method, to register a segment against a cleaner
>   * add more helpers to create arrays from a segment e.g. `toIntArray`
>   * add some `asSlice` overloads (to make up for the fact that now segments 
> are more frequently used as cursors)
>   * rename `baseAddress` to `address` (so that `MemorySegment` can implement 
> `Addressable`)
> * `MemoryAddress`
>   * drop `segment` accessor
>   * drop `rebase` method and replace it with `segmentOffset` which returns 
> the offset (a `long`) of this address relative to a given segment
> * `MemoryAccess`
>   * New class supporting several static dereference helpers; the helpers are 
> organized by carrier and access mode, where a carrier is one of the usual 
> suspect (a Java primitive, minus `boolean`); the access mode can be simple 
> (e.g. access base address of given segment), or indexed, in which case the 
> accessor takes a segment and either a low-level byte offset,or a high level 
> logical index. The classification is reflected in the naming scheme (e.g. 
> `getByte` vs. `getByteAtOffset` vs `getByteAtIndex`).
> * `MemoryHandles`
>   * drop `withOffset` combinator
>   * drop `withStride` combinator
>   * the basic memory access handle factory now returns a var handle which 
> takes a `MemorySegment` and a `long` - from which it is easy to derive all 
> the other handles using plain var handle combinators.
> * `Addressable`
>   * This is a new interface which is attached to entities which can be 
> projected to a `MemoryAddress`. For now, both `MemoryAddress` and 
> `MemorySegment` implement it; we have plans, with JEP 389 [2] to add more 
> implementations. Clients can 

Working on JDK-8072719

[Kartik Ohri]

Hi!
I think that JDK-8072719
, enhancing
checked collections to accept a Predicate, can be a valuable addition to
the JDK. I would like to start a discussion on whether this is a good fit
for the Collections framework and the JDK. If it is decided to include this
feature, I would be interested in contributing a patch for the same.
Thanks.
Regards,
Kartik

Re: RFR: 8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1

[Lance Andersen]

On Thu, 12 Nov 2020 15:48:33 GMT, Roger Riggs  wrote:

> TestNG 7.1 changed/corrected the way that @BeforeGroups are selected at 
> runtime.
> The test was depending on @BeforeGroups to initialize common security policy 
> for a number of tests.
> The tests are modified to individually setup the needed security manager and 
> policy.

Marked as reviewed by lancea (Reviewer).

-

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

RFR: 8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1

[Roger Riggs]

TestNG 7.1 changed/corrected the way that @BeforeGroups are selected at runtime.
The test was depending on @BeforeGroups to initialize common security policy 
for a number of tests.
The tests are modified to individually setup the needed security manager and 
policy.

-

Commit messages:
 - 8256244: java/lang/ProcessHandle/PermissionTest.java fails with TestNG 7.1

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

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

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

[Roger Riggs]

On Tue, 10 Nov 2020 19:52:14 GMT, Naoto Sato  wrote:

>> 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:
> 
>   Added a test case for user defined temporal field resolution with day 
> period.

Marked as reviewed by rriggs (Reviewer).

-

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

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

[Roger Riggs]

On Tue, 10 Nov 2020 00:07:16 GMT, Naoto Sato  wrote:

>> 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:
> 
>   Clarified 24:00 for "midnight" type in the spec. Some clean up.

src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 
1483:

> 1481:  * exception is thrown and the day period is ignored.
> 1482:  * 
> 1483:  * "midnight" type allows both "00:00" as the start-of-day and 
> "24:00" as the

Editorial:  Add "The " at the beginning of the sentence.

-

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

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

[Roger Riggs]

On Tue, 10 Nov 2020 19:52:14 GMT, Naoto Sato  wrote:

>> 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:
> 
>   Added a test case for user defined temporal field resolution with day 
> period.

test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java line 
877:

> 875: try {
> 876: dtf.parse("0 at night");
> 877: throw new RuntimeException("DateTimeParseException should be 
> thrown");

Testng has `Assert.fail(message)` for this case.

-

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

Re: RFR: 8230501: Class data support for hidden classes

[Jorn Vernee]

On Thu, 12 Nov 2020 15:11:03 GMT, Jorn Vernee  wrote:

>> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live 
>> objects
>> be shared between a hidden class and other classes.  A hidden class can load
>> these live objects as dynamically-computed constants via this API.
>> 
>> Specdiff
>> http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html
>> 
>> With this class data support and hidden classes, 
>> `sun.misc.Unsafe::defineAnonymousClass`
>> will be deprecated for removal.  Existing libraries should replace their
>> calls to `sun.misc.Unsafe::defineAnonymousClass` with 
>> `Lookup::defineHiddenClass`
>> or `Lookup::defineHiddenClassWithClassData`.
>> 
>> This patch also updates the implementation of lambda meta factory and
>> `MemoryAccessVarHandleGenerator` to use class data.   No performance 
>> difference
>> observed in the jdk.incubator.foreign microbenchmarks.   A side note: 
>> `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of 
>> JDK-8254162 but it helps validating the class data support.
>> 
>> Background
>> --
>> 
>> This is an enhancement following up JEP 371: Hidden Classes w.r.t.
>> "Constant-pool patching" in the "Risks and Assumption" section.
>> 
>> A VM-anonymous class can be defined with its constant-pool entries already
>> resolved to concrete values. This allows critical constants to be shared
>> between a VM-anonymous class and the language runtime that defines it, and
>> between multiple VM-anonymous classes. For example, a language runtime will
>> often have `MethodHandle` objects in its address space that would be useful
>> to newly-defined VM-anonymous classes. Instead of the runtime serializing
>> the objects to constant-pool entries in VM-anonymous classes and then
>> generating bytecode in those classes to laboriously `ldc` the entries,
>> the runtime can simply supply `Unsafe::defineAnonymousClass` with references
>> to its live objects. The relevant constant-pool entries in the newly-defined
>> VM-anonymous class are pre-linked to those objects, improving performance
>> and reducing footprint. In addition, this allows VM-anonymous classes to
>> refer to each other: Constant-pool entries in a class file are based on 
>> names.
>> They thus cannot refer to nameless VM-anonymous classes. A language runtime 
>> can,
>> however, easily track the live Class objects for its VM-anonymous classes and
>> supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new 
>> class's
>> constant pool entries to other VM-anonymous classes.
>> 
>> This extends the hidden classes to allow live objects to be injected
>> in a hidden class and loaded them via condy.
>> 
>> Details
>> ---
>> 
>> A new `Lookup::defineHiddenClassWithClassData` API takes additional
>> `classData` argument compared to `Lookup::defineHiddenClass`.
>> Class data can be method handles, lookup objects, arbitrary user objects
>> or collections of all of the above.
>> 
>> This method behaves as if calling `Lookup::defineHiddenClass` to define
>> a hidden class with a private static unnamed field that is initialized
>> with `classData` at the first instruction of the class initializer.
>> 
>> `MethodHandles::classData(Lookup lookup, String name, Class type)`
>> is a bootstrap method to load the class data of the given lookup's lookup 
>> class.
>> The hidden class will be initialized when `classData` method is called if
>> the hidden class has not been initialized.
>> 
>> For a class data containing more than one single element, libraries can
>> create their convenience method to load a single live object via condy.
>> We can reconsider if such a convenience method is needed in the future.
>> 
>> Frameworks sometimes want to dynamically create a hidden class (HC) and add 
>> it
>> it the lookup class nest and have HC to carry secrets hidden from that nest.
>> In this case, frameworks should not to use private static finals (in the HCs
>> they spin) to hold secrets because a nestmate of HC may obtain access to
>> such a private static final and observe the framework's secret.  It should 
>> use
>> condy.  In addition, we need to differentiate if a lookup object is created 
>> from
>> the original lookup class or created from teleporting e.g. `Lookup::in`
>> and `MethodHandles::privateLookupIn`.
>> 
>> This proposes to add a new `ORIGINAL` bit that is only set if the lookup
>> object is created by `MethodHandles::lookup` or by bootstrap method 
>> invocation.
>> The operations only apply to a Lookup object with original access are:
>>- create method handles for caller-sensitve methods
>>- obtain class data associated with the lookup class
>> 
>> No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which
>> ignores the ORIGINAL bit.
>> 
>> 
>> Compatibility Risks
>> ---
>> 
>> `Lookup::lookupModes` includes a new `ORIGINAL` bit.  Most lookup operations
>> ignore this original bit except 

Re: RFR: 8230501: Class data support for hidden classes

[Jorn Vernee]

On Wed, 11 Nov 2020 18:52:10 GMT, Mandy Chung  wrote:

> Provide the `Lookup::defineHiddenClassWithClassData` API that allows live 
> objects
> be shared between a hidden class and other classes.  A hidden class can load
> these live objects as dynamically-computed constants via this API.
> 
> Specdiff
> http://cr.openjdk.java.net/~mchung/jdk16/webrevs/8230501/specdiff/overview-summary.html
> 
> With this class data support and hidden classes, 
> `sun.misc.Unsafe::defineAnonymousClass`
> will be deprecated for removal.  Existing libraries should replace their
> calls to `sun.misc.Unsafe::defineAnonymousClass` with 
> `Lookup::defineHiddenClass`
> or `Lookup::defineHiddenClassWithClassData`.
> 
> This patch also updates the implementation of lambda meta factory and
> `MemoryAccessVarHandleGenerator` to use class data.   No performance 
> difference
> observed in the jdk.incubator.foreign microbenchmarks.   A side note: 
> `MemoryAccessVarHandleGenerator` is removed in the upcoming integration of 
> JDK-8254162 but it helps validating the class data support.
> 
> Background
> --
> 
> This is an enhancement following up JEP 371: Hidden Classes w.r.t.
> "Constant-pool patching" in the "Risks and Assumption" section.
> 
> A VM-anonymous class can be defined with its constant-pool entries already
> resolved to concrete values. This allows critical constants to be shared
> between a VM-anonymous class and the language runtime that defines it, and
> between multiple VM-anonymous classes. For example, a language runtime will
> often have `MethodHandle` objects in its address space that would be useful
> to newly-defined VM-anonymous classes. Instead of the runtime serializing
> the objects to constant-pool entries in VM-anonymous classes and then
> generating bytecode in those classes to laboriously `ldc` the entries,
> the runtime can simply supply `Unsafe::defineAnonymousClass` with references
> to its live objects. The relevant constant-pool entries in the newly-defined
> VM-anonymous class are pre-linked to those objects, improving performance
> and reducing footprint. In addition, this allows VM-anonymous classes to
> refer to each other: Constant-pool entries in a class file are based on names.
> They thus cannot refer to nameless VM-anonymous classes. A language runtime 
> can,
> however, easily track the live Class objects for its VM-anonymous classes and
> supply them to `Unsafe::defineAnonymousClass`, thus pre-linking the new 
> class's
> constant pool entries to other VM-anonymous classes.
> 
> This extends the hidden classes to allow live objects to be injected
> in a hidden class and loaded them via condy.
> 
> Details
> ---
> 
> A new `Lookup::defineHiddenClassWithClassData` API takes additional
> `classData` argument compared to `Lookup::defineHiddenClass`.
> Class data can be method handles, lookup objects, arbitrary user objects
> or collections of all of the above.
> 
> This method behaves as if calling `Lookup::defineHiddenClass` to define
> a hidden class with a private static unnamed field that is initialized
> with `classData` at the first instruction of the class initializer.
> 
> `MethodHandles::classData(Lookup lookup, String name, Class type)`
> is a bootstrap method to load the class data of the given lookup's lookup 
> class.
> The hidden class will be initialized when `classData` method is called if
> the hidden class has not been initialized.
> 
> For a class data containing more than one single element, libraries can
> create their convenience method to load a single live object via condy.
> We can reconsider if such a convenience method is needed in the future.
> 
> Frameworks sometimes want to dynamically create a hidden class (HC) and add it
> it the lookup class nest and have HC to carry secrets hidden from that nest.
> In this case, frameworks should not to use private static finals (in the HCs
> they spin) to hold secrets because a nestmate of HC may obtain access to
> such a private static final and observe the framework's secret.  It should use
> condy.  In addition, we need to differentiate if a lookup object is created 
> from
> the original lookup class or created from teleporting e.g. `Lookup::in`
> and `MethodHandles::privateLookupIn`.
> 
> This proposes to add a new `ORIGINAL` bit that is only set if the lookup
> object is created by `MethodHandles::lookup` or by bootstrap method 
> invocation.
> The operations only apply to a Lookup object with original access are:
>- create method handles for caller-sensitve methods
>- obtain class data associated with the lookup class
> 
> No change to `Lookup::hasFullPrivilegeAccess` and `Lookup::toString` which
> ignores the ORIGINAL bit.
> 
> 
> Compatibility Risks
> ---
> 
> `Lookup::lookupModes` includes a new `ORIGINAL` bit.  Most lookup operations
> ignore this original bit except creating method handles for caller-sensitive 
> methods
> that expects the lookup from the original lookup 

Re: RFR: 8255787: Tag container tests that use cGroups with cgroups keyword

[Harold Seigel]

On Wed, 11 Nov 2020 00:27:59 GMT, Serguei Spitsyn  wrote:

>> Please review this small change to add a cgroups keyword to tests that use 
>> cgroups.  The fix was tested by running Mach5 container tests.
>
> Hi Harold,
> 
> The fix looks good.
> 
> Thanks,
> Serguei

Thanks Serguei!
Harold

-

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

Integrated: 8255787: Tag container tests that use cGroups with cgroups keyword

[Harold Seigel]

On Tue, 10 Nov 2020 21:24:25 GMT, Harold Seigel  wrote:

> Please review this small change to add a cgroups keyword to tests that use 
> cgroups.  The fix was tested by running Mach5 container tests.

This pull request has now been integrated.

Changeset: 4df8abc2
Author:Harold Seigel 
URL:   https://git.openjdk.java.net/jdk/commit/4df8abc2
Stats: 20 lines in 15 files changed: 15 ins; 0 del; 5 mod

8255787: Tag container tests that use cGroups with cgroups keyword

Reviewed-by: sspitsyn

-

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

Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v5]

[Alan Bateman]

On Thu, 12 Nov 2020 04:23:37 GMT, Hui Shi  wrote:

>> src/java.base/share/classes/jdk/internal/reflect/NativeConstructorAccessorImpl.java
>>  line 44:
>> 
>>> 42: private DelegatingConstructorAccessorImpl parent;
>>> 43: private int numInvocations;
>>> 44: private int generated;
>> 
>> What is the reason for using an int? I remember there was a suggestion for 
>> three states but two states seems okay so curious why it was changed from 
>> boolean to int.
>> The restoring to 0 in the event of failure should probably be a 
>> volatile-write. Might be clearer to declare it as a volatile.
>
> @AlanBateman 
> 
>> What is the reason for using an int? I remember there was a suggestion for 
>> three states but two states seems okay so curious why it was changed from 
>> boolean to int.
> 
> shipilev suggested not to use sub-word CAS, so change compareAndSetBoolean to 
> compareAndSetInt. Change field "generated" from boolean to int doesn't 
> increase object size (no extra byte/boolean to fold into same word).
> 
>> The restoring to 0 in the event of failure should probably be a 
>> volatile-write. Might be clearer to declare it as a volatile.
> 
> "generated" field  is read once and CAS once in this method. It doesn't 
> likely cached and no visibiliy order required.

okay, although I think it would be clearer for maintainers to declare it as 
volatile because it is set with CAS.

-

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

Re: RFR: 8254162: Implementation of Foreign-Memory Access API (Third Incubator) [v29]

[Maurizio Cimadamore]

> This patch contains the changes associated with the third incubation round of 
> the foreign memory access API incubation  (see JEP 393 [1]). This iteration 
> focus on improving the usability of the API in 3 main ways:
> 
> * first, by providing a way to obtain truly *shared* segments, which can be 
> accessed and closed concurrently from multiple threads
> * second, by providing a way to register a memory segment against a 
> `Cleaner`, so as to have some (optional) guarantee that the memory will be 
> deallocated, eventually
> * third, by not requiring users to dive deep into var handles when they first 
> pick up the API; a new `MemoryAccess` class has been added, which defines 
> several useful dereference routines; these are really just thin wrappers 
> around memory access var handles, but they make the barrier of entry for 
> using this API somewhat lower.
> 
> A big conceptual shift that comes with this API refresh is that the role of 
> `MemorySegment` and `MemoryAddress` is not the same as it used to be; it used 
> to be the case that a memory address could (sometimes, not always) have a 
> back link to the memory segment which originated it; additionally, memory 
> access var handles used `MemoryAddress` as a basic unit of dereference.
> 
> This has all changed as per this API refresh;  now a `MemoryAddress` is just 
> a dumb carrier which wraps a pair of object/long addressing coordinates; 
> `MemorySegment` has become the star of the show, as far as dereferencing 
> memory is concerned. You cannot dereference memory if you don't have a 
> segment. This improves usability in a number of ways - first, it is a lot 
> easier to wrap native addresses (`long`, essentially) into a `MemoryAddress`; 
> secondly, it is crystal clear what a client has to do in order to dereference 
> memory: if a client has a segment, it can use that; otherwise, if the client 
> only has an address, it will have to create a segment *unsafely* (this can be 
> done by calling `MemoryAddress::asSegmentRestricted`).
> 
> A list of the API, implementation and test changes is provided below. If  you 
> have any questions, or need more detailed explanations, I (and the  rest of 
> the Panama team) will be happy to point at existing discussions,  and/or to 
> provide the feedback required. 
> 
> A big thank to Erik Osterlund, Vladimir Ivanov and David Holmes, without whom 
> the work on shared memory segment would not have been possible; also I'd like 
> to thank Paul Sandoz, whose insights on API design have been very helpful in 
> this journey.
> 
> Thanks 
> Maurizio 
> 
> Javadoc: 
> 
> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/javadoc/jdk/incubator/foreign/package-summary.html
> 
> Specdiff: 
> 
> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/specdiff/jdk/incubator/foreign/package-summary.html
> 
> CSR: 
> 
> https://bugs.openjdk.java.net/browse/JDK-8254163
> 
> 
> 
> ### API Changes
> 
> * `MemorySegment`
>   * drop factory for restricted segment (this has been moved to 
> `MemoryAddress`, see below)
>   * added a no-arg factory for a native restricted segment representing 
> entire native heap
>   * rename `withOwnerThread` to `handoff`
>   * add new `share` method, to create shared segments
>   * add new `registerCleaner` method, to register a segment against a cleaner
>   * add more helpers to create arrays from a segment e.g. `toIntArray`
>   * add some `asSlice` overloads (to make up for the fact that now segments 
> are more frequently used as cursors)
>   * rename `baseAddress` to `address` (so that `MemorySegment` can implement 
> `Addressable`)
> * `MemoryAddress`
>   * drop `segment` accessor
>   * drop `rebase` method and replace it with `segmentOffset` which returns 
> the offset (a `long`) of this address relative to a given segment
> * `MemoryAccess`
>   * New class supporting several static dereference helpers; the helpers are 
> organized by carrier and access mode, where a carrier is one of the usual 
> suspect (a Java primitive, minus `boolean`); the access mode can be simple 
> (e.g. access base address of given segment), or indexed, in which case the 
> accessor takes a segment and either a low-level byte offset,or a high level 
> logical index. The classification is reflected in the naming scheme (e.g. 
> `getByte` vs. `getByteAtOffset` vs `getByteAtIndex`).
> * `MemoryHandles`
>   * drop `withOffset` combinator
>   * drop `withStride` combinator
>   * the basic memory access handle factory now returns a var handle which 
> takes a `MemorySegment` and a `long` - from which it is easy to derive all 
> the other handles using plain var handle combinators.
> * `Addressable`
>   * This is a new interface which is attached to entities which can be 
> projected to a `MemoryAddress`. For now, both `MemoryAddress` and 
> `MemorySegment` implement it; we have plans, with JEP 389 [2] to add more 
> implementations. Clients can largely ignore this interface, which comes in 
> really handy when 

Re: RFR: 8256063: Module::getPackages on an unnamed module may return packages that are in a named module

[Chris Hegarty]

On Wed, 11 Nov 2020 23:02:49 GMT, Mandy Chung  wrote:

> If `Module::getPackages` is invoked on the unnamed module of a class loader
> then the set of packages returned will include the packages of any named 
> modules
> that are defined to that class loader.  This is a spec and implementation bug.
> 
> `Module::getPackages` is changed to return the packages that are defined in
> the unnamed module. 
> 
> CSR: https://bugs.openjdk.java.net/browse/JDK-8256242

Marked as reviewed by chegar (Reviewer).

-

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