[foreign-memaccess+abi] RFR: 8291639: Improve the ability to visualize a MemorySegment in human readable forms [v2]
Rémi Forax
forax at openjdk.org
Fri May 26 13:11:27 UTC 2023
On Fri, 26 May 2023 13:06:57 GMT, Per Minborg <pminborg at openjdk.org> wrote:
>> This PR proposes to add a "Record Mapper" that allows `MemorySegment` instances to be projected onto `Record` types given `GroupLayout` instances.
>>
>> Here is a simple example of how to use the mapper:
>>
>>
>> private static final GroupLayout POINT_LAYOUT = MemoryLayout.structLayout(
>> JAVA_INT.withName("x"),
>> JAVA_INT.withName("y"));
>>
>> MemorySegment segment = MemorySegment.ofArray(new int[]{3, 4});
>>
>> Point point = POINT_LAYOUT.recordMapper(Point.class)
>> .apply(segment); // Point[x=3, y=4]
>>
>>
>> I think the implementation can be improved later, for example by de-duplicating handling of arrays and maybe add recursive "un-pealing" of multidimensional arrays.
>
> Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 29 commits:
>
> - Merge master
> - Fix typos
> - Clean up toString methods
> - Add support for MemoryAddress arrays and improve docs
> - Improve javadoc
> - Change to IAE if mapper fails
> - Remove caching
> - Add tests for initial issue examples
> - Clean up and optimize
> - Add support for multidimensional records
> - ... and 19 more: https://git.openjdk.org/panama-foreign/compare/bee4503b...1d7db589
src/java.base/share/classes/jdk/internal/foreign/LayoutRecordMapper.java line 108:
> 106: throw new IllegalArgumentException("There is no mapping for " +
> 107: missingComponents + " in " + type.getName() +
> 108: "(" + String.join(", ", recordComponents.stream().map(RecordComponent::getName).collect(Collectors.joining(", "))) + ")" +
Two joins, String.join and Collectors.joining() ??
src/java.base/share/classes/jdk/internal/foreign/LayoutRecordMapper.java line 123:
> 121: // where X is the component type.
> 122: MethodHandle[] handles = componentAndLayoutList.stream()
> 123: .map(cl -> {
map() + filter(Object::isNull) should be a flatMap() or a mapMulti()
src/java.base/share/classes/jdk/internal/foreign/LayoutRecordMapper.java line 128:
> 126: long byteOffset = layout.byteOffset(pathElement) + offset;
> 127:
> 128: return switch (cl.layout()) {
this switch is too big, there is even a switch in that switch, which makes the code hard to read
-------------
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/833#discussion_r1206753921
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/833#discussion_r1206758374
PR Review Comment: https://git.openjdk.org/panama-foreign/pull/833#discussion_r1206756692
More information about the panama-dev
mailing list