Optimize analyizing source paths during evaluation - #47
Draft
sonkehahn-shopify wants to merge 2 commits into
Draft
Optimize analyizing source paths during evaluation#47sonkehahn-shopify wants to merge 2 commits into
sonkehahn-shopify wants to merge 2 commits into
Conversation
`GitSourceAccessor::readBlob` asked libgit2 for the `filter` attribute of every single blob it read. That lookup is O(number of `.gitattributes` rules): `git_attr_get_ext` is passed no attribute session, so it redoes its attribute setup and rematches every rule of every parent directory's `.gitattributes` on each call. On a World-shaped tree (a root `.gitattributes` with ~1400 anchored LFS rules) that is ~25us per file. Sampling `nix eval` on a synthetic clone of that shape (10k files) showed `lfs::Fetch::shouldFetch` taking 513 of 2273 main-thread samples -- 23% of the whole ingestion. A blob can only need smudging if it is a git-lfs pointer, and the spec requires a pointer to start with the `version` key. Check that first and only ask git about the handful of blobs that pass. After the change the same profile shows 2 samples, and total CPU for the benchmark drops from 3.82s to 3.20s (this is an -O0 build, so the win is larger in a release build where libgit2 is a bigger share). Behaviour is unchanged for the emitted content. The only visible difference is that an lfs-enrolled file whose blob does not even start with `version ` (i.e. content committed without the filter running) is now passed through silently instead of warning "should have been a git-lfs pointer" -- detecting that case is exactly what costs O(rules) per file. Files that do start with `version ` still warn as before. Note that `GitExportIgnoreSourceAccessor::isExportIgnored` pays the same per-path cost (496 of 2273 samples in the same profile) and is not addressed here.
`GitExportIgnoreSourceAccessor::isAllowedUncached` asked libgit2 for the
`export-ignore` attribute of every path it filtered. `git_attr_get_ext` is
called without an attribute session, so each call re-runs `attr_setup` and
then rematches *every* rule of *every* applicable `.gitattributes` with
`wildmatch` — there is no literal fast path in `git_attr_fnmatch__match`.
Ingesting a directory of a monorepo whose root `.gitattributes` carries
~1400 (mostly git-lfs) rules therefore costs O(rules x files).
Nothing about the *content* of a file can rule out `export-ignore`, so the
trick used for git-lfs pointers does not apply. What can be ruled out cheaply
is a whole directory: the attribute can only be assigned by a source that
spells its name out, and the sources that apply to a directory's entries are
its ancestors' `.gitattributes` files plus `$GIT_DIR/info/attributes` and the
global `core.attributesfile` (`GIT_ATTR_CHECK_NO_SYSTEM` excludes only the
system-wide file).
So `GitRepo::mayExportIgnore(commit, path)` substring-searches those sources
once, and the accessor memoises the answer per directory, chaining each
directory onto its parent's. Where it comes out false — the common case — the
libgit2 lookup is skipped entirely, turning O(files) attribute resolutions
into O(directories) tree lookups. The gate is conservative in both directions:
an unreadable external attribute file, a mere mention in a comment, or a
working-directory accessor (where libgit2 resolves attributes against the
index, which we cannot read through `next`) all fall back to asking libgit2.
Measured on a synthetic monorepo-shaped repository (10000 files, the World
root `.gitattributes`), `fetchGit { exportIgnore = true; }`:
`isExportIgnored` drops from 551/2266 profile samples (24%) to 11/2594 (0.4%),
and CPU time from 3.00s to 2.14s in an -O0 build.
Also generalises the unit-test fixture to build nested trees, and adds tests
for a rule in a subdirectory and for a rule above `attrPathPrefix` — neither
was covered before (the functional test's `exportIgnore = true` case is
commented out).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.