Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v16]

[Paul Sandoz]

On Wed, 30 Mar 2022 21:51:16 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-424 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/424
>
> Maurizio Cimadamore has updated the pull request with a new target base due 
> to a merge or a rebase. The pull request now contains 31 commits:
> 
>  - Merge branch 'master' into foreign-preview
>  - Tweak FunctionDescriptor::argumentLayouts to return an immutable list
>  - Fix bad usage of `@link` with primitive array types
>  - Switch to daemon threads for async upcalls
>  - Use thread local storage to optimize attach of async threads
>  - Drop support for Constable from MemoryLayout/FunctionDescriptor
>  - Merge branch 'master' into foreign-preview
>  - Revert changes to RunTests.gmk
>  - Add --enable-preview to micro benchmark java options
>  - Address more review comments
>  - ... and 21 more: 
> https://git.openjdk.java.net/jdk/compare/ce27d9dd...247e5eb5

Java code looks good (i did not go through the tests). As is common no 
comments, since code was reviewed in smaller steps in the panama-foreign 
respository.

-

Marked as reviewed by psandoz (Reviewer).

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

Re: RFR: 8276025: Hotspot's libsvml.so may conflict with user dependency [v2]

[Paul Sandoz]

On Thu, 4 Nov 2021 19:49:08 GMT, Sandhya Viswanathan  
wrote:

>> This patch removes conflicts with libsvml.so distributed with Intel's MKL 
>> library:
>>   Renames exported symbols from __svml to __jsvml.
>>   Renames library from libsvml.so to libjsvml.so.
>>   Updates the stubGenerator_x86_64.cpp  accordingly to load libjsvml.so and 
>> the renamed symbols.
>>   Updates tests to look for the new library.
>> 
>> Please review.
>> 
>> Best Regards,
>> Sandhya
>
> Sandhya Viswanathan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   change filename to jsvml*

I did a case insensitive regex search `[^j]svml` over the checked out PR and 
did not find anything relevant that was missed.

-

Marked as reviewed by psandoz (Reviewer).

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

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v12]

[Paul Sandoz]

On Mon, 1 Nov 2021 22:36:40 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-419 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/419
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Tweak javadoc of loaderLookup

Marked as reviewed by psandoz (Reviewer).

-

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

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v10]

[Paul Sandoz]

On Tue, 2 Nov 2021 10:30:42 GMT, Maurizio Cimadamore  
wrote:

>> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/Utils.java line 
>> 111:
>> 
>>> 109: class VarHandleCache {
>>> 110: private static final Map handleMap 
>>> = new ConcurrentHashMap<>();
>>> 111: private static final Map 
>>> handleMapNoAlignCheck = new ConcurrentHashMap<>();
>> 
>> Something to consider later if this is an issue. Since the number of 
>> `ValueLayout` instances is fixed, carrier x order = 18, we can use stable 
>> arrays with ordinals on the instances.
>
> What about alignment?

Drat, `skipAlignmentCheck` misled me but perhaps there is still benefit for 
common constants with 8 bit and size alignment and fallback otherwise.

-

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

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v10]

[Paul Sandoz]

On Mon, 1 Nov 2021 12:05:32 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-419 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/419
>
> Maurizio Cimadamore has updated the pull request with a new target base due 
> to a merge or a rebase. The pull request now contains 17 commits:
> 
>  - Add cache for memory address var handles
>  - Merge branch 'master' into JEP-419
>  - Fix regression in VaList treatment on AArch64 (contributed by @nick-arm)
>  - Merge branch 'master' into JEP-419
>  - Fix copyright header in TestArrayCopy
>  - Fix failing microbenchmarks. Contributed by @FrauBoes (thanks!)
>  - * use `invokeWithArguments` to simplify new test
>  - Add test for liveness check with high-aririty downcalls
>(make sure that if an exception occurs in a downcall because of liveness,
>ref count of other resources are left intact).
>  - * Fix javadoc issue in VaList
>* Fix bug in concurrent logic for shared scope acquire
>  - Address review comments
>  - ... and 7 more: 
> https://git.openjdk.java.net/jdk/compare/5bb1992b...9b519343

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

> 109: class VarHandleCache {
> 110: private static final Map handleMap = 
> new ConcurrentHashMap<>();
> 111: private static final Map 
> handleMapNoAlignCheck = new ConcurrentHashMap<>();

Something to consider later if this is an issue. Since the number of 
`ValueLayout` instances is fixed, carrier x order = 18, we can use stable 
arrays with ordinals on the instances.

-

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

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v2]

[Paul Sandoz]

On Tue, 12 Oct 2021 20:51:02 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-419 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/419
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix TestLinkToNativeRBP test

Like with previous reviews of code for Panama JEPs there are not many comments 
on this PR, since prior reviews occurred for PRs in the panama-foreign repo.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 
77:

> 75:  * Furthermore, if the function descriptor's return layout is a group 
> layout, the resulting downcall method handle accepts
> 76:  * an extra parameter of type {@link SegmentAllocator}, which is used by 
> the linker runtime to allocate the
> 77:  * memory region associated with the struct returned by  the downcall 
> method handle.

Suggestion:

 * memory region associated with the struct returned by the downcall method 
handle.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 
88:

> 86:  * in which the specialized signature of a given variable arity callsite 
> is described in full. Alternatively,
> 87:  * if the foreign library allows it, clients might also be able to 
> interact with variable arity methods
> 88:  * using by passing a trailing parameter of type {@link VaList}.

Suggestion:

 * by passing a trailing parameter of type {@link VaList}.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 
145:

> 143:  * Lookup a symbol in the standard libraries associated with this 
> linker.
> 144:  * The set of symbols available for lookup is unspecified, as it 
> depends on the platform and on the operating system.
> 145:  * @return a linker-specific library lookup which is suitable to 
> find symbols in the standard libraries associated with this linker.

Suggestion:

 * @return a symbol in the standard libraries associated with this linker.

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

> 91: Objects.requireNonNull(argLayouts);
> 92: Arrays.stream(argLayouts).forEach(Objects::requireNonNull);
> 93: return new FunctionDescriptor(null, argLayouts);

We need to clone `argLayouts`, otherwise the user can modify the contents. 
Internally, using `List.of` is useful, since it does the cloning and null 
checks, and that is the type that is returned. Also `argumentLayouts` uses 
`Arrays.asList` which supports modification of the contents.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/SegmentAllocator.java
 line 394:

> 392:  * 
> 393:  * The returned allocator might throw an {@link OutOfMemoryError} if 
> the total memory allocated with this allocator
> 394:  * exceeds the arena size, or the system capacity. Furthermore, the 
> returned allocator is not thread safe, and all

The "the returned allocator is not thread safe" contradicts the prior sentence 
"An allocator associated with a shared resource scope is thread-safe 
and allocation requests may be performed concurrently".

Perhaps just say:
"

The returned allocator is not thread safe if the given scope is a shared scope. 
Concurrent allocation needs to be guarded with synchronization primitives. 
"

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
 line 260:

> 258: 
> 259: @Override
> 260: public final MemorySegment asOverlappingSlice(MemorySegment other) {

Please ignore these comments if you wish.

Adding `max() // exclusive` to complement `min()` might be useful.

In these cases i find it easier to sort the segments e.g. `var a = this; var b 
= other; if (a.min() > b.min()) { // swap a and b }` then the subsequent logic 
tends to get simpler e.g. overlap test is `if (b.min() < a.max())`, 
`segmentOffset` is always +ve.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/ConfinedScope.java 
line 100:

> 98: do {
> 99: value = (int)ASYNC_RELEASE_COUNT.getVolatile(this);
> 100: } while (!ASYNC_RELEASE_COUNT.compareAndSet(this, value, 
> value + 1));

Use `getAndAdd` (and ignore the return value).

-

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

Re: RFR: 8268768: idea.sh has been updated in surprising and incompatible ways

[Paul Sandoz]

On Tue, 15 Jun 2021 14:04:56 GMT, Maurizio Cimadamore  
wrote:

> As the title says (please also refer to the JBS issue which describes all the 
> issues in more details), the IDE support for IntelliJ has been updated with 
> many enhancements as part of a seemingly innocuous "path handling" fix. The 
> IDE doesn't appear to be usable in the same way it was in the past and many 
> functionalities have been broken as a result (including support for jtreg 
> test execution using the plugin).
> 
> For the above reasons, I'm reverting the plugin and idea.sh code to last 
> known working version. Any _targeted_ fix can be re-applied after the revert. 
> Larger enhancements need to be discussed in the proper venue:
> 
> https://openjdk.java.net/groups/ide-support/

Agreed, let's back up and reevaluate.

-

Marked as reviewed by psandoz (Reviewer).

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

Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v12]

[Paul Sandoz]

On Fri, 4 Jun 2021 09:46:31 GMT, Jan Lahoda  wrote:

>> This is a preview of a patch implementing JEP 406: Pattern Matching for 
>> switch (Preview):
>> https://bugs.openjdk.java.net/browse/JDK-8213076
>> 
>> The current draft of the specification is here:
>> http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html
>> 
>> A summary of notable parts of the patch:
>> -to support cases expressions and patterns in cases, there is a new common 
>> superinterface for expressions and patterns, `CaseLabelTree`, which 
>> expressions and patterns implement, and a list of case labels is returned 
>> from `CaseTree.getLabels()`.
>> -to support `case default`, there is an implementation of `CaseLabelTree` 
>> that represents it (`DefaultCaseLabelTree`). It is used also to represent 
>> the conventional `default` internally and in the newly added methods.
>> -in the parser, parenthesized patterns and expressions need to be 
>> disambiguated when parsing case labels.
>> -Lower has been enhanced to handle `case null` for ordinary 
>> (boxed-primitive, String, enum) switches. This is a bit tricky for boxed 
>> primitives, as there is no value that is not part of the input domain so 
>> that could be used to represent `case null`. Requires a bit shuffling with 
>> values.
>> -TransPatterns has been enhanced to handle the pattern matching switch. It 
>> produces code that delegates to a new bootstrap method, that will classify 
>> the input value to the switch and return the case number, to which the 
>> switch then jumps. To support guards, the switches (and the bootstrap 
>> method) are restartable. The bootstrap method as such is written very simply 
>> so far, but could be much more optimized later.
>> -nullable type patterns are `case String s, null`/`case null, String 
>> s`/`case null: case String s:`/`case String s: case null:`, handling of 
>> these required a few tricks in `Attr`, `Flow` and `TransPatterns`.
>> 
>> The specdiff for the change is here (to be updated):
>> http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixing typo.

A really nice feature, and a significant amount of work in javac. I mostly 
focused on the bootstrap and API aspects, and left some minor comments (most of 
which you can choose to apply or not as you see fit).

I suspect the bootstrap might evolve as we get feedback and switch is enhanced 
with further forms of matching. But, at the moment it looks good.

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 87:

> 85:  * returns {@literal -1}.
> 86:  * 
> 87:  * the {@code target} is not {@code null}, then the method of the 
> call site

Suggestion:

 * If the {@code target} is not {@code null}, then the method of the call 
site

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 92:

> 90:  * 
> 91:  *   the element is of type {@code Class} and the target value
> 92:  *   is a subtype of this {@code Class}; or

Suggestion:

 *   the element is of type {@code Class} that is assignable
 *   from the target's class; or

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 162:

> 160: return i;
> 161: } else {
> 162: if (label instanceof Integer constant) {

Minor suggestion: use `else if` rather than nest

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 166:

> 164: return i;
> 165: }
> 166: if (target instanceof Character input && 
> constant.intValue() == input.charValue()) {

Use `else if`

src/jdk.compiler/share/classes/com/sun/source/tree/CaseLabelTree.java line 31:

> 29: 
> 30: /**A marker interface for {@code Tree}s that may be used as {@link 
> CaseTree} labels.
> 31:  *

Suggestion:

/**
 * A marker interface for {@code Tree}s that may be used as {@link CaseTree} 
labels.
 *

src/jdk.compiler/share/classes/com/sun/source/tree/DefaultCaseLabelTree.java 
line 30:

> 28: 
> 29: /** A case label that marks {@code default} in {@code case null, default}.
> 30:  * @since 17

Suggestion:

/** 
 * A case label that marks {@code default} in {@code case null, default}.
 *
 * @since 17

test/jdk/java/lang/runtime/SwitchBootstrapsTest.java line 55:

> 53: catch (NoSuchMethodException | IllegalAccessException e) {
> 54: throw new RuntimeException(e);
> 55: }

Suggestion:

catch (ReflectiveOperationException e) {
throw new AssertionError(e, "Should not happen");
}

test/jdk/java/lang/runtime/SwitchBootstrapsTest.java line 73:

> 71: }
> 72: 
> 73: public void testTypes() throws Throwable {

As a follow up issue consider adding tests for `null` values

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v11]

[Paul Sandoz]

On Wed, 19 May 2021 22:16:18 GMT, Sandhya Viswanathan 
 wrote:

>> This PR contains Short Vector Math Library support related changes for 
>> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), 
>> in preparation for when targeted.
>> 
>> Intel Short Vector Math Library (SVML) based intrinsics in native x86 
>> assembly provide optimized implementation for Vector API transcendental and 
>> trigonometric methods.
>> These methods are built into a separate library instead of being part of 
>> libjvm.so or jvm.dll.
>> 
>> The following changes are made:
>>The source for these methods is placed in the jdk.incubator.vector module 
>> under src/jdk.incubator.vector/linux/native/libsvml and 
>> src/jdk.incubator.vector/windows/native/libsvml.
>>The assembly source files are named as “*.S” and include files are named 
>> as “*.S.inc”.
>>The corresponding build script is placed at 
>> make/modules/jdk.incubator.vector/Lib.gmk.
>>Changes are made to build system to support dependency tracking for 
>> assembly files with includes.
>>The built native libraries (libsvml.so/svml.dll) are placed in bin 
>> directory of JDK on Windows and lib directory of JDK on Linux.
>>The C2 JIT uses the dll_load and dll_lookup to get the addresses of 
>> optimized methods from this library.
>> 
>> Build system changes and module library build scripts are contributed by 
>> Magnus (magnus.ihse.bur...@oracle.com).
>> 
>> Looking forward to your review and feedback.
>> 
>> Performance:
>> Micro benchmark Base Optimized Unit Gain(Optimized/Base)
>> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90
>> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05
>> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94
>> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79
>> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55
>> Double128Vector.COS 49.94 245.89 ops/ms 4.92
>> Double128Vector.COSH 26.91 126.00 ops/ms 4.68
>> Double128Vector.EXP 71.64 379.65 ops/ms 5.30
>> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18
>> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44
>> Double128Vector.LOG 61.95 279.84 ops/ms 4.52
>> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03
>> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79
>> Double128Vector.SIN 49.36 240.79 ops/ms 4.88
>> Double128Vector.SINH 26.59 103.75 ops/ms 3.90
>> Double128Vector.TAN 41.05 152.39 ops/ms 3.71
>> Double128Vector.TANH 45.29 169.53 ops/ms 3.74
>> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96
>> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01
>> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78
>> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44
>> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04
>> Double256Vector.COS 58.26 389.77 ops/ms 6.69
>> Double256Vector.COSH 29.44 151.11 ops/ms 5.13
>> Double256Vector.EXP 86.67 564.68 ops/ms 6.52
>> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80
>> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62
>> Double256Vector.LOG 71.52 394.90 ops/ms 5.52
>> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54
>> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05
>> Double256Vector.SIN 57.06 380.98 ops/ms 6.68
>> Double256Vector.SINH 29.40 117.37 ops/ms 3.99
>> Double256Vector.TAN 44.90 279.90 ops/ms 6.23
>> Double256Vector.TANH 54.08 274.71 ops/ms 5.08
>> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35
>> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57
>> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04
>> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32
>> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69
>> Double512Vector.COS 59.88 837.04 ops/ms 13.98
>> Double512Vector.COSH 30.34 172.76 ops/ms 5.70
>> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14
>> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34
>> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34
>> Double512Vector.LOG 74.84 996.00 ops/ms 13.31
>> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72
>> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34
>> Double512Vector.POW 37.42 384.13 ops/ms 10.26
>> Double512Vector.SIN 59.74 728.45 ops/ms 12.19
>> Double512Vector.SINH 29.47 143.38 ops/ms 4.87
>> Double512Vector.TAN 46.20 587.21 ops/ms 12.71
>> Double512Vector.TANH 57.36 495.42 ops/ms 8.64
>> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06
>> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16
>> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44
>> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28
>> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53
>> Double64Vector.COS 23.42 152.01 ops/ms 6.49
>> Double64Vector.COSH 17.34 113.34 ops/ms 6.54
>> Double64Vector.EXP 27.08 203.53 ops/ms 7.52
>> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15
>> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59
>> Double64Vector.LOG 26.75 142.63 ops/ms 5.33
>> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40
>> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38
>> Double64Vector.SIN 23.28 146.91 ops/ms 6.31
>> Double64Vector.SINH 17.62 88.59 ops/ms 5.03
>> Double64Vector.TAN 21.00 86.43 ops/ms 4.12
>> Double64Vector.TANH 23.75 111.35 ops/ms 4.69
>> Float128Vector.ACOS 57.52 110.65 

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v2]

[Paul Sandoz]

On Mon, 3 May 2021 21:41:26 GMT, Paul Sandoz  wrote:

>> Sandhya Viswanathan has updated the pull request with a new target base due 
>> to a merge or a rebase. The pull request now contains six commits:
>> 
>>  - Merge master
>>  - remove whitespace
>>  - Merge master
>>  - Small fix
>>  - cleanup
>>  - x86 short vector math optimization for Vector API
>
> Tier 1 to 3 tests pass for the default set of build profiles.

> Thanks a lot for the review @PaulSandoz @iwanowww @erikj79.
> Paul and Vladimir, I have implemented your review comments. Please take a 
> look.

`case VECTOR_OP_OR` is still present.

-

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

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v9]

[Paul Sandoz]

On Wed, 19 May 2021 03:37:11 GMT, Sandhya Viswanathan 
 wrote:

>> This PR contains Short Vector Math Library support related changes for 
>> [JEP-414 Vector API (Second Incubator)](https://openjdk.java.net/jeps/414), 
>> in preparation for when targeted.
>> 
>> Intel Short Vector Math Library (SVML) based intrinsics in native x86 
>> assembly provide optimized implementation for Vector API transcendental and 
>> trigonometric methods.
>> These methods are built into a separate library instead of being part of 
>> libjvm.so or jvm.dll.
>> 
>> The following changes are made:
>>The source for these methods is placed in the jdk.incubator.vector module 
>> under src/jdk.incubator.vector/linux/native/libsvml and 
>> src/jdk.incubator.vector/windows/native/libsvml.
>>The assembly source files are named as “*.S” and include files are named 
>> as “*.S.inc”.
>>The corresponding build script is placed at 
>> make/modules/jdk.incubator.vector/Lib.gmk.
>>Changes are made to build system to support dependency tracking for 
>> assembly files with includes.
>>The built native libraries (libsvml.so/svml.dll) are placed in bin 
>> directory of JDK on Windows and lib directory of JDK on Linux.
>>The C2 JIT uses the dll_load and dll_lookup to get the addresses of 
>> optimized methods from this library.
>> 
>> Build system changes and module library build scripts are contributed by 
>> Magnus (magnus.ihse.bur...@oracle.com).
>> 
>> Looking forward to your review and feedback.
>> 
>> Performance:
>> Micro benchmark Base Optimized Unit Gain(Optimized/Base)
>> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90
>> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05
>> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94
>> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79
>> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55
>> Double128Vector.COS 49.94 245.89 ops/ms 4.92
>> Double128Vector.COSH 26.91 126.00 ops/ms 4.68
>> Double128Vector.EXP 71.64 379.65 ops/ms 5.30
>> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18
>> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44
>> Double128Vector.LOG 61.95 279.84 ops/ms 4.52
>> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03
>> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79
>> Double128Vector.SIN 49.36 240.79 ops/ms 4.88
>> Double128Vector.SINH 26.59 103.75 ops/ms 3.90
>> Double128Vector.TAN 41.05 152.39 ops/ms 3.71
>> Double128Vector.TANH 45.29 169.53 ops/ms 3.74
>> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96
>> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01
>> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78
>> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44
>> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04
>> Double256Vector.COS 58.26 389.77 ops/ms 6.69
>> Double256Vector.COSH 29.44 151.11 ops/ms 5.13
>> Double256Vector.EXP 86.67 564.68 ops/ms 6.52
>> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80
>> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62
>> Double256Vector.LOG 71.52 394.90 ops/ms 5.52
>> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54
>> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05
>> Double256Vector.SIN 57.06 380.98 ops/ms 6.68
>> Double256Vector.SINH 29.40 117.37 ops/ms 3.99
>> Double256Vector.TAN 44.90 279.90 ops/ms 6.23
>> Double256Vector.TANH 54.08 274.71 ops/ms 5.08
>> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35
>> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57
>> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04
>> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32
>> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69
>> Double512Vector.COS 59.88 837.04 ops/ms 13.98
>> Double512Vector.COSH 30.34 172.76 ops/ms 5.70
>> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14
>> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34
>> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34
>> Double512Vector.LOG 74.84 996.00 ops/ms 13.31
>> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72
>> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34
>> Double512Vector.POW 37.42 384.13 ops/ms 10.26
>> Double512Vector.SIN 59.74 728.45 ops/ms 12.19
>> Double512Vector.SINH 29.47 143.38 ops/ms 4.87
>> Double512Vector.TAN 46.20 587.21 ops/ms 12.71
>> Double512Vector.TANH 57.36 495.42 ops/ms 8.64
>> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06
>> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16
>> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44
>> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28
>> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53
>> Double64Vector.COS 23.42 152.01 ops/ms 6.49
>> Double64Vector.COSH 17.34 113.34 ops/ms 6.54
>> Double64Vector.EXP 27.08 203.53 ops/ms 7.52
>> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15
>> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59
>> Double64Vector.LOG 26.75 142.63 ops/ms 5.33
>> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40
>> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38
>> Double64Vector.SIN 23.28 146.91 ops/ms 6.31
>> Double64Vector.SINH 17.62 88.59 ops/ms 5.03
>> Double64Vector.TAN 21.00 86.43 ops/ms 4.12
>> Double64Vector.TANH 23.75 111.35 ops/ms 4.69
>> Float128Vector.ACOS 57.52 110.65 

Re: RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics [v2]

[Paul Sandoz]

On Wed, 28 Apr 2021 21:11:26 GMT, Sandhya Viswanathan 
 wrote:

>> Intel Short Vector Math Library (SVML) based intrinsics in native x86 
>> assembly provide optimized implementation for Vector API transcendental and 
>> trigonometric methods.
>> These methods are built into a separate library instead of being part of 
>> libjvm.so or jvm.dll.
>> 
>> The following changes are made:
>>The source for these methods is placed in the jdk.incubator.vector module 
>> under src/jdk.incubator.vector/linux/native/libsvml and 
>> src/jdk.incubator.vector/windows/native/libsvml.
>>The assembly source files are named as “*.S” and include files are named 
>> as “*.S.inc”.
>>The corresponding build script is placed at 
>> make/modules/jdk.incubator.vector/Lib.gmk.
>>Changes are made to build system to support dependency tracking for 
>> assembly files with includes.
>>The built native libraries (libsvml.so/svml.dll) are placed in bin 
>> directory of JDK on Windows and lib directory of JDK on Linux.
>>The C2 JIT uses the dll_load and dll_lookup to get the addresses of 
>> optimized methods from this library.
>> 
>> Build system changes and module library build scripts are contributed by 
>> Magnus (magnus.ihse.bur...@oracle.com).
>> 
>> This work is part of second round of incubation of the Vector API.
>> JEP: https://bugs.openjdk.java.net/browse/JDK-8261663
>> 
>> Please review.
>> 
>> Performance:
>> Micro benchmark Base Optimized Unit Gain(Optimized/Base)
>> Double128Vector.ACOS 45.91 87.34 ops/ms 1.90
>> Double128Vector.ASIN 45.06 92.36 ops/ms 2.05
>> Double128Vector.ATAN 19.92 118.36 ops/ms 5.94
>> Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79
>> Double128Vector.CBRT 45.77 208.36 ops/ms 4.55
>> Double128Vector.COS 49.94 245.89 ops/ms 4.92
>> Double128Vector.COSH 26.91 126.00 ops/ms 4.68
>> Double128Vector.EXP 71.64 379.65 ops/ms 5.30
>> Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18
>> Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44
>> Double128Vector.LOG 61.95 279.84 ops/ms 4.52
>> Double128Vector.LOG10 59.34 239.05 ops/ms 4.03
>> Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79
>> Double128Vector.SIN 49.36 240.79 ops/ms 4.88
>> Double128Vector.SINH 26.59 103.75 ops/ms 3.90
>> Double128Vector.TAN 41.05 152.39 ops/ms 3.71
>> Double128Vector.TANH 45.29 169.53 ops/ms 3.74
>> Double256Vector.ACOS 54.21 106.39 ops/ms 1.96
>> Double256Vector.ASIN 53.60 107.99 ops/ms 2.01
>> Double256Vector.ATAN 21.53 189.11 ops/ms 8.78
>> Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44
>> Double256Vector.CBRT 56.45 397.13 ops/ms 7.04
>> Double256Vector.COS 58.26 389.77 ops/ms 6.69
>> Double256Vector.COSH 29.44 151.11 ops/ms 5.13
>> Double256Vector.EXP 86.67 564.68 ops/ms 6.52
>> Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80
>> Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62
>> Double256Vector.LOG 71.52 394.90 ops/ms 5.52
>> Double256Vector.LOG10 65.43 362.32 ops/ms 5.54
>> Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05
>> Double256Vector.SIN 57.06 380.98 ops/ms 6.68
>> Double256Vector.SINH 29.40 117.37 ops/ms 3.99
>> Double256Vector.TAN 44.90 279.90 ops/ms 6.23
>> Double256Vector.TANH 54.08 274.71 ops/ms 5.08
>> Double512Vector.ACOS 55.65 687.54 ops/ms 12.35
>> Double512Vector.ASIN 57.31 777.72 ops/ms 13.57
>> Double512Vector.ATAN 21.42 729.21 ops/ms 34.04
>> Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32
>> Double512Vector.CBRT 56.78 834.38 ops/ms 14.69
>> Double512Vector.COS 59.88 837.04 ops/ms 13.98
>> Double512Vector.COSH 30.34 172.76 ops/ms 5.70
>> Double512Vector.EXP 99.66 1608.12 ops/ms 16.14
>> Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34
>> Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34
>> Double512Vector.LOG 74.84 996.00 ops/ms 13.31
>> Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72
>> Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34
>> Double512Vector.POW 37.42 384.13 ops/ms 10.26
>> Double512Vector.SIN 59.74 728.45 ops/ms 12.19
>> Double512Vector.SINH 29.47 143.38 ops/ms 4.87
>> Double512Vector.TAN 46.20 587.21 ops/ms 12.71
>> Double512Vector.TANH 57.36 495.42 ops/ms 8.64
>> Double64Vector.ACOS 24.04 73.67 ops/ms 3.06
>> Double64Vector.ASIN 23.78 75.11 ops/ms 3.16
>> Double64Vector.ATAN 14.14 62.81 ops/ms 4.44
>> Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28
>> Double64Vector.CBRT 16.47 107.50 ops/ms 6.53
>> Double64Vector.COS 23.42 152.01 ops/ms 6.49
>> Double64Vector.COSH 17.34 113.34 ops/ms 6.54
>> Double64Vector.EXP 27.08 203.53 ops/ms 7.52
>> Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15
>> Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59
>> Double64Vector.LOG 26.75 142.63 ops/ms 5.33
>> Double64Vector.LOG10 25.85 139.71 ops/ms 5.40
>> Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38
>> Double64Vector.SIN 23.28 146.91 ops/ms 6.31
>> Double64Vector.SINH 17.62 88.59 ops/ms 5.03
>> Double64Vector.TAN 21.00 86.43 ops/ms 4.12
>> Double64Vector.TANH 23.75 111.35 ops/ms 4.69
>> Float128Vector.ACOS 57.52 110.65 ops/ms 1.92
>> Float128Vector.ASIN 57.15 117.95 ops/ms 2.06
>> Float128Vector.ATAN 22.52 318.74 

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]

[Paul Sandoz]

On Tue, 17 Nov 2020 22:21:18 GMT, Jim Laskey  wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is found in RandomGenerator and RandomGeneratorFactory. Further description 
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>
> Jim Laskey has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 40 commits:
> 
>  - Merge branch 'master' into 8248862
>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Update package-info.java
>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Updated RandomGeneratorFactory javadoc.
>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Updated documentation for RandomGeneratorFactory.
>  - Merge branch 'master' into 8248862
>  - Merge branch 'master' into 8248862
>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Move RandomGeneratorProperty
>  - Merge branch 'master' into 8248862
>  - 8248862: Implement Enhanced Pseudo-Random Number Generators
>
>Clear up javadoc
>  - 8248862; Implement Enhanced Pseudo-Random Number Generators
>
>remove RandomGeneratorProperty from API
>  - ... and 30 more: 
> https://git.openjdk.java.net/jdk/compare/f7517386...6fe94c68

I am unsure if the intent is also to support external libraries providing 
`RandomGenerator` implementations. Currently there is an implicit contract for 
properties (reflectively invoking a method returning a map with a set of 
entries with known keys). Since the service provider implementation is the 
`RandomGenerator` itself, rather than `RandomGeneratorFactory` it is harder 
expose the meta-data with a clearer contract.

src/java.base/share/classes/java/util/Random.java line 592:

> 590: 
> 591: @Override
> 592: public Spliterator.OfInt makeIntsSpliterator(long index, long fence, 
> int origin, int bound) {

Unsure if this and the other two methods are intended to be public or not, 
since they are at the end of the class and override methods of a module private 
class. In principle there is nothing wrong with such `Spliterator` factories, 
but wonder if they are really needed given the `Stream` returning methods. The 
arrangement of classes makes it awkward to hide these methods.

src/java.base/share/classes/java/util/SplittableRandom.java line 171:

> 169:  * RandomGenerator properties.
> 170:  */
> 171: static Map getProperties() {

With records exiting preview in 16 this map of properties could i think be 
represented as a record instance, with better type safety, where 
`RandomSupport.RandomGeneratorProperty` enum values become typed fields 
declared on the record class. Something to consider after integration perhaps?

src/java.base/share/classes/java/util/SplittableRandom.java line 211:

> 209:  * 
> http://zimbry.blogspot.com/2011/09/better-bit-mixing-improving-on.html
> 210:  */
> 211: private static long mix64(long z) {

Usages be replaced with calls to `RandomSupport.mixStafford13`?

src/java.base/share/classes/module-info.java line 250:

> 248: exports jdk.internal.util.xml.impl to
> 249: jdk.jfr;
> 250: exports jdk.internal.util.random;

Unqualified export, should this be `to jdk.random`?

src/jdk.random/share/classes/module-info.java line 50:

> 48:  */
> 49: module jdk.random {
> 50: uses java.util.random.RandomGenerator;

Are these `uses` declarations needed? `ServiceLoader` is not used by this 
module, and `RandomSupport` is not a service interface.

src/jdk.random/share/classes/module-info.java line 53:

> 51: uses RandomSupport;
> 52: 
> 53: exports jdk.random to

Why is this needed?

src/java.base/share/classes/java/util/random/package-info.java line 50:

> 48:  * given its name.
> 49:  *
> 50:  *  The principal supporting class is {@link RandomGenertatorFactor}. 
> This

s/RandomGenertatorFactor/RandomGenertatorFactory

src/java.base/share/classes/java/util/random/package-info.java line 140:

> 138:  *
> 139:  *  For applications with no special requirements,
> 140:  * "L64X128MixRandom" has a good balance among speed, space,

The documentation assumes that the `jdk.random` module is present in the JDK 
image. Perhaps we need to spit the specifics to `jdk.random`?

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

> 1209: Udiff = -Udiff;
> 1210: U2 = U1;
> 1211: U1 -= Udiff;

Updated `U1` never used (recommend running the code through a checker e.g. use 
IntelliJ)

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

> 329: }
> 330: // Finally, we need to make sure the last z words are not all 
> zero.
> 331: search: {

Nice! a rarely used feature :-)

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

> 1155: /*
> 1156:

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

[Paul Sandoz]

On Thu, 22 Oct 2020 14:26:37 GMT, Maurizio Cimadamore  
wrote:

>> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractNativeScope.java
>>  line 120:
>> 
>>> 118: }
>>> 119: }
>>> 120: throw new AssertionError("Cannot get here!");
>> 
>> This code is a little confusing, effectively using an exception for control 
>> flow, within a retry loop. I recommend performing an explicit bounds check 
>> to determine if a new segment of `BLOCK_SIZE` is required from which to 
>> slice into. It will also be faster than the exceptional case.
>
> I hear you - that said, note that doing the bound check is not as trivial as 
> it seems; you have to take into account the value of `sp` and add required 
> alignment padding, and then perform a bound check against that - all logic 
> which would need to be duplicated across the normal and the exceptional 
> cases. Which is why the code settled the way it is. I'll see what I can do.

Yeah, i would have probably done the same thing initially.

-

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

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

[Paul Sandoz]

On Thu, 22 Oct 2020 14:31:12 GMT, Jorn Vernee  wrote:

>> src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java line 36:
>> 
>>> 34: import static java.lang.invoke.MethodHandleStatics.newInternalError;
>>> 35: 
>>> 36: /** TODO */
>> 
>> Is the TODO to make this class public later and adjust the return type of 
>> `downcallHandle`?
>
> IIRC this was added to silence a javac linter warning. Something should be 
> added here. There is/was no plan to make this class public though.

It's odd the lint warning is triggering on a package private class and private 
methods. Separately, I recommend updating `make/CompileJavaModules.gmk` and 
adding `-Xdoclint:all/protected` for the module (i recently did this for the 
vector API see 
[here](https://github.com/openjdk/jdk/commit/000143504408ac7938e9f493c17c4dbb994045f9#diff-118e609b9974c0ce8af7950711461c7ab4620c9d4f4c99d231f598696f8e05d0)

-

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

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

[Paul Sandoz]

On Thu, 22 Oct 2020 13:30:13 GMT, Maurizio Cimadamore  
wrote:

>> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java 
>> line 126:
>> 
>>> 124:  *
>>> 125:  * @param symbol   downcall symbol.
>>> 126:  * @param type the method type.
>> 
>> s/method type/carrier type ?
>
> Not sure about this one? E.g. in my mental model, I often have seen "carrier 
> type" associated with j.l.Class ?

Ah, i see, i find it confusing that "carrier type" is mentioned in the 
`@throws`, and was assuming it was an alias for method type, did you really 
mean method type?

>> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java 
>> line 139:
>> 
>>> 137:  *
>>> 138:  * The returned segment is not thread-confined, and it 
>>> only features
>>> 139:  * the {@link MemorySegment#CLOSE} access mode. When the returned 
>>> segment is closed,
>> 
>> Implying that it is shared? If so might be better to state that directly 
>> (with a link), and can be closed explicitly or left until can be collected 
>> by the GC?
>
> `The returned segment is not thread-confined` ? Since it features 
> CLOSE, it can be closed explicitly - I'm not sure 100% of what additional 
> clarification is required - but I'm happy to make this clearer (I need more 
> info).

Sometimes it's clearer to state the non-negative term i.e. _shared_ which is 
now something more explicit e.g.
> The returned segment is _shared_ [add link?] (not thread-confined)

That is really what i was trying to get at, rather than the CLOSE+GC aspects.

-

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

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

[Paul Sandoz]

On Tue, 20 Oct 2020 17:23:26 GMT, Maurizio Cimadamore  
wrote:

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

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

[Paul Sandoz]

On Wed, 21 Oct 2020 11:33:27 GMT, Maurizio Cimadamore  
wrote:

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

Integrated: 8255020: Minor updates to docs jdk.incubator.vector

[Paul Sandoz]

On Mon, 19 Oct 2020 18:41:31 GMT, Paul Sandoz  wrote:

> Minor updates, with no specification changes, to the documentation of Vector 
> API.
> 
> The compilation of the Vector module was updated to turn on doclint errors 
> for >= protected documentation.

This pull request has now been integrated.

Changeset: 00014350
Author:Paul Sandoz 
URL:   https://git.openjdk.java.net/jdk/commit/00014350
Stats: 24 lines in 4 files changed: 15 ins; 0 del; 9 mod

8255020: Minor updates to docs jdk.incubator.vector

Reviewed-by: erikj, darcy

-

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

RFR: 8255020: Minor updates to docs jdk.incubator.vector

[Paul Sandoz]

Minor updates, with no specification changes, to the documentation of Vector 
API.

The compilation of the Vector module was updated to turn on doclint errors for 
>= protected documentation.

-

Commit messages:
 - 8255020:Minor updates to docs jdk.incubator.vector

Changes: https://git.openjdk.java.net/jdk/pull/746/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=746=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8255020
  Stats: 24 lines in 4 files changed: 15 ins; 0 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/746.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/746/head:pull/746

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

Integrated: 8223347: Integration of Vector API (Incubator)

[Paul Sandoz]

On Fri, 25 Sep 2020 20:14:29 GMT, Paul Sandoz  wrote:

> This pull request is for integration of the Vector API. It was previously 
> reviewed under conditions when mercurial was
> used for the source code control system. Review threads can be found here 
> (searching for issue number 8223347 in the
> title):  
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html
> 
> If mercurial was still being used the code would be pushed directly, once the 
> CSR is approved. However, in this case a
> pull request is required and needs explicit reviewer approval. Between the 
> final review and this pull request no code
> has changed, except for that related to merging.

This pull request has now been integrated.

Changeset: 0c99b192
Author:Paul Sandoz 
URL:   https://git.openjdk.java.net/jdk/commit/0c99b192
Stats: 295107 lines in 336 files changed: 292957 ins; 1062 del; 1088 mod

8223347: Integration of Vector API (Incubator)

Co-authored-by: Vivek Deshpande 
Co-authored-by: Qi Feng 
Co-authored-by: Ian Graves 
Co-authored-by: Jean-Philippe Halimi 
Co-authored-by: Vladimir Ivanov 
Co-authored-by: Ningsheng Jian 
Co-authored-by: Razvan Lupusoru 
Co-authored-by: Smita Kamath 
Co-authored-by: Rahul Kandu 
Co-authored-by: Kishor Kharbas 
Co-authored-by: Eric Liu 
Co-authored-by: Aaloan Miftah 
Co-authored-by: John R Rose 
Co-authored-by: Shravya Rukmannagari 
Co-authored-by: Paul Sandoz 
Co-authored-by: Sandhya Viswanathan 
Co-authored-by: Lauren Walkowski 
Co-authored-by: Yang Zang 
Co-authored-by: Joshua Zhu 
Co-authored-by: Wang Zhuo 
Co-authored-by: Jatin Bhateja 
Reviewed-by: erikj, chegar, kvn, darcy, forax, briangoetz, aph, epavlova, 
coleenp

-

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

Re: RFR: 8223347: Integration of Vector API (Incubator) [v6]

[Paul Sandoz]

> This pull request is for integration of the Vector API. It was previously 
> reviewed under conditions when mercurial was
> used for the source code control system. Review threads can be found here 
> (searching for issue number 8223347 in the
> title):  
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html
> 
> If mercurial was still being used the code would be pushed directly, once the 
> CSR is approved. However, in this case a
> pull request is required and needs explicit reviewer approval. Between the 
> final review and this pull request no code
> has changed, except for that related to merging.

Paul Sandoz has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now
contains 11 commits:

 - Merge master
 - Merge master
 - Merge master
 - Fix related to merge
 - HotspotIntrinsicCandidate to IntrinsicCandidate
 - Merge master
 - Fix permissions
 - Fix permissions
 - Merge master
 - Vector API new files
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/96a1f08e...3346d292

-

Changes: https://git.openjdk.java.net/jdk/pull/367/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=367=05
  Stats: 295107 lines in 336 files changed: 292957 ins; 1062 del; 1088 mod
  Patch: https://git.openjdk.java.net/jdk/pull/367.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/367/head:pull/367

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

Re: RFR: 8223347: Integration of Vector API (Incubator) [v5]

[Paul Sandoz]

> This pull request is for integration of the Vector API. It was previously 
> reviewed under conditions when mercurial was
> used for the source code control system. Review threads can be found here 
> (searching for issue number 8223347 in the
> title):  
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html
> 
> If mercurial was still being used the code would be pushed directly, once the 
> CSR is approved. However, in this case a
> pull request is required and needs explicit reviewer approval. Between the 
> final review and this pull request no code
> has changed, except for that related to merging.

Paul Sandoz has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now
contains 10 commits:

 - Merge master
 - Merge master
 - Fix related to merge
 - HotspotIntrinsicCandidate to IntrinsicCandidate
 - Merge master
 - Fix permissions
 - Fix permissions
 - Merge master
 - Vector API new files
 - Integration of Vector API (Incubator)

-

Changes: https://git.openjdk.java.net/jdk/pull/367/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=367=04
  Stats: 295107 lines in 336 files changed: 292957 ins; 1062 del; 1088 mod
  Patch: https://git.openjdk.java.net/jdk/pull/367.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/367/head:pull/367

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

Re: RFR: 8223347: Integration of Vector API (Incubator) [v4]

[Paul Sandoz]

> This pull request is for integration of the Vector API. It was previously 
> reviewed under conditions when mercurial was
> used for the source code control system. Review threads can be found here 
> (searching for issue number 8223347 in the
> title):  
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html
> 
> If mercurial was still being used the code would be pushed directly, once the 
> CSR is approved. However, in this case a
> pull request is required and needs explicit reviewer approval. Between the 
> final review and this pull request no code
> has changed, except for that related to merging.

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

 - Merge master
 - Fix related to merge
 - HotspotIntrinsicCandidate to IntrinsicCandidate
 - Merge master
 - Fix permissions
 - Fix permissions
 - Merge master
 - Vector API new files
 - Integration of Vector API (Incubator)

-

Changes: https://git.openjdk.java.net/jdk/pull/367/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=367=03
  Stats: 295107 lines in 336 files changed: 292957 ins; 1062 del; 1088 mod
  Patch: https://git.openjdk.java.net/jdk/pull/367.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/367/head:pull/367

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

Re: RFR: 8223347: Integration of Vector API (Incubator) [v3]

[Paul Sandoz]

> This pull request is for integration of the Vector API. It was previously 
> reviewed under conditions when mercurial was
> used for the source code control system. Review threads can be found here 
> (searching for issue number 8223347 in the
> title):  
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html
> 
> If mercurial was still being used the code would be pushed directly, once the 
> CSR is approved. However, in this case a
> pull request is required and needs explicit reviewer approval. Between the 
> final review and this pull request no code
> has changed, except for that related to merging.

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

  Fix related to merge

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/367/files
  - new: https://git.openjdk.java.net/jdk/pull/367/files/9cca17b8..d5acb4ff

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

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

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

Re: RFR: 8223347: Integration of Vector API (Incubator) [v2]

[Paul Sandoz]

> This pull request is for integration of the Vector API. It was previously 
> reviewed under conditions when mercurial was
> used for the source code control system. Review threads can be found here 
> (searching for issue number 8223347 in the
> title):  
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html
> 
> If mercurial was still being used the code would be pushed directly, once the 
> CSR is approved. However, in this case a
> pull request is required and needs explicit reviewer approval. Between the 
> final review and this pull request no code
> has changed, except for that related to merging.

Paul Sandoz has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now
contains seven commits:

 - HotspotIntrinsicCandidate to IntrinsicCandidate
 - Merge master
 - Fix permissions
 - Fix permissions
 - Merge master
 - Vector API new files
 - Integration of Vector API (Incubator)

-

Changes: https://git.openjdk.java.net/jdk/pull/367/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=367=01
  Stats: 295150 lines in 336 files changed: 292957 ins; 1062 del; 1131 mod
  Patch: https://git.openjdk.java.net/jdk/pull/367.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/367/head:pull/367

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

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

[Paul Sandoz]

On Fri, 9 Oct 2020 11:34:56 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. `

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

[Paul Sandoz]

On Thu, 8 Oct 2020 13:59:20 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. `

Re: RFR: 8223347: Integration of Vector API (Incubator)

[Paul Sandoz]

On Thu, 1 Oct 2020 00:25:46 GMT, Jie Fu  wrote:

>> @DamonFool we can follow up later for that fix (and others in 
>> `vectorIntrinsics`), after this PR integrates. I don't
>> want to perturb the code that has already been reviewed, which requires yet 
>> more additional review.
>
> Hi @PaulSandoz ,
> 
> I think it would be better to integrate it [1] in this MR.
> 
> I have tested this MR on our AVX512 machines and it still crashes.
> Also, for the sake of maintenance, it seems NOT a good idea to push a 
> problematic commit into the jdk main-line repo.
> 
> As for the review process, I don't think it's a problem since the fix [1] is 
> clear and small enough.
> 
> What do you think?
> 
> Thanks.
> 
> [1] 
> https://github.com/openjdk/panama-vector/commit/1af35c357066743935bd3f48ce3610a41761f89a

@DamonFool I appreciate your efforts on this but i want to hold back on that 
issue and follow up very quickly after
integration of this PR. This change has been through an extremely long and 
arduous review process, and i want to stick
to what was reviewed and not ask reviewers to go through further cycles on what 
overall is a very large change.
Unfortunately this change is in a holding pattern waiting for the CSR to be 
approved thereby increasing the window
where we might find further issues (that if we had already integrated may have 
been dealt with separately perhaps in a
less timely fashion with respect to that integration). Unless an issue is 
extremely severe I think we should queue them
up in `panama-vector/vectorIntrinsics` (there is at least one more for ARM SVE 
that is queued up). Since the issue you
describe effects one instruction, for one type, on AVX512, its impact is 
limited and will be mitigated by a quick
follow up.

-

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

RFR: 8223347: Integration of Vector API (Incubator)

[Paul Sandoz]

This pull request is for integration of the Vector API. It was previously 
reviewed under conditions when mercurial was
used for the source code control system. Review threads can be found here 
(searching for issue number 8223347 in the
title):

https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/thread.html
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-May/thread.html
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/thread.html

If mercurial was still being used the code would be pushed directly, once the 
CSR is approved. However, in this case a
pull request is required and needs explicit reviewer approval. Between the 
final review and this pull request no code
has changed, except for that related to merging.

-

Commit messages:
 - Fix permissions
 - Fix permissions
 - Merge master
 - Vector API new files
 - Integration of Vector API (Incubator)

Changes: https://git.openjdk.java.net/jdk/pull/367/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=367=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8223347
  Stats: 295107 lines in 336 files changed: 292957 ins; 1062 del; 1088 mod
  Patch: https://git.openjdk.java.net/jdk/pull/367.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/367/head:pull/367

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

Re: RFR: 8138732: Rename @HotSpotIntrinsicCandidate to @IntrinsicCandidate and move it to the jdk.internal.vm.annotation package [v2]

[Paul Sandoz]

On Wed, 9 Sep 2020 09:49:44 GMT, Philippe Marschall 
 wrote:

>> Hello, newbie here
>> 
>> I picked JDK-8138732 to work on because it has a "starter" label and I 
>> believe I understand what to do.
>> 
>> - I tried to update the copyright year to 2020 in every file.
>> - I decided to change `@since` from 9 to 16 since it is a new annotation 
>> name in a new package.
>> - I tried to keep code changes to a minimum, eg not change to imports if 
>> fully qualified class names are used instead of
>>   imports. In some cases I did minor reordering of imports to keep them 
>> sorted alphabetically.
>> - All tier1 tests pass.
>> - One jpackage/jlink tier2 test fails but I don't believe it is related.
>> - Some tier3 Swing tests fail but I don't think they are related.
>
> Philippe Marschall 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:
>   8138732: Rename HotSpotIntrinsicCandidate to IntrinsicCandidate

Marked as reviewed by psandoz (Reviewer).

-

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

Re: JDK 12 RFR of JDK-8205615: Start of release updates for JDK 12 / JDK-8205621: Increment JDK version for JDK 12

[Paul Sandoz]

This generally looks good. Having it all consolidated helps a lot, and we are 
slowly chipping away at reducing this for each release.

src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java
—

 118 V55(55, 0),   // JDK 11: constant dynamic
 119 V56(56, 0);   // JDK 12


We can add nestmates to the list of stuff in 11

Paul.

> On Jun 26, 2018, at 9:30 AM, joe darcy  wrote:
> 
> Hi Alan,
> 
> 
> On 6/26/2018 2:23 AM, Alan Bateman wrote:
>> On 25/06/2018 19:10, joe darcy wrote:
>>> Hello,
>>> 
>>> With the JDK 11 and 12 split fast approaching [1], it is time to work on 
>>> the various start of release update tasks for JDK 12. Those tasks are being 
>>> tracked under the umbrella bug JDK-8205615: "Start of release updates for 
>>> JDK 12".
>>> 
>>> This thread is to review the build-related portions of the work including 
>>> JDK-8205621: "Increment JDK version for JDK 12." Current webrev:
>>> 
>>> http://cr.openjdk.java.net/~darcy/8205615.4/
> 
>>> 
>>> A handful of test failures still need to be addressed, so there will be 
>>> some minor adjustments to the aggregate set of changes before they are 
>>> pushed.
>>> 
> 
> Slightly modified version now up in
> 
> http://cr.openjdk.java.net/~darcy/8205615.5/
> 
> * Changed projected ship date in make/autoconf/version-numbers
> * Some changes and merges to jdeprscan; I'll ask Stuart to review those
> 
> Some hotspot test changes in the work too based on a separate review thread.
> 
>> I looked through the changes with the exception of javac and test/langtools 
>> and it all looks okay to me. The updates to the tests, for MR JARs in 
>> particular, look fine.
> 
> Hopefully after this round of changes the MR JAR tests won't need any more 
> updates when we go from 12 -> 13 and beyond.
> 
>> 
>> I see David brought up the -source/-target 12 in SetupJavaCompilers.gmk. I 
>> assume it wouldn't be too hard to have that use the value from 
>> DEFAULT_VERSION_FEATURE.
>> 
>> Keeping 10 in the list of DEFAULT_ACCEPTABLE_BOOT_VERSIONS seems right for 
>> now, I assume 10 will be dropped from that list as soon as 11 ships.
>> 
> 
> That is my understanding of the policy discussion from Mark:
> 
> http://mail.openjdk.java.net/pipermail/jdk-dev/2018-April/001075.html
> 
> Thanks,
> 
> -Joe

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

[Paul Sandoz]

> On Mar 1, 2018, at 4:41 PM, Lance Andersen  wrote:
> 
> While running the JDK regression tests, I found a test that needed to be 
> updated, test/langtools/tools/jdeps/modules/DotFileTest.java, due to the 
> update to the java.sql module
> 
> The updated webrev is at 
> http://cr.openjdk.java.net/~lancea/8197533/webrev.03/ 
> 
> 

+1

Paul.

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

[Paul Sandoz]

+1

> On Mar 1, 2018, at 8:59 AM, Lance Andersen  wrote:
>> 
>> +1, i second Joe’s request to update package-info.java while we are 
>> opportunistically cleaning this area up.
> 
> Per your and Joe’s request, please see 
> cr.openjdk.java.net/~lancea/8197533/webrev.02 
>  which has your 
> suggested update to use package-info.java
> 
> Thank you both
> 
> Best
> Lance

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

[Paul Sandoz]

> On Feb 28, 2018, at 1:22 PM, Lance Andersen <lance.ander...@oracle.com> wrote:
> 
>> 
>> On Feb 28, 2018, at 2:20 PM, Lance Andersen <lance.ander...@oracle.com 
>> <mailto:lance.ander...@oracle.com>> wrote:
>> 
>> Hi Paul,
>> 
>> Thank you for the review.
>>> On Feb 28, 2018, at 1:40 PM, Paul Sandoz <paul.san...@oracle.com 
>>> <mailto:paul.san...@oracle.com>> wrote:
>>> 
>>> Compatible module refactoring in action!
>>> 
>>> Looks good, one comment:
>>> 
>>> test/jdk/javax/transaction/xa/testng/JavaSqlModuleDriver.java
>>> 
>>> This is not a valid Java source file can you merge the jtreg meta data into 
>>> XAExceptionTests instead?
>> 
>> As we discussed offline, I will change the above file and Driver.java.  I 
>> naively assumed this was OK as the change  to add Driver.java was made after 
>> I had originally added these tests in 2015.
> 
> http://cr.openjdk.java.net/~lancea/8197533/webrev.01/ 
> <http://cr.openjdk.java.net/~lancea/8197533/webrev.01/> has the updated tests

+1, i second Joe’s request to update package-info.java while we are 
opportunistically cleaning this area up.

Paul.

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

[Paul Sandoz]

Compatible module refactoring in action!

Looks good, one comment:

test/jdk/javax/transaction/xa/testng/JavaSqlModuleDriver.java

This is not a valid Java source file can you merge the jtreg meta data into 
XAExceptionTests instead?

Paul.

> On Feb 28, 2018, at 10:25 AM, Lance Andersen  
> wrote:
> 
> Hi all,
> 
> This RFR request moves the javax.transaction.xa package out of the java.sql 
> module and into its own module java.transaction.xa.  One of the motivators 
> for this change is due to the fact JSR 907 1.3 MR indicated that the 
> javax.transaction.xa package will be subsumed by Java SE.
> 
> There should be no compatibility issues with this change. Any module that 
> `requires java.sql` will continue to have access to the public classes in the 
> javax.transaction.xa package at both compile-time and run-time. 
> 
> 
> The CSR has been approved
> 
> The webrev can be found at: 
> http://cr.openjdk.java.net/~lancea/8197533/webrev.00/
> 
> Best
> Lance
> 
>  
> 
> Lance Andersen| 
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> lance.ander...@oracle.com 
> 
> 
> 

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 18 Dec 2017, at 17:52, David Holmes <david.hol...@oracle.com> wrote:
> 
> On 19/12/2017 11:42 AM, Paul Sandoz wrote:
>> Looks good to me.
>> I am including HS dev for the class file version changes:
>> http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-classfile-version/webrev/
>> http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-classfile-version-build-changes/webrev/
>> We can push the above patches under the following issue rather that merge 
>> into Joe's patch:
>> https://bugs.openjdk.java.net/browse/JDK-8191913
>> Hopefully by the next release we can merge together and with less changes 
>> required.
> 
> Changes seem okay, but I'm still a little concerned about using classfile 
> version 55 before we actually bump the JDK version to 11.
> 

Thanks, jdk and hs 1 to 4 tier testing showed no issues.


> And it still isn't clear to me who will be doing the actual version update 
> and when?
> 

Unsure, we are still working out the steps, hopefully it would follow on fairly 
quickly.

Plus there are quite a few follow on actions we can do to make this simpler for 
next time (especially for hardcoded values in build scripts and tests).

Paul.

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

Looks good to me.

I am including HS dev for the class file version changes:

  http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-classfile-version/webrev/ 

  
http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-classfile-version-build-changes/webrev/
 


We can push the above patches under the following issue rather that merge into 
Joe's patch:

  https://bugs.openjdk.java.net/browse/JDK-8191913 


Hopefully by the next release we can merge together and with less changes 
required.

Paul.

> On 18 Dec 2017, at 17:33, Joseph D. Darcy  wrote:
> 
> Hello,
> 
> A follow-up on defining a final field RELEASE_CURRENT  as an alias for the 
> latest SourceVersion constant to allow easier updating of annotations using 
> SourceVersion constants. Somewht surprisingly, this is not legal Java code; 
> an annotation must use an enum constant directly and not a constant 
> expression which evaluates to an enum constant.
> 
> This rules is given in JLS 9.7.1 Normal Annotations:
> 
>> T [ the element type] is not an array type, and the type of V [the element 
>> value] is assignment compatible (§5.2) with T, and:
>> 
>>If T is a primitive type or String, then V is a constant expression 
>> (§15.28).
>>If T is Class or an invocation of Class (§4.5), then V is a class literal 
>> (§15.8.2).
>>If T is an enum type (§8.9), then V is an enum constant (§8.9.1).
>>V is not null.
> https://docs.oracle.com/javase/specs/jls/se9/html/jls-9.html#jls-9.7.1
> 
> The wording goes back to JLS 3rd edition which introduced enums and 
> annotations.
> 
> Formally, the constant expression concept in JLS 15.28 only applies to 
> primitive types and Strings. Presumably, it would not be too difficult to 
> expand this concept to include class literals and enum constants.
> 
> In any case, in the mean time the suggested idiom will not work and the 
> changes of RELEASE_9 => RELEASE_10 will need to stay.
> 
> Updated webrev with a minor merge:
> 
>http://cr.openjdk.java.net/~darcy/8173382.3/
> 
> Cheers,
> 
> -Joe
> 

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

Here are two webrevs (in order that can be merged with the other): the class 
file version changes; and the build changes related to that.

  http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-classfile-version/webrev/
  
http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-classfile-version-build-changes/webrev/

Testing of all three patches 
(jdk-tier1,jdk-tier2,jdk-tier3,hs-tier1,hs-tier2,hs-tier3,hs-tier4,hs-tier5) 
produced no related test failures.

Paul.


> On 11 Dec 2017, at 23:04, joe darcy  wrote:
> 
> Hello,
> 
> With the JDK 11 line of development opening up in a few days, a few changes 
> are needed to prepared to turn a JDK 10 code base into a JDK 11 one including:
> 
>JDK-8173382: Add -source 11 and -target 11 to javac
>JDK-8193291: Add SourceVersion.RELEASE_11
> 
>Webev:
>http://cr.openjdk.java.net/~darcy/8173382.1/
>CSRs:
>JDK-8193350: Add -source 11 and -target 11 to javac
>JDK-8193351: Add SourceVersion.RELEASE_11
> 
> Please review the preliminary webrev and CSRs.
> 
> Note that breaking with previous contentions, the current iteration of the 
> change does *not* add -source/-target "1.11" as an alias for "11". Only "11" 
> is recognized.
> 
> I didn't see how to add support for an "11" value for "--release" so I 
> problem listed two tests until that is sorted out.
> 
> The build is simultaneously updated to user -source/-target 11, hence the 
> inclusion of build-dev.
> 
> Various langtools tests and test infrastructure is updated too.
> 
> Thanks,
> 
> -Joe
> 

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

Hi Iris, Erik

We should be able to automate all the following by referring to the latest 
release variable, yes?

  Nashorn build targets version 9 source
  https://bugs.openjdk.java.net/browse/JDK-8188012
  http://hg.openjdk.java.net/jdk10/master/rev/0e67ab18b511

  symbolgenerator targets jdk 9 source
  https://bugs.openjdk.java.net/browse/JDK-8188013
  http://hg.openjdk.java.net/jdk10/master/rev/b4c8426fe105

  Update link to license in Docs.gmk
  https://bugs.openjdk.java.net/browse/JDK-818999
  http://hg.openjdk.java.net/jdk10/master/rev/a6e591e12f12

Paul.


> On 15 Dec 2017, at 08:43, Iris Clark  wrote:
> 
> Hi.
> 
> Another build target we need to update is LICENSE_URL in make/Docs.gmk.  
> Here's the equivalent bug for 10:
> 
>https://bugs.openjdk.java.net/browse/JDK-8189919
> 
> I've requested this link:
> 
>
> http://www.oracle.com/technetwork/java/javase/terms/license/java11speclicense.html
> 
> Thanks,
> iris
> 
> -Original Message-
> From: Erik Joelsson
> Sent: Friday, December 15, 2017 4:41 AM
> To: joe darcy ; compiler-...@openjdk.java.net; 
> build-dev ; STUART.MARKS 
> Cc: David Holmes 
> Subject: Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 
> to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11
> 
> Looking at JDK-8188015, I was just reminded of JDK-8188012 and
> JDK-8188013 which indicates that we have more than one place where 10 needs 
> to be bumped to 11
> 
> make/BuildNashorn.gmk
> make/langtools/src/classes/build/tools/symbolgenerator/TransitiveDependencies.java
> 
> /Erik
> 
> On 2017-12-12 16:36, Erik Joelsson wrote:
>> Build change looks good.
>> 
>> /Erik
>> 
>> 
>> On 2017-12-12 08:04, joe darcy wrote:
>>> Hello,
>>> 
>>> With the JDK 11 line of development opening up in a few days, a few
>>> changes are needed to prepared to turn a JDK 10 code base into a JDK
>>> 11 one including:
>>> 
>>> JDK-8173382: Add -source 11 and -target 11 to javac
>>> JDK-8193291: Add SourceVersion.RELEASE_11
>>> 
>>> Webev:
>>> http://cr.openjdk.java.net/~darcy/8173382.1/
>>> CSRs:
>>> JDK-8193350: Add -source 11 and -target 11 to javac
>>> JDK-8193351: Add SourceVersion.RELEASE_11
>>> 
>>> Please review the preliminary webrev and CSRs.
>>> 
>>> Note that breaking with previous contentions, the current iteration
>>> of the change does *not* add -source/-target "1.11" as an alias for
>>> "11". Only "11" is recognized.
>>> 
>>> I didn't see how to add support for an "11" value for "--release" so
>>> I problem listed two tests until that is sorted out.
>>> 
>>> The build is simultaneously updated to user -source/-target 11, hence
>>> the inclusion of build-dev.
>>> 
>>> Various langtools tests and test infrastructure is updated too.
>>> 
>>> Thanks,
>>> 
>>> -Joe
>>> 
>> 
> 

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 13 Dec 2017, at 19:51, mark.reinh...@oracle.com wrote:
> 
> 2017/12/13 15:49:53 -0800, paul.san...@oracle.com:
>> On 13 Dec 2017, at 14:36, mark.reinh...@oracle.com wrote:
>>> I understand that other incoming changes are waiting for the 11 version
>>> bump, but can't we at least do the straightforward parameterizations and
>>> introduce _CURRENT constants in this round?  We can leave anything that
>>> requires code generation to post-11.
>> 
>> On the lang tools side that does seem easy, but I’ll defer to Joe.
>> 
>> With some guidance from the build engineers i would gladly consolidate
>> class file version declaration in C/++ files with a passed in macro,
>> if this can be expediently worked out. I wanna push condy as early as
>> possible in 11 :-)
> 
> Understood, but let's do the right thing (or at least closer to that)
> with this yak while we're here.
> 
>> If we could do something in jvm.h that would be ideal. We already have
>> the definitions JVM_CLASSFILE_MAJOR_VERSION and
>> JVM_CLASSFILE_MINOR_VERSION pulled in via classfile_constants.h, so
>> how can the build process inject values into those macros?
> 
> I just went through this for the versioning changes.  The attached patch
> (against d2a837cf9ff1) does what you want, I think.
> 

Thanks, Eric’s patch takes this a few steps further and i will merge that into 
mine.


>> For class file versions embedded in Java code it would make it easier
>> if we had programmatic access. Perhaps we could have static methods on
>> Runtime.Version to get the major/minor class file versions?
> 
> Sure, or maybe better to make them instance methods of Runtime.

Yes, given Runtime.Version's focus on representing a version string for the 
platform.


>  Parsing
> `System.getProperty("java.class.version")` every time is no fun.
> 

Indeed. I’ll follow up on this yak shave with a separate issue.

Paul.

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 14 Dec 2017, at 03:58, Erik Joelsson  wrote:
> 
> And here it is: http://cr.openjdk.java.net/~erikj/8173382/webrev.01/
> 
> Feel free to incorporate this into whichever changeset you want, or I can 
> push this separately.
> 

Thanks! that is exactly what i was looking for. I will merge it into my patch.

Paul.

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 13 Dec 2017, at 14:36, mark.reinh...@oracle.com wrote:
> 
> 2017/12/13 14:09:44 -0800, paul.san...@oracle.com:
>>> On 13 Dec 2017, at 13:18, mark.reinh...@oracle.com wrote:
>>> How much of this can we parameterize and/or automate?
>> 
>> I suspect quite a bit, as you present below, and i agree we should try
>> and automate as much as possible. At the moment it’s very
>> spaghetti-like and quite fragile.
>> 
>> I suggest we get the initial version bump for 11 out of the way as
>> soon as JDK 10 branches off from the mainline then work on automation
>> for subsequent releases.
> 
> I understand that other incoming changes are waiting for the 11 version
> bump, but can't we at least do the straightforward parameterizations and
> introduce _CURRENT constants in this round?  We can leave anything that
> requires code generation to post-11.
> 

On the lang tools side that does seem easy, but I’ll defer to Joe.

With some guidance from the build engineers i would gladly consolidate class 
file version declaration in C/++ files with a passed in macro, if this can be 
expediently worked out. I wanna push condy as early as possible in 11 :-)
If we could do something in jvm.h that would be ideal. We already have the 
definitions JVM_CLASSFILE_MAJOR_VERSION and JVM_CLASSFILE_MINOR_VERSION pulled 
in via classfile_constants.h, so how can the build process inject values into 
those macros?

For class file versions embedded in Java code it would make it easier if we had 
programmatic access. Perhaps we could have static methods on Runtime.Version to 
get the major/minor class file versions?

Paul.

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 13 Dec 2017, at 13:18, mark.reinh...@oracle.com wrote:
> 
> 2017/12/13 8:44:33 -0800, paul.san...@oracle.com:
>>> On 12 Dec 2017, at 20:56, david.hol...@oracle.com wrote:
>>> 
>>> Anyway, none of the proposed changes have any impact on hotspot
>>> AFAICT. It is only when the actual version is updated to 11 that
>>> hotspot, and other entities will have to be updated. I'm still
>>> unclear if someone is actually driving the entire "update to version
>>> 11" process? Is there an umbrella issue for it?
>> 
>> No yet, i was hoping we could consolidate as much as possible under
>> one issue thereby minimising coordination/process i.e. combine the
>> release and class file versions under one patch. Then stand back and
>> see what is missing e.g. ctsym stuff etc.
> 
> How much of this can we parameterize and/or automate?

I suspect quite a bit, as you present below, and i agree we should try and 
automate as much as possible. At the moment it’s very spaghetti-like and quite 
fragile.

I suggest we get the initial version bump for 11 out of the way as soon as JDK 
10 branches off from the mainline then work on automation for subsequent 
releases.

Paul.

>  We are going to
> have to do it every six months, after all.
> 
> Could we define a CLASSFILE_VERSION in make/autoconf/version-numbers and
> then propagate that value everywhere else?  (In any actual release it'll
> be VERSION_FEATURE + 44, but make it a separate variable to allow for
> experimentation as with, e.g., MVT.)
> 
> Looking at Joe's webrev, many of the changes from 10 to 11 could instead
> be changed to Runtime.getRuntime().version().major() (in Java code) or
> VERSION_FEATURE (in Makefiles).
> 
> In javax.lang.model.SourceVersion we could define a new constant, say
> RELEASE_CURRENT, whose value is that of the latest release.  Then
> annotations such as
> 
>  @SupportedSourceVersion(SourceVersion.RELEASE_11)
> 
> could be replaced by
> 
>  @SupportedSourceVersion(SourceVersion.RELEASE_CURRENT)
> 
> Similarly, JDK_CURRENT could be defined in com.sun.tools.javac.Target
> and then many references to JDK10 could instead refer to that.
> 
> In cases where additional code is needed, can we generate it during
> the build via another Gensrc*.gmk Makefile?  That'd be more work now,
> but it could save us a lot of time in the long run.
> 
> - Mark

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

Joe,

Here's a patch on top of your patch:

  
http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-release-classfile-version/webrev/
 
<http://cr.openjdk.java.net/~psandoz/jdk/JDK-8173382-release-classfile-version/webrev/>

Currently building/testing with both patches applied.

Paul.


> On 13 Dec 2017, at 08:44, Paul Sandoz <paul.san...@oracle.com> wrote:
> 
> 
> 
>> On 12 Dec 2017, at 20:56, David Holmes <david.hol...@oracle.com> wrote:
>> 
>> Anyway, none of the proposed changes have any impact on hotspot AFAICT. It 
>> is only when the actual version is updated to 11 that hotspot, and other 
>> entities will have to be updated. I'm still unclear if someone is actually 
>> driving the entire "update to version 11" process? Is there an umbrella 
>> issue for it?
>> 
> 
> No yet, i was hoping we could consolidate as much as possible under one issue 
> thereby minimising coordination/process i.e. combine the release and class 
> file versions under one patch. Then stand back and see what is missing e.g. 
> ctsym stuff etc.
> 
> Paul.

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 12 Dec 2017, at 20:56, David Holmes  wrote:
> 
> Anyway, none of the proposed changes have any impact on hotspot AFAICT. It is 
> only when the actual version is updated to 11 that hotspot, and other 
> entities will have to be updated. I'm still unclear if someone is actually 
> driving the entire "update to version 11" process? Is there an umbrella issue 
> for it?
> 

No yet, i was hoping we could consolidate as much as possible under one issue 
thereby minimising coordination/process i.e. combine the release and class file 
versions under one patch. Then stand back and see what is missing e.g. ctsym 
stuff etc.

Paul.

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 12 Dec 2017, at 10:03, Paul Sandoz <paul.san...@oracle.com> wrote:
> 
> 
> 
>> On 12 Dec 2017, at 09:52, joe darcy <joe.da...@oracle.com> wrote:
>> 
>> Hi Paul,
>> 
>> There is sense in working in including the class file version update at the 
>> same time if the added complexity doesn't delay the changes.
>> 
> 
> I sense it should be so complex and delay since the additional code changes 
> can be copied from the prior changes which recently went through significant 
> review.

"I sense it should *not* be…”

Paul.

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

> On 12 Dec 2017, at 09:52, joe darcy <joe.da...@oracle.com> wrote:
> 
> Hi Paul,
> 
> There is sense in working in including the class file version update at the 
> same time if the added complexity doesn't delay the changes.
> 

I sense it should be so complex and delay since the additional code changes can 
be copied from the prior changes which recently went through significant review.

If it helps, and since i am proposing additional work, i can merge in such 
changes into your patch.


> Did the patch for the increment to 54 require previous changes in the bundled 
> ASM?
> 

The increment in and of itself requires only minimal changes to the internal 
ASM, it does not require the integration of a new version of ASM.  Constant 
dynamic only requires a minimal update to the internal ASM to skip over the new 
constant pool entry, since we don’t currently use ASM process or generate 
constant dynamic entries.

Paul.


> Thanks,
> 
> -Joe
> 
> 
> On 12/12/2017 9:31 AM, Paul Sandoz wrote:
>> Hi Joe,
>> 
>> I would like to see the class file version increment [*] in the same patch 
>> (the unified repo makes this easier now).
>> 
>> Then we have everything in one changeset (except for any ctsym changes?) 
>> that can be easily templated when the next increment is required, since we 
>> will be doing this far more regularly from now on.
>> 
>> Are there any technical impediments as to why this would be problematic?
>> 
>> Thanks,
>> Paul.
>> 
>> [*] here is the increment to 54 
>> http://hg.openjdk.java.net/jdk/jdk/rev/89829dd3cc54
>> 
> 

Re: Initial JDK 11 RFR of JDK-8173382: Add -source 11 and -target 11 to javac - Java Bug System & JDK-8193291: Add SourceVersion.RELEASE_11

[Paul Sandoz]

Hi Joe,

I would like to see the class file version increment [*] in the same patch (the 
unified repo makes this easier now).

Then we have everything in one changeset (except for any ctsym changes?) that 
can be easily templated when the next increment is required, since we will be 
doing this far more regularly from now on.

Are there any technical impediments as to why this would be problematic?

Thanks,
Paul.

[*] here is the increment to 54 
http://hg.openjdk.java.net/jdk/jdk/rev/89829dd3cc54 


> On 11 Dec 2017, at 23:04, joe darcy  wrote:
> 
> Hello,
> 
> With the JDK 11 line of development opening up in a few days, a few changes 
> are needed to prepared to turn a JDK 10 code base into a JDK 11 one including:
> 
>JDK-8173382: Add -source 11 and -target 11 to javac
>JDK-8193291: Add SourceVersion.RELEASE_11
> 
>Webev:
>http://cr.openjdk.java.net/~darcy/8173382.1/
>CSRs:
>JDK-8193350: Add -source 11 and -target 11 to javac
>JDK-8193351: Add SourceVersion.RELEASE_11
> 
> Please review the preliminary webrev and CSRs.
> 
> Note that breaking with previous contentions, the current iteration of the 
> change does *not* add -source/-target "1.11" as an alias for "11". Only "11" 
> is recognized.
> 
> I didn't see how to add support for an "11" value for "--release" so I 
> problem listed two tests until that is sorted out.
> 
> The build is simultaneously updated to user -source/-target 11, hence the 
> inclusion of build-dev.
> 
> Various langtools tests and test infrastructure is updated too.
> 
> Thanks,
> 
> -Joe
> 

Re: RFR: JDK-8192771: Boot JDK jar tool used to construct the modular JAR for java.jnlp

[Paul Sandoz]

+1

I tested this out with the class file version change patch and the build no 
longer fails.

Thanks,
Paul.

> On 4 Dec 2017, at 15:21, Erik Joelsson  wrote:
> 
> To be able to bump the classfile version in JDK 10, we need to be able to 
> build certain jar files using the newly built jar tool during the build. This 
> patch enables overriding of the jar cmd in the build.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8192771
> 
> Webrev: http://cr.openjdk.java.net/~erikj/8192771/webrev.01/
> 
> /Erik
> 

Re: RFR: 8160000: Runtime.version() cause startup regressions in 9+119

[Paul Sandoz]

> On 26 Jun 2016, at 21:55, Claes Redestad  wrote:
> 
> Hi,
> 
> 9+119 changed java.util.regex to initialize java.lang.invoke early, causing a 
> number of easily reproducible startup regressions.
> 
> This patch uses the fact that we already maintain the version string 
> constituents during build time to simplify creation of the 
> java.lang.Runtime.version().
> 
> Webrev: http://cr.openjdk.java.net/~redestad/816/webrev.3/
> Bug: https://bugs.openjdk.java.net/browse/JDK-816
> 
> Since getting Runtime.version() now does not have to touch java.util.regex 
> classes we end up slightly ahead of previous builds for applications which 
> does not use regular expressions.
> 
> Thanks!
> 

Looks good.

- Perhaps it’s worth pre-sizing the array list exactly by counting the ‘.’ 
before processing? or is 4 always pre-sized exactly?

- add an assert to check Version produced by version() is the same as that 
produced the previous way parsing the sys prop

-
 957 if (!VersionProps.VERSION_PRE.isEmpty()) {
 958 pre = Optional.of(VersionProps.VERSION_PRE);
 959 } else {
 960 pre = Optional.empty();
 961 }

Encapsulate that and the other two into a separate method e.g. optionalOfEmpty 
then do:

  version = new Version(…
 optionalOfEmpty(VersionProps.VERSION_PRE),
 …
 );

Paul.

Re: RFR (L) JEP 280: Indify String Concatenation (integration)

[Paul Sandoz]

I think it quite reasonable to push everything to jdk9/dev.

Paul.

> On 27 Jan 2016, at 14:55, Aleksey Shipilev  
> wrote:
> 
> Hi again,
> 
> This is a formal pre-integration review thread for JEP 280 ("Indify
> String Concatenation") integration:
>   http://openjdk.java.net/jeps/280
> 
> The JEP is Targeted, the CCC is approved, the code reviews and
> pre-integration checks are clean.
> 
> Code changes are happening simultaneously in four components:
> 
> a) (M) javac changes that emit indy:
>   http://cr.openjdk.java.net/~shade/8085796/webrev.langtools.05/
> 
> b) (L) JDK changes with StringConcatFactory and friends, plus fixing
> the regression tests that do not expect additional indys:
>   http://cr.openjdk.java.net/~shade/8085796/webrev.jdk.08/
> 
> c) (XS) Build changes that force emitting the "legacy" inline
> StringBuilder concat in a few cases (e.g. when pre-JDK 9 bytecode is
> expected):
>   http://cr.openjdk.java.net/~shade/8085796/webrev.root.00/
> 
> d) (XS) HotSpot changes that fix a GC regression test that now
> allocates some metaspace from within the test methods having a string
> concatenation:
>   http://cr.openjdk.java.net/~shade/8085796/webrev.hs.00/
> 
> These changes were already reviewed by multiple people, and so I would
> like to keep the comments only for serious breaking issues at this
> point. (Note that this thread cross-posts over several mailing lists:
> bike-shedding discussion would get multiplied a lot!)
> 
> Formal acknowledgements from Reviewers would be appreciated. Pending no
> show-stopper comments, I'd like to push this through jdk9/dev in 24 hours.
> 
> Thanks,
> -Aleksey
>