Skip to content

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

Merged
merged 4 commits into from
Mar 29, 2021
Merged

Reland rustwide and tokio upgrade #1332

merged 4 commits into from
Mar 29, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 24, 2021

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.

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.

jyn514 added 4 commits March 24, 2021 11:26
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.
@jyn514 jyn514 added the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label Mar 24, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 24, 2021

Ok cool, I ran about 80 builds with no leaks.
image

@jyn514 jyn514 added S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Mar 24, 2021
@jyn514 jyn514 changed the title [WIP] Reland rustwide and tokio upgrade Reland rustwide and tokio upgrade Mar 24, 2021
@Nemo157
Copy link
Member

Nemo157 commented Mar 26, 2021

Code looks good, I'm setting up an overnight test running builds to see if I get any leaks.

@Nemo157
Copy link
Member

Nemo157 commented Mar 29, 2021

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

image

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.

@jyn514 jyn514 merged commit 9f78dc7 into rust-lang:master Mar 29, 2021
@jyn514 jyn514 deleted the fix-leaks branch March 29, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RUSTSEC-2020-0053: dirs is unmaintained, use dirs-next instead
2 participants