RFR: 7903188: Log time spent waiting to acquire exclusive access lock [v2]
Jonathan Gibbons
jjg at openjdk.org
Tue Jul 2 23:55:29 UTC 2024
On Thu, 20 Jun 2024 10:45:51 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Can I please get a review of this change which proposes to implement the enhancement requested in https://bugs.openjdk.org/browse/CODETOOLS-7903188?
>>
>> jtreg supports the ability to sequentially execute tests, instead of concurrently, for tests belonging to a pre-configured `exclusiveAccess.dirs` directory. The `MainAction` and `ShellAction` before launching the test, first acquire a lock. The lock acquisition can be time consuming and depends on how long an already running test from that directory takes to complete. This lock acquisiton time isn't reported anywhere in the jtreg action's `section` in the report. Because of this, it sometimes makes it difficult to determine where the unaccounted time is spent.
>>
>> The change in this PR prints out how long it took to acquire a exclusive access before launching the test. It's only printed if the test was configured with exclusiveAccess. The reported message will look like:
>>
>>
>> #section:main
>> ----------messages:(7/230)----------
>> command: main Test
>> reason: User specified action: run main Test
>> started: Wed Jun 19 14:45:21 IST 2024
>> exclusiveAccess wait time (seconds): 20.289
>> Mode: othervm
>> finished: Wed Jun 19 14:45:21 IST 2024
>> elapsed time (seconds): 0.123
>>
>>
>> An existing self test has been updated to verify this change. All existing tests continue to pass with this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>
> A more cleaner fix - let the Action class dealing with acquiring and releasing locks and printing the wait duration
Some bad javadoc comments; not a show stopper, but worth addressing to reduce e future confusion.
src/share/classes/com/sun/javatest/regtest/config/RegressionTestSuite.java line 220:
> 218: /**
> 219: * @param td the test description
> 220: * {@return true if the test is configured to run exclusively, false otherwise}
This is in the wrong place.
As written, the inline tag `{@return...}` is part of the description of the preceding block tag `@param`
When using the inline form of the `return` tag, it should appear at the beginning of the description.
src/share/classes/com/sun/javatest/regtest/config/RegressionTestSuite.java line 232:
> 230: /**
> 231: * @param td the test description
> 232: * {@return true if the test is configured to run exclusively, false otherwise}
same bad `{@return}` tag as above.
-------------
Changes requested by jjg (Lead).
PR Review: https://git.openjdk.org/jtreg/pull/208#pullrequestreview-2155078872
PR Review Comment: https://git.openjdk.org/jtreg/pull/208#discussion_r1663297439
PR Review Comment: https://git.openjdk.org/jtreg/pull/208#discussion_r1663303476
More information about the jtreg-dev
mailing list