Reuse sandbox containers across build commands#127
Draft
syphar wants to merge 26 commits intorust-lang:mainfrom
Draft
Reuse sandbox containers across build commands#127syphar wants to merge 26 commits intorust-lang:mainfrom
syphar wants to merge 26 commits intorust-lang:mainfrom
Conversation
Attempt both sandbox container deletions during cleanup so one delete failure does not leak the other cached container.
Reuse a single sandbox container and enforce a fixed source mount kind across reused commands.
Move source mount mode selection from per-command state into SandboxBuilder so reusable sandboxes have a single fixed mount configuration.
Remove the unused sandbox mode without fixed directories and make start() construct the reusable session form directly.
syphar
changed the title
Avoid rebuilding an ephemeral container when the command workdir is inside the sandbox source tree.
Document the reused-container model and its callback limitations, and keep failed `docker rm` calls from hiding the leaked container ID during cleanup.
Create the container during sandbox startup so Docker failures surface before the first command, and reuse the same helper when recreating a stopped container.
Run sandboxed commands from the sandbox source directory by default instead of forcing ".". This keeps explicit workdirs inside the source tree and preserves the runtime panic for paths that escape it.
Summarize the new container reuse behavior and the breaking API changes in the unreleased notes.
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.
doing some tests I realised that only
docker createanddocker rmsometimes takes 5-10s (also very often it's <100ms).Docs.rs is running multiple commands:
for each target.
This leads me to the assumption that this could be a major reason why there is such a big difference between local builds and docs.rs builds for the same crate. The numbers roughly fit, #125 will give us more data.
Between these commands, we don't need a new container, we can just run more commands in the same container. This refactors rustwide into supporting this.
This also changes the memory-peak metric to be per-sandbox (= per container), and not per-command / call. Our source in docker is exactly that, and the API should reflect it. This is fine for the optimized docs.rs usage. Any
TODO