Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Fri, 13 May 2022 09:42:44 GMT, Maurizio Cimadamore wrote: >> This change here also closes >> [JDK-8259034](https://bugs.openjdk.java.net/browse/JDK-8259034) > > @uschindler - the issue you mention with respect lack of UOE for wrong file > system applies to BB as well. I suggest filing an issue. See https://bugs.openjdk.java.net/browse/JDK-8286735 - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Fri, 13 May 2022 11:59:12 GMT, Maurizio Cimadamore wrote: >> RFE = issue? > >> RFE = issue? > > issue, with type RFE (request for enhancement) See: https://bugs.openjdk.java.net/browse/JDK-8286734 - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Fri, 13 May 2022 11:01:09 GMT, Uwe Schindler wrote: > RFE = issue? issue, with type RFE (request for enhancement) - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Fri, 13 May 2022 09:43:55 GMT, Maurizio Cimadamore
wrote:
>> src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 1164:
>>
>>> 1162: }
>>> 1163: if (unmapper != null) {
>>> 1164: AbstractMemorySegmentImpl segment = new
>>> MappedMemorySegmentImpl(unmapper.address(), unmapper, size,
>>
>> When reviewing the method for MappedByteBuffer: I think to make this
>> consistent the "old" method should also use `address()` which applies the
>> pagePosition. Currently it is confusing:
>> - New code returning `MemorySegment` uses `unmapper.address()`
>> - Old code returning `MappedByteBuffer` uses `unmapper.address +
>> unmapper.pagePosition` (fields)
>>
>> Should I open an issue or a PR to fix this (because this is already merged)?
>>
>> See the mailing list posts:
>> - https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016981.html
>> - https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016990.html
>
> Please file an RFE. I suspect that there will be more little improvements and
> consolidation like this we'll want to make to this code.
RFE = issue?
-
PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Fri, 13 May 2022 08:33:11 GMT, Uwe Schindler wrote: >> src/java.base/share/classes/java/nio/channels/FileChannel.java line 1045: >> >>> 1043: * >>> 1044: * @throws UnsupportedOperationException >>> 1045: * If an unsupported map mode is specified. >> >> I think this should mention that UOE is also throws if the filesystem >> implementation does not support memory mapping. This may happen for >> filesystems like jrtfs or custom wrapper filesystems that do not implement >> the method below. >> >> As this is already merged, should I open a PR fixing the docs or open an >> issue? > > This change here also closes > [JDK-8259034](https://bugs.openjdk.java.net/browse/JDK-8259034) @uschindler - the issue you mention with respect lack of UOE for wrong file system applies to BB as well. I suggest filing an issue. - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Fri, 13 May 2022 08:29:13 GMT, Uwe Schindler wrote:
>> Maurizio Cimadamore has updated the pull request with a new target base due
>> to a merge or a rebase. The pull request now contains 65 commits:
>>
>> - Merge branch 'master' into foreign-preview
>> - Merge branch 'master' into foreign-preview
>> - Fix crashes in heap segment benchmarks due to misaligned access
>> - Merge branch 'master' into foreign-preview
>> - Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java
>>
>>Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
>> - Add tests for loaderLookup/restricted method corner cases
>> - * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI
>>* Tweak restricted method check to work when there's no caller class
>> - Tweak examples in SymbolLookup javadoc
>> - 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>
>> - ... and 55 more:
>> https://git.openjdk.java.net/jdk/compare/82aa0455...f170415b
>
> src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 1164:
>
>> 1162: }
>> 1163: if (unmapper != null) {
>> 1164: AbstractMemorySegmentImpl segment = new
>> MappedMemorySegmentImpl(unmapper.address(), unmapper, size,
>
> When reviewing the method for MappedByteBuffer: I think to make this
> consistent the "old" method should also use `address()` which applies the
> pagePosition. Currently it is confusing:
> - New code returning `MemorySegment` uses `unmapper.address()`
> - Old code returning `MappedByteBuffer` uses `unmapper.address +
> unmapper.pagePosition` (fields)
>
> Should I open an issue or a PR to fix this (because this is already merged)?
>
> See the mailing list posts:
> - https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016981.html
> - https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016990.html
Please file an RFE. I suspect that there will be more little improvements and
consolidation like this we'll want to make to this code.
-
PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Thu, 12 May 2022 15:45:01 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 65 commits:
>
> - Merge branch 'master' into foreign-preview
> - Merge branch 'master' into foreign-preview
> - Fix crashes in heap segment benchmarks due to misaligned access
> - Merge branch 'master' into foreign-preview
> - Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java
>
>Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
> - Add tests for loaderLookup/restricted method corner cases
> - * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI
>* Tweak restricted method check to work when there's no caller class
> - Tweak examples in SymbolLookup javadoc
> - 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>
> - ... and 55 more:
> https://git.openjdk.java.net/jdk/compare/82aa0455...f170415b
Some late comments and suggestions to fix inconsistencies and wrong javadocs.
src/java.base/share/classes/java/nio/channels/FileChannel.java line 1045:
> 1043: *
> 1044: * @throws UnsupportedOperationException
> 1045: * If an unsupported map mode is specified.
I think this should mention that UOE is also throws if the filesystem
implementation does not support memory mapping. This may happen for filesystems
like jrtfs or custom wrapper filesystems that do not implement the method below.
As this is already merged, should I open a PR fixing the docs or open an issue?
src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 1164:
> 1162: }
> 1163: if (unmapper != null) {
> 1164: AbstractMemorySegmentImpl segment = new
> MappedMemorySegmentImpl(unmapper.address(), unmapper, size,
When reviewing the method for MappedByteBuffer: I think to make this consistent
the "old" method should also use `address()` which applies the pagePosition.
Currently it is confusing:
- New code returning `MemorySegment` uses `unmapper.address()`
- Old code returning `MappedByteBuffer` uses `unmapper.address +
unmapper.pagePosition` (fields)
Should I open an issue or a PR to fix this (because this is already merged)?
See the mailing list posts:
- https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016981.html
- https://mail.openjdk.java.net/pipermail/panama-dev/2022-May/016990.html
-
PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
On Fri, 13 May 2022 08:25:01 GMT, Uwe Schindler wrote: >> Maurizio Cimadamore has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 65 commits: >> >> - Merge branch 'master' into foreign-preview >> - Merge branch 'master' into foreign-preview >> - Fix crashes in heap segment benchmarks due to misaligned access >> - Merge branch 'master' into foreign-preview >> - Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java >> >>Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> >> - Add tests for loaderLookup/restricted method corner cases >> - * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI >>* Tweak restricted method check to work when there's no caller class >> - Tweak examples in SymbolLookup javadoc >> - 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> >> - ... and 55 more: >> https://git.openjdk.java.net/jdk/compare/82aa0455...f170415b > > src/java.base/share/classes/java/nio/channels/FileChannel.java line 1045: > >> 1043: * >> 1044: * @throws UnsupportedOperationException >> 1045: * If an unsupported map mode is specified. > > I think this should mention that UOE is also throws if the filesystem > implementation does not support memory mapping. This may happen for > filesystems like jrtfs or custom wrapper filesystems that do not implement > the method below. > > As this is already merged, should I open a PR fixing the docs or open an > issue? This change here also closes [JDK-8259034](https://bugs.openjdk.java.net/browse/JDK-8259034) - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v45]
> 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 65 commits: - Merge branch 'master' into foreign-preview - Merge branch 'master' into foreign-preview - Fix crashes in heap segment benchmarks due to misaligned access - Merge branch 'master' into foreign-preview - Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Add tests for loaderLookup/restricted method corner cases - * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI * Tweak restricted method check to work when there's no caller class - Tweak examples in SymbolLookup javadoc - 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> - ... and 55 more: https://git.openjdk.java.net/jdk/compare/82aa0455...f170415b - Changes: https://git.openjdk.java.net/jdk/pull/7888/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7888=44 Stats: 65711 lines in 373 files changed: 43720 ins; 19432 del; 2559 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
