On Thu, 1 Sep 2022 18:35:24 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:
> 
>   Reflecting review feedback.

make/common/JavaCompilation.gmk line 455:

> 453:          $$(call LogWarn, Compiling up to $$(words $$($1_SRCS)) files 
> for $1)
> 454:          $$(eval $$(call ListPathsSafely, $1_SRCS, $$($1_FILELIST)))
> 455:          $$(eval $1_MODFILES := $$?)

I think you really mean `$$($1_SRCS)` here, since the vardeps file is a build 
internal file that javac do not and should not care about. While I'm sure it 
will ignore it, the code here will be clearer if we don't pass it around when 
we shouldn't.

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

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

Reply via email to