On Wed, 21 Jan 2026 15:24:15 GMT, Kevin Walls <[email protected]> wrote:
>> Ivan Bereziuk 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 15 additional >> commits since the last revision: >> >> - remove TimeStamp::No as it's not used. virtual should be flipped to >> override in bulk (afressed clang warning) >> - Merge branch 'master' into 8357828_add_timestamp_to_jcmd >> - changes to jcmd.md >> - undo changes to reorder_help_cmd() >> - cleanup >> - add timestamp to VM.version. Add test >> - updated jcmd usage text >> - undo the changes to the modified earlier tests as timestamp presence is >> fully backwards compatible >> - introduce -T as a commong flag >> - Merge branch 'master' into 8357828_add_timestamp_to_jcmd >> - ... and 5 more: https://git.openjdk.org/jdk/compare/e81dffc4...5f1cefe0 > > OK thanks, yes not changing all the dcmds in > https://github.com/openjdk/jdk/pull/29325/files is nice. 8-) > > > Regarding whether JCmd.java could do the timestamp, or we need to change the > native side: > Yes a problem there is the timestamp duplication issue: if JCmd can > optionally print a timestamp as a header before running the command, that > timestamp info is duplicated in Thread.print. > > But is it really that bad, and is it worth the extra argument processing? > (In diagnosticFramework, we need to introduce parse_common options just for > this one flag.) > > If JCmd.java does the timestamp: if you opt-in to JCmd with a timestamp, you > would get the new timestamp in the new format, followed by the old > Thread.print timestamp in its format... > This avoids the new complexity in the native code, and keeps the new > timestamp handling in the simple JCmd.java wrapper. > It sidesteps the problem that Thread.print uses an arguably "wrong" format. > (An in time, maybe Thread.print would migrate to not printing a timestamp.) > > > (JCmd could optionally print a duration at the end as was hinted somewhere > earlier. > Heap dumping prints its own time taken, but few other commands consider it > interesting.) @kevinjwalls > not changing all the dcmds in https://github.com/openjdk/jdk/pull/29325/files > is nice. 8-) Yes. That is indeed an option to consider. In here * I've removed "print timestamp" handling from `DCmd::execute()`. Now it's handled in `DCmd::Executor::parse_and_execute()`. The additional parameter passed to `DCmd::execute` serves the only purpose - let `Thread.print` dcmd be backwards compatible and print a "timestamp" of old format when "-T" flag was passed. * I have also embraced an ISO 8601 format. Example `2026-01-21T16:58:49.518+0100`; Alan, David, thank you. It might be fine to keep framework of "common jcmd flags" in place. Executors would need to know about "-format=json" if that becomes a common flag in the future. Other than that, it might be fine to let "Thread.print" print two timestamps which will make the implementation simpler: https://github.com/openjdk/jdk/pull/29325 Q: what would be best name for the flag. Currently it's "-T". Should we use a different name? ------------- PR Comment: https://git.openjdk.org/jdk/pull/27368#issuecomment-3779616328
