RFR: 8287008: Improve tests for thread dumps in JSON format [v3]

[Alan Bateman]

On Mon, 23 May 2022 06:09:58 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Alan Bateman has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove spurious colon
>
> test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java line 115:
> 
>> 113: 
>> 114:                 // find the thread container that corresponds to the executor
>> 115:                 String name = Objects.toIdentityString(executor);
> 
> I don't understand the need for `toIdentityString()` rather than just `toString()`

The implementation lends on the string returned by Object::toString when not overridden. In this case, either will do so I can change it to toString.

> test/lib/jdk/test/lib/threaddump/ThreadDump.java line 179:
> 
>> 177:                 return this;
>> 178:             if (name().startsWith(name + "/"))
>> 179:                 return this;
> 
> It's not clear to me why this is here. What does it mean if the name of the container starts with `<name>/`?

With the exception of the root container, the names have the form "name1/name2" where name1 is the "external name" and name2 identifies the internal thread grouping.  The external name is the object identity for thread pools and thread-per-executors created by user code. In time we may expose ways to provide a name.

> test/lib/jdk/test/lib/threaddump/ThreadDump.java line 299:
> 
>> 297: 
>> 298:             // add to map if not already encountered
>> 299:             var container = map.computeIfAbsent(name, k -> new ThreadContainer(name));
> 
> Why might the container already be in `map`?

They aren't topologically sorted so it's possible to encounter a thread container before its parent.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8784