perf: byte-level path handling on Unix to bypass Components iterator#245
Draft
stormslowly wants to merge 1 commit into
Draft
perf: byte-level path handling on Unix to bypass Components iterator#245stormslowly wants to merge 1 commit into
stormslowly wants to merge 1 commit into
Conversation
Merging this PR will improve performance by 5.13%
Performance Changes
Tip Curious why this is faster? Comment Comparing |
3 tasks
On Unix, an OsStr is raw bytes and '/', '.', '..' are always single-byte ASCII, so the heavy std::path::Components state machine (Windows-prefix detection, Component enum construction, double-ended-iter bookkeeping) is unnecessary for the resolver's hot paths. Changes: - src/path.rs: unix_normalize and unix_normalize_with replace the Components-based PathUtil::normalize / normalize_with on Unix. - src/cache.rs: byte-level parent_path replaces Path::parent in Cache::value; join_last_segment replaces Path::strip_prefix + normalize_with in realpath_uncached, since parent.path is already a strict byte prefix of self.path. Note: an earlier draft of this PR also rewrote require_without_parse to dispatch on the specifier head via a byte table. That hunk was dropped (see #246) because the maintenance cost of keeping a parallel impl of Path::components in sync with std outweighed the small isolated win. The remaining changes here cover the dominant Ir cost paths. 138/138 non-PNP unit tests pass. The 6 PNP tests already fail on main without these changes.
bc97d1c to
8cefe78
Compare
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
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.
Why
std::path::Componentsruns a full state machine on every iteration — Windows-prefix detection,Componentenum construction, double-ended-iter bookkeeping. On Unix the resolver doesn't need any of that:OsStris raw bytes and/,.,..are always single-byte ASCII regardless of UTF-8 content in segments.What
src/path.rs—unix_normalizeandunix_normalize_withreplaceComponents-basedPathUtil::normalize/normalize_withon Unix. Non-Unix paths keep the existing implementation.src/cache.rs— byte-levelparent_pathreplacesPath::parentinCache::value(the recursive parent walk that drove most ofparse_next_component_back).realpath_uncacheduses a newjoin_last_segmentinstead ofPath::strip_prefix+normalize_with, sinceparent.pathis already a strict byte prefix ofself.pathwhen produced byparent_path.Scope change
An earlier revision of this PR also rewrote
require_without_parseto dispatch on the specifier's first byte via a hand-rolledclassify_specifier_headtable. That hunk was extracted to #246 and closed — the maintenance cost of keeping a parallel impl ofPath::componentsin sync with std outweighed the small isolated win.The previous PR body cited −7.28% Ir on
resolver[single-thread]; that number included the now-dropped opt3. A fresh callgrind diff for opt1+opt2 alone is pending and will be posted before this PR is taken out of draft.Test plan
cargo test --lib -- --skip 'tests::pnp::'— 138 / 138 pass (1 environment-only symlink fixture failure also present on main)cargo clippy --all-features -- -D warnings— cleanresolver[single-thread]for opt1+opt2 alone#[cfg(not(unix))]branches