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

[Jaikiran Pai]

On Wed, 26 Mar 2025 00:45:39 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:
> 
>   tests fixed to expect lib classes along with test classes.

Hello Leonid,

Not a code review, but a general review about the proposal.

> The only reliable fix would be don't use shared class directory at all and compile all libraries for each test.

By all libraries, do you mean all classes that reside under the directory tree of each of the `@library` that been declared on a specific test definition?

> Although it looks like huge performance overhead, the impact is low. 

The before/after numbers that you posted for the JDK repo look reasonable. JDK isn't the sole user of `jtreg`, but yes I think it's the largest user, so we might have to consider if this change needs to be configurable in some way to allow select projects (like the JDK) to opt-in to this new behaviour.

> The libraries are not often compiled using build tag.

I have a contradicting understanding of that. In fact, the jtreg documentation here https://openjdk.org/jtreg/tag-spec.html for `@library` tag strongly recommends using `@build` for library classes used in the test:

> In general, classes in library directories are not automatically compiled as part of a compilation command explicitly naming the source files containing those classes. A test that relies upon library classes should contain appropriate @build directives to ensure that the classes will be compiled. It is strongly recommended that tests do not rely on the use of implicit compilation by the Java compiler. Such an approach is generally fragile, and may lead to incomplete recompilation when a test or library code has been modified.

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

PR Comment: https://git.openjdk.org/jtreg/pull/256#issuecomment-2754765881