On Wed, 31 Aug 2022 17:51:31 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

This is a really cool feature, thanks for doing it!

make/common/JavaCompilation.gmk line 451:

> 449:     # $$($1_VARDEPS_FILE) is used as dependency to track changes in set 
> of
> 450:     # list of files.
> 451:     $$($1_COMPILE_TARGET): $$($1_SRCS) $$($1_DEPENDS) \

What's the reason for inlining this recipe into the main compilation recipe? I 
would think generating the modfiles file in the FILELIST rule would help reduce 
the noise from non source files in the prerequisites list (as the old FILELIST 
rule had a lot less prerequisites listed). Keeping this recipe separate also 
limits regeneration of the FILELIST.

make/common/JavaCompilation.gmk line 456:

> 454:          $$(call LogWarn, Compiling up to $$(words $$($1_SRCS)) files 
> for $1)
> 455:          $$(eval $$(call ListPathsSafely, $1_SRCS, $$($1_FILELIST)))
> 456:          $$(call WriteFile, "-XDmodifiedInputs=$$? ", $$@.modfiles)

I'm not sure WriteFile is safe to use here. `$?` could potentially contain all 
files in `$1_SRCS`, which is why we use ListPathsSafely in the first place. 
This is only a problem if using make <4.0, but as long as that's not 
prohibited, we need to keep it working without the file function.

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

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

Reply via email to