RFR: 8346609: Improve MemorySegment.toString [v2]
Maurizio Cimadamore
mcimadamore at openjdk.org
Tue Jan 7 11:50:47 UTC 2025
On Tue, 7 Jan 2025 11:30:13 GMT, Per Minborg <pminborg at openjdk.org> wrote:
>> This PR proposes to improve the current `MemorySegment.toString()` method from:
>>
>> `MemorySegment{ address: 0x60000264c540, byteSize: 8 }`
>>
>> to
>>
>> `MemorySegment{ native, address: 0x60000264c540, byteSize: 8}`
>>
>> Tests passes tier1-tier3
>
> Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>
> - Reduce elements in toString
> - Merge branch 'master' into ms-tostring-improvement2
> - Implement an alternate MS::toString
> - Improve MemorySegment::toString
src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java line 490:
> 488: public String toString() {
> 489: return "MemorySegment{ " +
> 490: heapBase().map(hb -> "heapBase: " + hb).orElse(isMapped() ? "mapped" : "native") +
This is not correct. Heap base can be empty for read-only segments.
Have we considered printing one of:
* HeapMemorySegment
* NativeMemorySegment
* MappedMemorySegment
E.g. add the heap/native/mapped as part of the "class" qualifier, before the `{` ? That seems more readable (of course the drawback is that it might suggest that these classes exist somewhere, which they don't.
A possible compromise would be:
MemorySegment{ kind: heap/native/mapped, [heapBase: xyz ], address: xyz, size: xyz}
This is consistent with the terminology found in the javadoc:
> There are two kinds of memory segments: ...
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22826#discussion_r1905337003
More information about the core-libs-dev
mailing list