Skip to content

Accelerate startup time of make #2110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 7, 2019

Conversation

dscho
Copy link
Member

@dscho dscho commented Mar 7, 2019

There is a find call in the Makefile to list all the header files that is pretty expensive on Windows, especially in scenarios where there are multiple worktrees as subdirectories of the main worktree, or when there are unrelated worktrees under the main worktree.

This patch already made it into git.git's next, and I'd like to have it in Git for Windows early, to accelerate my own development (and that of others, too).

In d85b0df (Makefile: use `find` to determine static header
dependencies, 2014-08-25), we switched from a static list of header
files to a dynamically-generated one, asking `find` to enumerate them.

Back in those days, we did not use `$(LIB_H)` by default, and many a
`make` implementation seems smart enough not to run that `find` command
in that case, so it was deemed okay to run `find` for special targets
requiring this macro.

However, as of ebb7baf (Makefile: add a hdr-check target,
2018-09-19), $(LIB_H) is part of a global rule and therefore must be
expanded. Meaning: this `find` command has to be run upon every
`make` invocation. In the presence of many a worktree, this can tax the
developers' patience quite a bit.

Even in the absence of worktrees or other untracked files and
directories, the cost of I/O to generate that list of header files is
simply a lot larger than a simple `git ls-files` call.

Therefore, just like in 3353397 (Makefile: ask "ls-files" to list
source files if available, 2011-10-18), we now prefer to use `git
ls-files` to enumerate the header files to enumerating them via `find`,
falling back to the latter if the former failed (which would be the case
e.g. in a worktree that was extracted from a source .tar file rather
than from a clone of Git's sources).

This has one notable consequence: we no longer include `command-list.h`
in `LIB_H`, as it is a generated file, not a tracked one, but that is
easily worked around. Of the three sites that use `LIB_H`, two
(`LOCALIZED_C` and `CHK_HDRS`) already handle generated headers
separately. In the third, the computed-dependency fallback, we can just
add in a reference to $(GENERATED_H).

Likewise, we no longer include not-yet-tracked header files in `LIB_H`.

Given the speed improvements, these consequences seem a comparably small
price to pay.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho requested review from drizzd and landstander668 March 7, 2019 09:04
@dscho
Copy link
Member Author

dscho commented Mar 7, 2019

This is the version that made it to next: git@92b88eba9f7e (the only changes are the two new footers in the commit message).

@dscho dscho requested a review from orgads March 7, 2019 11:57
@dscho
Copy link
Member Author

dscho commented Mar 7, 2019

Thanks, @orgads!

@dscho dscho merged commit 89cb7de into git-for-windows:master Mar 7, 2019
@dscho dscho added this to the v2.21.0(2) milestone Mar 7, 2019
@dscho dscho deleted the avoid-find-in-makefile branch March 11, 2019 14:32
dscho added a commit to dscho/git that referenced this pull request May 13, 2019
git-for-windows-ci pushed a commit that referenced this pull request May 14, 2019
dscho added a commit that referenced this pull request May 21, 2019
dscho added a commit that referenced this pull request May 22, 2019
dscho added a commit that referenced this pull request May 25, 2019
dscho added a commit that referenced this pull request Jun 4, 2019
dscho added a commit that referenced this pull request Jun 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants