Skip to content
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

fix performance regression #96

Merged
merged 2 commits into from
Feb 21, 2018
Merged

fix performance regression #96

merged 2 commits into from
Feb 21, 2018

Conversation

BurntSushi
Copy link
Owner

This PR fixes a performance regression introduced by 0f4441f. We had accidentally introduced an additional stat call for every directory entry on Windows.

In some cases, we were relying on things like "not(unix)" to mean "windows"
or "not(windows)" to mean "unix". Instead, we should split this in three
cases: unix, windows or not(unix or windows).
This commit fixes a performance regression introduced in commit 0f4441,
which aimed to fix OneDrive traversals. In particular, we added an
additional stat call to every directory entry, which can be quite
disastrous for performance. We fix this by being more fastidious about
reusing the Metadata that comes from fs::DirEntry, which is, conveniently,
cheap to acquire specifically on Windows.

The performance regression was reported against ripgrep:
BurntSushi/ripgrep#820
@BurntSushi BurntSushi merged commit ddac44a into master Feb 21, 2018
@BurntSushi BurntSushi deleted the ag/fix-perf branch February 21, 2018 00:24
BurntSushi added a commit to BurntSushi/ripgrep that referenced this pull request Feb 21, 2018
This commit fixes a performance regression in Windows that resulted from
fallout from fixing #705. In particular, we introduced an additional
stat call for every single directory entry, which can be quite
disastrous for performance.

There is a corresponding companion PR that fixes the same bug in
walkdir: BurntSushi/walkdir#96

Fixes #820
BurntSushi added a commit to BurntSushi/ripgrep that referenced this pull request Feb 21, 2018
This commit fixes a performance regression in Windows that resulted from
fallout from fixing #705. In particular, we introduced an additional
stat call for every single directory entry, which can be quite
disastrous for performance.

There is a corresponding companion PR that fixes the same bug in
walkdir: BurntSushi/walkdir#96

Fixes #820
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.

1 participant