On Tue, 20 Jan 2026 14:51:27 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/438fc675...5f1cefe0 > > Lots of duplication in the execute methods, of the same common condition > around calling print_local_time... > That's a reason in favour of the jcmd driver printing the timestamp itself so > it can be done in one place, if required. > (The local attach API is not slow enough to cause a problematic latency in > when the timestamp was printed, and when the command was actually run. > Certainly not compared to the current workaround of running: "date; jcmd ...") > > > To deal with commands that already show a timestamp, if JCmd.java is where > the timestamp is printed: if jcmd can send a parameter over the new attach > api to the diagnostic command, then it can send a boolean "i have already > printed a timestamp". Then only an existing command which already prints a > timestamp would need to check. > > > Or separately in David's note earlier: > "And rather than add timestamp printing code to each dcmd it would make more > sense to me to have a global dcmd flag that says "print a timestamp" (with a > given format). That way users opt-in and we don't have to remember to add > this for new Dcmds." > This relates to if the printing is done in the native code, and as I read it > this was about having a flag checked before the individual execute methods, > so it can be done in one place (not sure but maybe this would be > DCmd::Executor::parse_and_execute()). @kevinjwalls I have created a v2 MR that handles timestamp in `DCmd::Executor::parse_and_execute` and doesn't change the executors. https://github.com/openjdk/jdk/pull/29325 The drawback is that timestamp can be printed twice in case of `Thread.print` diagnostic command. ------------- PR Comment: https://git.openjdk.org/jdk/pull/27368#issuecomment-3773628920
