On Tue, 11 Jun 2024 07:54:31 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> Inigo Mediavilla Saiz 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:
>> 
>>  - Merge remote-tracking branch 'upstream/master' into 
>> txominpelu_8330846_add_stack_vthreads
>>  - Include virtual thread name in output
>>  - Incorporate @tstuefe's remarks
>>  - Remove dead code
>>  - Remove extra indentation (leave it for the next PR)
>>  - Cleanup test
>>    
>>    - Stop virtualthread
>>    - Remove unneeded imports
>>    - Remove modules that are not needed
>>  - Fix copyright year
>>  - Print mounted virtual thread after carrier
>>  - Add indentation for virtual thread stack
>>  - Update 
>> test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java
>>    
>>    Co-authored-by: Andrey Turbanov <turban...@gmail.com>
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/7953d9d6...ba3385a4
>
> test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java 
> line 52:
> 
>> 50:         output.shouldMatch(".*at " + 
>> Pattern.quote(DummyRunnable.class.getName()) + "\\.run.*");
>> 51:         output.shouldMatch(".*at " + 
>> Pattern.quote(DummyRunnable.class.getName()) + "\\.compute.*");
>> 52:         shouldFinish.set(true);
> 
> This just checks that the stack frames are printed. I think you also want to 
> check that the output contains and line with "Mounted virtual thread and the 
> thread ID.

Good point ! I've done that in: 9591b28a8cef36b9507ab67365ffcb9400d06dd9

> test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java 
> line 78:
> 
>> 76:             while (true) {
>> 77:                 if (shouldFinish.get()) {
>> 78:                     break;
> 
> You can use Thread.onSpinWait in the loop to reduce the impact of the tight 
> loop.
> 
> Also you can use `while (!shouldFinish.get())` to avoid the explicit break.

Indeed, that will make it both clearer and more efficient: 
9591b28a8cef36b9507ab67365ffcb9400d06dd9

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1634476963
PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1634477629

Reply via email to