-
Notifications
You must be signed in to change notification settings - Fork 212
Reland rustwide and tokio upgrade #1332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This reverts commit 87dd4be. The cause of the leak was that `Regex::new` leaked 2.5 MB of memory on every build (in `parse_rustc_version`). Searching around I found rust-lang/regex#742, which said this was a regression in thread_local. Updating thread_local to 1.1.3 fixed the issue. For future reference: I debugged the leak as follows. 1. Build the crate: `cargo build` 2. Run the daemon under massif without registry watching: `valgrind --tool=massif target/debug/cratesfyi daemon --registry-watcher disabled` 3. Add a whole bunch of crates to the queue: `for i in $(seq 1 100); do cargo run -q queue add serde 1.0.$i; done` 4. Stop the daemon with SIGINT when you get tired of waiting (I ran about 30 builds). 5. Run `massif-visualizer` on the generated file. Here are the changes to Cargo.lock: ``` > cargo update -p docs-rs Updating crates.io index Removing arrayref v0.3.6 Removing arrayvec v0.5.1 Adding attohttpc v0.16.3 Updating base64 v0.11.0 -> v0.13.0 Removing blake2b_simd v0.5.10 Adding bytes v1.0.1 Removing constant_time_eq v0.1.5 Updating core-foundation v0.7.0 -> v0.9.1 Updating core-foundation-sys v0.7.0 -> v0.8.2 Adding crypto-mac v0.10.0 Removing dirs v2.0.2 Adding dirs-next v2.0.0 Removing dirs-sys v0.3.5 Adding dirs-sys-next v0.1.2 Adding form_urlencoded v1.0.1 Adding getrandom v0.2.2 Updating h2 v0.2.5 -> v0.3.1 Adding hmac v0.10.1 Updating http-body v0.3.1 -> v0.4.1 Updating hyper v0.13.10 -> v0.14.4 Updating hyper-tls v0.4.1 -> v0.5.0 Adding ipnet v2.3.0 Updating js-sys v0.3.39 -> v0.3.49 Updating libc v0.2.74 -> v0.2.91 Adding mio v0.7.11 Removing mio-named-pipes v0.1.6 Updating miow v0.3.3 -> v0.3.7 Updating native-tls v0.2.4 -> v0.2.7 Adding ntapi v0.3.4 Updating once_cell v1.4.0 -> v1.7.2 Removing pin-project v0.4.17 Removing pin-project-internal v0.4.17 Adding postgres v0.19.0 Adding postgres-protocol v0.6.0 Adding postgres-types v0.2.0 Updating r2d2_postgres v0.16.0 -> v0.18.0 Adding rand v0.8.3 Adding rand_chacha v0.3.0 Adding rand_core v0.6.2 Adding rand_hc v0.3.0 Adding redox_syscall v0.2.5 Updating redox_users v0.3.4 -> v0.4.0 Updating reqwest v0.10.6 -> v0.11.2 Updating rusoto_core v0.45.0 -> v0.46.0 Updating rusoto_credential v0.45.0 -> v0.46.0 Updating rusoto_s3 v0.45.0 -> v0.46.0 Updating rusoto_signature v0.45.0 -> v0.46.0 Removing rust-argon2 v0.7.0 Updating rustwide v0.11.0 -> v0.12.0 Updating security-framework v0.4.4 -> v2.1.2 Updating security-framework-sys v0.4.3 -> v2.1.1 Updating serde_urlencoded v0.6.1 -> v0.7.0 Updating socket2 v0.3.12 -> v0.3.19 Adding tokio v1.4.0 Updating tokio-macros v0.2.5 -> v1.1.0 Adding tokio-native-tls v0.3.0 Adding tokio-postgres v0.7.0 Adding tokio-stream v0.1.5 Removing tokio-tls v0.3.1 Adding tokio-util v0.6.5 Updating url v2.1.1 -> v2.2.1 Adding wasi v0.10.2+wasi-snapshot-preview1 Updating wasm-bindgen v0.2.62 -> v0.2.72 Updating wasm-bindgen-backend v0.2.62 -> v0.2.72 Updating wasm-bindgen-futures v0.4.12 -> v0.4.22 Updating wasm-bindgen-macro v0.2.62 -> v0.2.72 Updating wasm-bindgen-macro-support v0.2.62 -> v0.2.72 Updating wasm-bindgen-shared v0.2.62 -> v0.2.72 Adding wildmatch v1.1.0 ```
``` $ cargo update -p postgres:0.17.5 Updating crates.io index Removing crypto-mac v0.8.0 Removing hmac v0.8.1 Removing mio-uds v0.6.8 Removing pin-project-lite v0.1.5 Removing postgres v0.17.5 Removing postgres-protocol v0.5.2 Removing postgres-types v0.1.3 Removing tokio v0.2.22 Removing tokio-postgres v0.5.5 Removing tokio-util v0.3.1 ```
This doesn't actually remove the duplicate 0.12 dependency until rustwide also updates.
Code looks good, I'm setting up an overnight test running builds to see if I get any leaks. |
I ran 200 builds on the server running in the docker-compose setup, checking the rss usage after each build: for i in $(seq 1 200); do
cargo run -- queue add bs58 0.4.0 2>/dev/null
sleep 180
ps -C cratesfyi -o rss --no-headers
done There is what looks like a slight leak, but it might just be heap fragmentation or something. It's the same as what I previously saw when testing e8a6303, I didn't record the full data the same as this, but at that version after 10 builds it was at 69MB and after 100 builds was at 173M, which are right on the line. Comparatively when testing 48444ef at 10 builds it had reached 109MB and 100 builds it had reached 530MB, so significantly more leakage. |
The cause of the leak was that
Regex::new
leaked 2.5 MB of memory onevery build (in
parse_rustc_version
). Searching around I foundrust-lang/regex#742, which said this was a
regression in thread_local. Updating thread_local to 1.1.3 fixed the
issue.
For future reference: I debugged the leak as follows.
cargo build
valgrind --tool=massif target/debug/cratesfyi daemon --registry-watcher disabled
for i in $(seq 1 100); do cargo run -q queue add serde 1.0.$i; done
massif-visualizer
on the generated file.Successor to #1264. Fixes #1102. Unblocks #1303.
This is a WIP because I tested the leaks with an ancient version of the lockfile, not the one in this PR; I need to test again before this merges.