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

[Leonid Mesnik]

On Wed, 26 Mar 2025 19:18:52 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 two additional commits since the last revision:
> 
>  - doc updated
>  - regression test added

Thank you for your feedback. See my comments inline. 
> _Mailing list message from [Jonathan Gibbons](mailto:jjg3 at pobox.com) on [jtreg-dev](mailto:jtreg-dev at mail.openjdk.org):_
> 
> On 3/25/25 10:52 AM, Leonid Mesnik 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.
> 
> Implicit compilation of the library classes into the test class directory is deplorable and should not be encourages.
> 
Most of tests are doing this already. The people are not going to add build tags if test works without them.  I don't think there is a way to encourage people to do this except by disabling implicit compilation. Even I do this once, it will be always the problematic to expect from jtreg test developers to add the build tags in new and update tests. 

> > For 2nd case
> > The library classes are compiled using @build tag and library classes are placed into some shared location.
> 
> Generally, library classes should be independent of any test classes and test class directory. As such, @build is the recommended way to build libraries.
> 
> > Class directory of a test case should be always used to compile a library
> 
> That would be an anti-pattern to be discouraged. Libraries are supposed to be shared between tests and as such, should not be beholden to any individual test class directory.

That's also that is broken for a lot of cases and since it was not restricted, the library tag might refer to mix of library and test code. 

So currently we have 2 type of libraries.
1) Shared, test-independent libraries like /test/lib that could be  shared as expected
There is an RFE to support such type of libraries separately. It might be decrease performance of single test execution for single test first time. However, it allow to better control libraries, support projects like Valhalla and also have fully incremented build. 
https://bugs.openjdk.org/browse/CODETOOLS-7903882

2)The test-mixed libraries, like "/" or "vmTesbase/"
that mix test and library code, and might even have same classes into different directories. 

Ideally, we'll get rid of them and switch to using only N known libraries, but so far need to support them without any test failures. 
So this fix is intended to do. Having feature that allow user to break the execution is not the good idea, better to have performance/disk penalty.
Requirement to always add @build for library classes is going to waste more developers time that I also prefer to avoid.

> 
> -- Jon

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

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