[anneal] Add playground compiler backend - #3539
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## G65mwqxbdjmwi2jj4gocgwsitq76ek3dh #3539 +/- ##
==================================================================
Coverage 91.85% 91.85%
==================================================================
Files 20 20
Lines 6093 6093
==================================================================
Hits 5597 5597
Misses 496 496 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8031649 to
0a09e52
Compare
gherrit-pr-id: Gklylrwoplohejnfq3oo4uymvshcq3o62
0a09e52 to
bdaf71d
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdaf71dcee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }; | ||
|
|
||
| let prepare_start = time::Instant::now(); | ||
| let (d, w, m) = try_join!(delete_files, write_files, modify_cargo_toml)?; |
There was a problem hiding this comment.
When two requests use the same Container concurrently (for example, one prewarmed stable coordinator serving multiple WebSocket requests), both do_request instances rewrite the same project files before spawn_cargo_task acquires any process permit. The later request can overwrite src/main.rs, Cargo.toml, or anneal-workspace/src/main.rs after the earlier request has prepared them but before cargo reads them, so the earlier response can compile or verify another user's code; guard the whole prepare/spawn/read sequence with a per-container lock or isolate each job in its own directory.
Useful? React with 👍 / 👎.
| job_id = stdin_shutdown_rx.recv() => { | ||
| let job_id = job_id.context(StdinShutdownReceiverEndedSnafu)?; | ||
| state.stdin_close(job_id); |
There was a problem hiding this comment.
Every process inserts a DropGuard into kill_tokens when it starts, but on the normal completion path the shutdown notification only removes the stdin sender. In a long-lived worker container that handles many successful executions, kill_tokens therefore grows without bound (one token per job) until the worker exits; remove the token when processing the job-completion/shutdown notification as well.
Useful? React with 👍 / 👎.
|
|
||
| let process_id = process_id.try_into().context(ProcessIdOutOfRangeSnafu { process_id })?; | ||
|
|
||
| let process = Process::new(process_id).context(InvalidProcessSnafu { process_id })?; |
There was a problem hiding this comment.
When a command exits before the blocking statistics task opens its /proc entry, Process::new returns InvalidProcess, and process_end later turns that statistics-task error into a failed worker operation instead of returning the child’s real exit status. This can make short successful commands, such as version probes or cache-hit builds under load, surface as backend errors; treat missing stats for an already-exited child as no statistics rather than failing the command.
Useful? React with 👍 / 👎.
| fn delete_files(&self) -> impl Iterator<Item = DeleteFileRequest> { | ||
| let files = match self.execution_tool { | ||
| ExecutionTool::AnnealVerify => vec![], | ||
| ExecutionTool::Cargo => vec![delete_previous_primary_file_request(self.crate_type)], |
There was a problem hiding this comment.
For Cargo execution requests this only removes the alternate src file and rewrites the primary source, but the program that cargo run executes has /playground as its writable cwd. A previous user can leave files such as build.rs or .cargo/config.toml behind, and Cargo will auto-detect and run/apply them on later users' builds in the same long-lived container; clean the package directory or run each request in an isolated workspace before accepting another request.
Useful? React with 👍 / 👎.
| # https://stackoverflow.com/a/7577209/155423 | ||
| cargo "build" "--target=wasm32-unknown-unknown" ${args[@]+"${args[@]}"} | ||
|
|
||
| for wasm in $(find target/ -name '*wasm' -not -path '*/deps/*'); do |
There was a problem hiding this comment.
This scans every non-deps .wasm under the shared target/ tree and converts each one, so a long-lived container can return stale output from an earlier wasm compile (for example, a prior debug artifact when the current request is release, or a prior cdylib when the current crate type produces no wasm). Restrict the path to the artifact Cargo just built, or clear/fail when there is not exactly one current output, before copying to compilation.
Useful? React with 👍 / 👎.
2 participants
Latest Update: v12 — Compare vs v11
📚 Full Patch History
Links show the diff between the row version and the column version.
⬇️ Download this PR
Branch
git fetch origin refs/heads/Gklylrwoplohejnfq3oo4uymvshcq3o62 && git checkout -b pr-Gklylrwoplohejnfq3oo4uymvshcq3o62 FETCH_HEADCheckout
git fetch origin refs/heads/Gklylrwoplohejnfq3oo4uymvshcq3o62 && git checkout FETCH_HEADCherry Pick
git fetch origin refs/heads/Gklylrwoplohejnfq3oo4uymvshcq3o62 && git cherry-pick FETCH_HEADPull
Stacked PRs enabled by GHerrit.