RFR: 8293116: Incremental JDK build could be sped up [v2]

[Erik Joelsson]

On Thu, 1 Sep 2022 12:49:09 GMT, Jan Lahoda <jlahoda at 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.

Sorry, I was so focused on JavaCompilation.gmk yesterday so I forgot about the rest of the build changes.

make/CompileInterimLangtools.gmk line 56:

> 54: 	$(call MakeDir, $(@D))
> 55: 	$(SED) $(INTERIM_TOOL_PROVIDER_PATTERN) $< > $@
> 56: 

Could you introduce a new section here with a comment header briefly describing what this is doing and why? It's currently under a section describing module-info.java. Let the last line of each section be the `TARGETS += ...` line.

make/CompileInterimLangtools.gmk line 92:

> 90:           Standard.java, \
> 91:       EXTRA_FILES := $(BUILDTOOLS_OUTPUTDIR)/gensrc/$1.interim/module-info.java \
> 92:                      $($1.interim_EXTRA_FILES), \

Style nit. We try to avoid aligning broken up lines like this. Please just use 4 space continuation indent. (See https://openjdk.org/groups/build/doc/code-conventions.html)

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

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