RFR: 7902847: Class directory of a test case should be always used to compile a library [v9]

[Roger Riggs]

On Tue, 15 Apr 2025 16:06:08 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> The classes from test libraries can be compiled implicitly as a test dependency or explicitly with @build tag.
>> 
>> For first case
>> Test library used as a source path during test compilation  and library classes as well as test classes are compiled into class directory. 
>> 
>> For 2nd case 
>> The library classes are compiled using @build tag and library classes are placed into some shared location.
>> 
>> These 2 cases might be mixed in the single test and can have part of library classes compiled into shared directory and part compiled into test classes directory.
>> 
>> jtreg uses classfiles to check if source code should be compiled. Let we have 2 classes LibA and Lib that extends LibA.
>>  So if class LibA might be  compiled into test class directory while LibB is compiled into library directory. Later  jtreg can try to use LibB class without and compilation because LibB already exists. Thus the CNFE will be thrown.
>> 
>> The another possible problem, is that the library code might include test code for libraries like "/" and "/vmTestbase"
>> so any test classes with same packages would be compiled into the same location because they are treated as library classes. So for tree like this:
>> test1/Test.java
>> test2/Test.java
>> with both tests having
>> 
>> @library "/"
>> @build Test
>> 
>> we are going just to have one Test class in library directory.
>> 
>> The only reliable fix would be don't use shared class directory at all and compile all libraries for each test. Although it looks like huge performance overhead, the impact is low. The libraries are not often compiled using build tag. 
>> 
>> Times for tier1 execution with fix:
>> test-results/jtreg_test_lib_test_tier1/text/timeStats.txt:Total elapsed time 0m 28s
>> test-results/jtreg_test_hotspot_jtreg_tier1/text/timeStats.txt:Total elapsed time 18m 14s
>> test-results/jtreg_test_jdk_tier1/text/timeStats.txt:Total elapsed time 15m 8s
>> test-results/jtreg_test_langtools_tier1/text/timeStats.txt:Total elapsed time 7m 59s
>> 
>> and before fix
>> test-results/jtreg_test_lib_test_tier1/text/timeStats.txt:Total elapsed time 0m 32s
>> test-results/jtreg_test_hotspot_jtreg_tier1/text/timeStats.txt:Total elapsed time 17m 51s
>> test-results/jtreg_test_jdk_tier1/text/timeStats.txt:Total elapsed time 14m 49s
>> test-results/jtreg_test_langtools_tier1/text/timeStats.txt:Total elapsed time 7m 56s
>> 
>> The full fix might require more testing and adding testcase.
>> Please note that there are plans to work on the 
>> https://bugs.openjd...
>
> Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed typo

Changes requested by rriggs (no project role).

src/share/doc/javatest/regtest/tag-spec.html line 256:

> 254: 
> 255: <p>In general, classes in library directories are not automatically compiled
> 256: as part of a compilation command explicitly naming the source files containing

Where is the "shareLibraries" property defined? I'm expecting to see it defined in the TEST.ROOT or TEST.properties table like "enablePreview", etc.

Also, I think it should not be plural. It applies to this particular library.

Maybe I misunderstand.

src/share/doc/javatest/regtest/tag-spec.html line 261:

> 259: A test that relies upon library classes might contain appropriate
> 260: <code>@build</code> directives to explictily point classes to be compiled.
> 261: This might be useful when test depends on library classes during runtime only.

Suggestion:

those classes. The library directory is used as an additional sourcepath for test compilation.
So jtreg implicitly compiles all test dependencies that are located in a library directory.
A test that relies upon library classes might contain appropriate
<code>@build</code> directives to explicitly compile classes.
This might be useful when a test depends on library classes during runtime only.

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

PR Review: https://git.openjdk.org/jtreg/pull/256#pullrequestreview-2772661934
PR Review Comment: https://git.openjdk.org/jtreg/pull/256#discussion_r2047042748
PR Review Comment: https://git.openjdk.org/jtreg/pull/256#discussion_r2047015200