On Fri, 9 Sep 2022 11:41:08 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:

>> Currently, when doing a small change inside a module that does not affect 
>> the API of the module, the build system will skip rebuild of the dependent 
>> modules. If there's a change that affects the module's API, the dependent 
>> modules are recompiled. So far, this seems to work reasonably.
>> 
>> But, for large modules, like `java.base`, any change to any of the module's 
>> sources will cause a full rebuild of that module, which may take tens of 
>> seconds. This patch strives to improve that, by:
>> -parsing only sources that were modified first,
>> -computing an "API" hash for them,
>> -if there is not a significant change in the file(s), only recompile the 
>> files, if there are significant changes to the files, recompile the whole 
>> module, as before
>> 
>> There is no attempt made to determine the minimal set of files that needs to 
>> be reparsed, all module's files are reparsed when the internal API check 
>> fails. This is in line with the existing code to skip the build of dependent 
>> modules.
>> 
>> Sadly, this is overall a bit more difficult to do, so the implementation 
>> uses some javac internals. The existing build-only `Depend` plugin is 
>> enhanced so that parsing of the initial files is redirected to go through 
>> the plugin, and it decides parses either only the modified files, or all 
>> files. This redirection requires a change to `main/JavaCompiler`, which is 
>> not in the boot JDK, so reflection is currently used to install the 
>> redirection. Once the new classes are in a boot JDK, we could clean up, and 
>> avoid the reflection.
>> 
>> On my laptop, doing:
>> 
>> touch src/java.base/share/classes/java/lang/Object.java ; time make
>> 
>> can take well over 30s without this patch. With this patch, it takes <5s for 
>> me. Changes to e.g. only method bodies should typically not require rebuild 
>> of the whole module.
>> 
>> The caveats of this patch include:
>> -it is more involved than the existing plugin
>> -it only speeds up only incremental Java compilation, so the other tasks the 
>> build system is doing when running `make`, like timestamp checks or starting 
>> the build server, are still happening. This may slow down builds on 
>> platforms with slow I/O
>> -for modules that are part of the interim toolchain (like jdk.compiler), the 
>> interim build is not using this new approach, and it will likely be slow. We 
>> might try to improve once the required hooks are part of the boot JDK
>> -as the check for modified "internal" API cannot use attribution (as it runs 
>> too early), it tries to be safe, and will mark some changes as "internal API 
>> changes", even if they in fact don't affect the file's API. This may be 
>> especially true for some changes to imports.
>> -it is necessary to store an (internal) API hash for every file, in addition 
>> to one for every module which is stored currently
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Regenerating modfiles as necessary.

This looks good to me now. Thank you for fixing this!

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

Marked as reviewed by erikj (Reviewer).

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

Reply via email to