On Tue, 20 Aug 2024 09:02:18 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:

>> Currently incremental builds for the microbenchmarks may take notable amount 
>> of time, like:
>> 
>> $ touch test/micro/org/openjdk/bench/java/io/BlackholedOutputStream.java; 
>> time make test TEST=jaxp:tier1
>> Building target 'test' in configuration 'linux-x86_64-server-release'
>> Compiling up to 656 files for BUILD_JDK_MICROBENCHMARK
>> Running Indify on microbenchmark classes
>> [snip]
>> ==============================
>> Test summary
>> ==============================
>>    TEST                                              TOTAL  PASS  FAIL ERROR
>>    jtreg:test/jaxp:tier1                                 0     0     0     0
>> ==============================
>> TEST SUCCESS
>> 
>> Finished building target 'test' in configuration 
>> 'linux-x86_64-server-release'
>> 
>> real    0m37,581s
>> user    2m4,747s
>> sys     0m7,223s
>> 
>> 
>> The microbenchmark compilation is not using the `Depend` plugin that avoids 
>> recompilation of other files if the change files only contain minor changes 
>> (i.e. non-API changes). The patch here proposes to enhance the build to use 
>> the `Depend` plugin. The change that enables that is `CREATE_API_DIGEST := 
>> true,`, but since both the `Depend` plugin and JMH framework needs to be 
>> added to the classpath the patch re-organizes the code a little to properly 
>> augment the classpath.
>> 
>> With this patch, a build similar to the above might be:
>> 
>> $ touch test/micro/org/openjdk/bench/java/io/BlackholedOutputStream.java; 
>> time make test TEST=jaxp:tier1
>> Building target 'test' in configuration 'linux-x86_64-server-release'
>> Compiling up to 656 files for BUILD_JDK_MICROBENCHMARK
>> Running Indify on microbenchmark classes
>> [snip]
>> ==============================
>> Test summary
>> ==============================
>>    TEST                                              TOTAL  PASS  FAIL ERROR
>>    jtreg:test/jaxp:tier1                                 0     0     0     0
>> ==============================
>> TEST SUCCESS
>> 
>> Finished building target 'test' in configuration 
>> 'linux-x86_64-server-release'
>> 
>> real    0m7,505s
>> user    0m14,128s
>> sys     0m3,158s
>
> also interesting, but probably preexisting, and might be platform-specific 
> (tested on Cygwin):
> - compile benchmarks with `make test TEST=jaxp:tier1`
> - edit `test/micro/org/openjdk/bench/javax/crypto/full/AESGCMBench.java`
> - touch `test/micro/org/openjdk/bench/javax/crypto/full/BenchBase.java`
> - compile again with `make test TEST=jaxp:tier1` - fails - cannot find 
> CryptoBase
> - touch `test/micro/org/openjdk/bench/javax/crypto/full/CryptoBase.java`
> - run the modified benchmark with `make test TEST=micro:small.AESGCMBench` - 
> succeeds, but uses the old AESGCMBench.class file
> 
> if you touch all 3 files at once (AESGCMBench, BenchBase, CryptoBase) and 
> then make, the correct (new) file is used.
> 
> (I changed 128 to 129 in [this 
> line](https://github.com/openjdk/jdk/blob/b9d49dcef22ab81a087d890bbac0329a5244a2ef/test/micro/org/openjdk/bench/javax/crypto/full/AESGCMBench.java#L45).
>  With that change the benchmark throws an exception, so it's easy to check if 
> the file was recompiled or not)

@djelinski (and others), thanks for finding the problem, and I apologize for 
nor figuring this out myself. The problem is that when compiling (Java) named 
modules, if javac needs to read something from a classfile, it will (also) look 
into the output directory (besides system classes, etc.) So, the incremental 
build works, because any class that is necessary, but its compilation has been 
skipped, is load from output directory.

That is not the case when compiling code inside the unnamed module - javac will 
not, by itself, look into the output directory for classfiles. And the 
microbenchmarks are in the unnnamed module. So, when we skip compilation of 
e.g. `BenchBase`, there's no place where javac could read it from. The 
traditional way to solve this issue was to put the output directory to the 
classpath. I have an update for this patch that adds the output directory to 
the classpath, running tests on it now.

Sorry for not realizing/finding this myself.

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

PR Comment: https://git.openjdk.org/jdk/pull/20616#issuecomment-2299054321

Reply via email to