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