Re: RFR: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java [v3]

2024-05-14 Thread Jonathan Gibbons
On Fri, 10 May 2024 13:42:43 GMT, Hannes Wallnöfer  wrote:

>> Please review a change to move a long-running shellsupport test from 
>> langtools tier1 to tier2, while modifying the tier1 test to only retrieve 
>> doc comments for members of `java.lang.StringBuilder`, which is where [the 
>> original bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This 
>> reduces the running time in the tier1 test from around 90 seconds to 2 
>> seconds on my computer. The tier1 test also adds a non-null check for 
>> retrieved doc comments.
>> 
>> I used the existing `langtools_jshell_unstable` test group to add the new 
>> test in langtools tier2, which is not a perfect match for the test, but 
>> close enough and helps to keep langtools `TEST.groups` file simple.
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Change quick test to run on a random subset of classes

Marked as reviewed by jjg (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18837#pullrequestreview-2056576654


Re: RFR: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java [v3]

2024-05-10 Thread Hannes Wallnöfer
On Fri, 10 May 2024 13:42:43 GMT, Hannes Wallnöfer  wrote:

>> Please review a change to move a long-running shellsupport test from 
>> langtools tier1 to tier2, while modifying the tier1 test to only retrieve 
>> doc comments for members of `java.lang.StringBuilder`, which is where [the 
>> original bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This 
>> reduces the running time in the tier1 test from around 90 seconds to 2 
>> seconds on my computer. The tier1 test also adds a non-null check for 
>> retrieved doc comments.
>> 
>> I used the existing `langtools_jshell_unstable` test group to add the new 
>> test in langtools tier2, which is not a perfect match for the test, but 
>> close enough and helps to keep langtools `TEST.groups` file simple.
>
> Hannes Wallnöfer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Change quick test to run on a random subset of classes

I have updated the PR to use a random subset of classes in the quick tier 1 
test. It now retrieves doc comments on 2% of classes, which corresponds to 
roughly 140 classes and takes around 7 seconds on my computer.

-

PR Comment: https://git.openjdk.org/jdk/pull/18837#issuecomment-2104639693


Re: RFR: 8294880: Review running time of jdk/internal/shellsupport/doc/JavadocHelperTest.java [v3]

2024-05-10 Thread Hannes Wallnöfer
> Please review a change to move a long-running shellsupport test from 
> langtools tier1 to tier2, while modifying the tier1 test to only retrieve doc 
> comments for members of `java.lang.StringBuilder`, which is where [the 
> original bug](https://bugs.openjdk.org/browse/JDK-8189778) occurred.  This 
> reduces the running time in the tier1 test from around 90 seconds to 2 
> seconds on my computer. The tier1 test also adds a non-null check for 
> retrieved doc comments.
> 
> I used the existing `langtools_jshell_unstable` test group to add the new 
> test in langtools tier2, which is not a perfect match for the test, but close 
> enough and helps to keep langtools `TEST.groups` file simple.

Hannes Wallnöfer has updated the pull request incrementally with one additional 
commit since the last revision:

  Change quick test to run on a random subset of classes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18837/files
  - new: https://git.openjdk.org/jdk/pull/18837/files/7a45c94d..d400cf1b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18837&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18837&range=01-02

  Stats: 132 lines in 2 files changed: 48 ins; 72 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/18837.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18837/head:pull/18837

PR: https://git.openjdk.org/jdk/pull/18837