Repository navigation
Resolve less per extracted file in the CLI - #98
Merged
Merged
Conversation
The CLI's verified extraction checks every member's destination before it is staged and again before it is published. Three parts of that redid work whose result could not have changed: - open_entry_named (and its RAR5 and redirection variants) computed the checked output path twice in a row, with only an in-memory reservation between, walking every component with lstat both times. - reject_alias resolved the private staging root on every call. It is now resolved once, when the storage is created. - For a destination that does not exist yet, which is most of them, canonicalize walked every component before failing, in reject_alias and again in the duplicate-identity check before publishing. On Unix an lstat now answers "missing" first, and reject_alias hands the destination's resolved path, when it exists, to the identity check. On Windows a failed resolve is already one failed open, so nothing changes there. Every check still runs with the same inputs, at the same points. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…them Cache the resolved staging root on the first alias check instead of during directory creation. Cross-device publication-only staging keeps its original behavior without an unnecessary canonicalize call or new resolution failure. Add direct coverage for existing destination identities, missing descendants through staging aliases, dangling links, non-directory ancestors, loop and permission errors, and retry after failed initial root resolution. The ordinary extraction path retains the PR's syscall savings. Both the plan and completed change received an independent Claude review with no blockers.
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.
rars extractis slow per file rather than per byte. Under strace on Linux it makes about 92 system calls per extracted file, where UnRAR makes about 18. Most of those come from the verified-extraction checks. This PR keeps every check, with the same inputs and at the same points, and drops three places that redid work whose result could not have changed.open_entry_named, and its RAR5 and redirection variants, computed the checked output path twice back to back. Only an in-memory reservation ran in between, solstatwalked every component of the path twice for nothing.reject_aliascanonicalized the private staging root each time it ran, twice per member. It is now resolved once, on the first alias check. Temporary storage used only for cross-device publication does not resolve its root.canonicalizeresolved every component before failing with NotFound. That happened inreject_alias, and again in the duplicate-identity check before publishing.lstatnow answers "missing" first. Other errors still go tocanonicalize, which reports them as before.reject_aliasreturns the destination's resolved path when it exists, and the identity check uses it instead of resolving the path again.lstatwould only add a call; the shortcut is Unix-only.On Linux this takes extraction from 92 to 68 system calls per file for RAR5, and from 97 to 72 for RAR 2.9 LZ (strace, 221 files).
readlinkatcalls fall by 60%.Hosted runners,
rars --threads 1 extractagainstmaster, three rounds in turn on the same archives (2,300–2,700 files, 130–160 MB, written by rars), MB/s:Decoding alone (
rars test) is unchanged. On an M1 Pro, extracting 8,869 files goes from 23–24 to 26–27 MB/s (RAR5), and from 25–26 to 28–30 (RAR 2.9 LZ).Initial author validation:
cargo test -p rars-cli: 333 tests pass, including the two real-terminal tests (with an arm64python3).clippy -D warningsandfmtare clean.What is left is larger, and I'd rather ask before touching it. On Windows, 7-Zip still extracts those archives about three times faster (172–182 MB/s for RAR 2.9 LZ, 134–136 for RAR5). I timed the remaining per-file steps on their own; in ms per 1,000 files on a hosted Windows runner:
lstaton every component, four timescanonicalize, five timesCreating files relative to directory handles that are opened once, without following links (openat-style, for example through cap-std), costs the same as the plain create on Linux, macOS and Windows (185 against 181 ms on Windows). It also closes the window between checking a path and opening it.
If that direction is of interest, I can sketch it for the verified-extraction path in an issue first.
🤖 Generated with Claude Code
Maintainer review follow-up: