Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v43]
On Mon, 9 May 2022 17:41:10 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 incrementally with one > additional commit since the last revision: > > Fix crashes in heap segment benchmarks due to misaligned access test/micro/org/openjdk/bench/java/lang/foreign/LoopOverPollutedSegments.java line 69: > 67: static final ValueLayout.OfInt JAVA_INT_UNALIGNED = > JAVA_INT.withBitAlignment(8); > 68: static final ValueLayout.OfFloat JAVA_FLOAT_UNALIGNED = > JAVA_FLOAT.withBitAlignment(8); > 69: static final VarHandle intHandleUnaligned = > JAVA_INT_UNALIGNED.arrayElementVarHandle(); To match `LoopOverNonConstantHeap`, this should probably be named `VH_INT_UNALIGNED`. Maybe these could also be moved into `org.openjdk.bench.java.lang.foreign.Utils`[^1] [^1]: https://github.com/openjdk/jdk/blob/7b774297b1d04e104a988ea5bd2f2b04c8de3461/test/micro/org/openjdk/bench/java/lang/foreign/Utils.java#L29-L43 - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v40]
On Fri, 6 May 2022 11:51:46 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 incrementally with one > additional commit since the last revision: > > Add tests for loaderLookup/restricted method corner cases src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 116: > 114: // if there is no caller class, act as if the call came from > unnamed module of system class loader > 115: Module module = currentClass != null ? > 116: currentClass.getModule() : > ClassLoader.getSystemClassLoader().getUnnamedModule(); **Nit:** maybe add a line break Suggestion: currentClass.getModule() : ClassLoader.getSystemClassLoader().getUnnamedModule(); - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v37]
On Tue, 3 May 2022 10:40:02 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 57 commits:
>
> - Merge branch 'master' into foreign-preview
> - Update
> src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template
>
>Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
> - Tweak support for Linker lookup
>Improve javadoc of SymbolLookup
> - Tweak Addressable javadoc
> - Downcall and upcall tests use wrong layout which is missing some trailing
> padding
> - Simplify libraryLookup impl
> - Address CSR comments
> - Merge branch 'master' into foreign-preview
> - Add ValueLayout changes
> - Tweak support for array element var handle
> - ... and 47 more:
> https://git.openjdk.java.net/jdk/compare/af1ee1cc...41d055ab
src/java.base/share/classes/java/lang/foreign/SymbolLookup.java line 116:
> 114: * Linker nativeLinker = Linker.nativeLinker();
> 115: * SymbolLookup stdlib = nativeLinker.defaultLookup();
> 116: * MemorySegment malloc = stdlib.lookup("malloc");
This should be:
Suggestion:
* Optional malloc = stdlib.lookup("malloc");
or
Suggestion:
* MemorySegment malloc = stdlib.lookup("malloc").orElseThrow();
src/java.base/share/classes/java/lang/foreign/SymbolLookup.java line 153:
> 151: static SymbolLookup loaderLookup() {
> 152: Class caller = Reflection.getCallerClass();
> 153: ClassLoader loader =
> Objects.requireNonNull(caller.getClassLoader());
Shouldn’t this be changed to throw `IllegalCallerException` instead of throwing
`NullPointerException` when the `caller`’s `ClassLoader` is `null`[^1] or when
`caller` itself is `null`[^2]?
[^1]: This happens when `caller` is on the bootstrap classpath.
[^2]: This happens when `SymbolLookup.loaderLookup()` is called by native code
and no **Java** code is on the call stack.
-
PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v35]
On Fri, 29 Apr 2022 08:15:32 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 incrementally with one
> additional commit since the last revision:
>
> Tweak Addressable javadoc
src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template
line 101:
> 99: }
> 100:
> 101: public final static class ScopedAccessError extends Error {
This should probably use the canonical modifier order as specified in
[JDK‑8276348] ([GH‑6213]):
Suggestion:
public static final class ScopedAccessError extends Error {
[JDK‑8276348]: https://bugs.openjdk.java.net/browse/JDK-8276348 "[JDK‑8276348]
Use blessed modifier order in java.base"
[GH‑6213]: https://github.com/openjdk/jdk/pull/6213
-
PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v15]
On Wed, 30 Mar 2022 20:59:34 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 incrementally with one
> additional commit since the last revision:
>
> Tweak FunctionDescriptor::argumentLayouts to return an immutable list
src/java.base/share/classes/java/lang/foreign/FunctionDescriptor.java line 73:
> 71: */
> 72: public List argumentLayouts() {
> 73: return Collections.unmodifiableList(argLayouts);
This change doesn’t seem to be necessary, as `FunctionDescriptor` is already
created with `List.of(…)` (or `Stream.toList()` in the case of
`FunctionDescriptor.VariadicFunction`), which produce immutable lists (although
`Stream.toList()` permits `null`s, which
`Stream.collect(Collectors.toImmutableList())` and `List.of(…)` doesn’t).
-
PR: https://git.openjdk.java.net/jdk/pull/7888
