On Thu, 1 Sep 2022 12:49:09 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. 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