On Thu, 30 Apr 2026 09:22:52 GMT, Casper Norrbin <[email protected]> wrote:
>> Hi everyone, `PrintClassLayout::print_class_layout` expects internal class >> names as its argument, but is called externally by users through `jcmd`. >> This leads to inconsistencies with other functions. For example: >> >> jcmd <pid> VM.class_hierarchy java.lang.String >> jcmd <pid> VM.class_print_layout java/lang/String >> >> >> In addition, its output is also in the internal format. This should be >> changed to also use the external dotted class name instead. ~~For this fix I >> changed the `Closure` used for finding classes to instead use external >> names, and also swapped the output print to use the external name.~~ I added >> a conversion from external dotted names to internal slashed names before we >> start searching for classes. That way we avoid comparing strings and avoid >> allocations from calls to `external_name()`. As a side effect, this now >> works on both internal and external names, as slashes are left untouched. >> >> Testing: >> - Tier 1 >> - Manual inspection with `jcmd` >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Casper Norrbin has updated the pull request incrementally with one additional > commit since the last revision: > > feedback fixes I'd missed the fact the user-supplied class name was also expected in internal form and needed to be changed to use Java form instead. I have some thoughts on that below but perhaps that is better handled in a separate RFE. src/hotspot/share/memory/heapInspection.cpp line 605: > 603: ResourceMark rm; > 604: char* normalized_name = NEW_RESOURCE_ARRAY(char, strlen(class_name) + > 1); > 605: strcpy(normalized_name, class_name); Do we have to make a copy here rather than modifying in place? The user-supplied string is already in a mutable C-heap allocated buffer, as processed by the DCmd framework. If we think it inappropriate to modify at this level perhaps we can push the conversion up into the DCmd framework itself? src/hotspot/share/memory/heapInspection.cpp line 608: > 606: for (char* p = normalized_name; *p != '\0'; p++) { > 607: if (*p == '.') { > 608: *p = '/'; This should use `JVM_SIGNATURE_DOT` and `JVM_SIGNATURE_SLASH` - though it makes me cringe that we have to do this ad-hoc conversion. ------------- PR Review: https://git.openjdk.org/valhalla/pull/2375#pullrequestreview-4210040001 PR Review Comment: https://git.openjdk.org/valhalla/pull/2375#discussion_r3172180640 PR Review Comment: https://git.openjdk.org/valhalla/pull/2375#discussion_r3172162108
