perf(path): write StringView components via write_view instead of copying to_owned#244
Merged
Merged
Conversation
…ying to_owned UnixPath::Show::output and WinPath::Show::output wrote each path component as logger.write_string(x.to_owned()) where x is a StringView. to_owned() copies the view into a newly allocated String, which write_string then copies again into the builder -- one allocation + two copies per component, where Logger already exposes write_view(StringView) doing the job in a single step. Path::normalize(p) is UnixPath::parse(p.0).to_string(), so every normalize goes through Show::output and pays the per-component copy. path_normalize bench (native, 3-run median, 10-segment path with .. and . x 200k iters): baseline: 282 ms patched : 209 ms (-25.9%)
peter-jerry-ye
force-pushed
the
pr-path-show-write-view
branch
from
ee69f7d to
53f5566
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
2 participants
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.
Summary
UnixPath'sShow::outputwrites each path component via:where
xis aStringView. Theto_owned()step copies the StringView into a newly allocatedString, whichwrite_stringthen immediately copies into the underlying builder. That's one allocation + two copies per component, where one copy would suffice.Loggeralready has awrite_view(StringView)method that targets exactly this case — no intermediateString.The same pattern appears in
path/win32/win_path.mbt'sWinPath::output(writes each component aswrite_string(component.to_owned())); fixed identically in this patch.Why this is hot
Path::normalize(p)isUnixPath::parse(p.0).to_string()— so every normalize call goes throughShow::outputand pays the per-component copy cost. On a realistic path with ~5–10 components the redundant work adds up fast.Benchmark
Scenario:
bench-x/cmd/path_normalize/main.mbt—Path::normalize+dirname+basename+extnameon a 10-segment path with../., × 200 000 iters. Native release, Linux x86_64, 3-run median wall time.Callgrind total instructions: 3.73 G → 2.71 G (-27.4%).
FixedArray::blit_from_stringfalls from 15.19% of the profile to 8.99% (and to a much smaller absolute number).Tests