Skip to content

bypass auto_da_alloc for metadata files #82047

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 8, 2021
Merged

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Feb 13, 2021

This saves about 0.7% when rerunning the UI test suite. I.e. when the metadata files exist and will be overwritten. No improvements expected for a clean build. So it might show up in incr-patched perf results.

regular rename:

Benchmark #1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     47.305 s ±  0.170 s    [User: 1631.540 s, System: 412.648 s]
  Range (min … max):   47.125 s … 47.856 s    20 runs

non-durable rename:

Benchmark #1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     46.930 s ±  0.064 s    [User: 1634.344 s, System: 396.038 s]
  Range (min … max):   46.759 s … 47.043 s    20 runs

There are more places that trigger auto_da_alloc behavior by overwriting existing files with O_TRUNC, but those are much harder to locate because O_TRUNC is set on open() but the writeback is triggered on close(). The latter is the part which shows up in profiles.

@rust-highfive
Copy link
Contributor

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 13, 2021
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 13, 2021

📌 Commit 1049d406098f129a8706d34e942feb2f0e64fe71 has been approved by davidtwco

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2021
@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 5, 2021
@crlf0710
Copy link
Member

crlf0710 commented Mar 5, 2021

Triage: a force push happened after r+. Switched back to waiting-on-review.

@davidtwco
Copy link
Member

Oops, missed this because the label was making my filter miss it.

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 5, 2021

📌 Commit 6a679ff has been approved by davidtwco

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2021
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Mar 6, 2021
bypass auto_da_alloc for metadata files

This saves about 0.7% when rerunning the UI test suite. I.e. when the metadata files exist and will be overwritten. No improvements expected for a clean build. So it might show up in incr-patched perf results.
```
regular rename:

Benchmark #1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     47.305 s ±  0.170 s    [User: 1631.540 s, System: 412.648 s]
  Range (min … max):   47.125 s … 47.856 s    20 runs

non-durable rename:

Benchmark #1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     46.930 s ±  0.064 s    [User: 1634.344 s, System: 396.038 s]
  Range (min … max):   46.759 s … 47.043 s    20 runs
```

There are more places that trigger auto_da_alloc behavior by overwriting existing files with O_TRUNC, but those are much harder to locate because `O_TRUNC` is set on `open()` but the writeback is triggered on `close()`. The latter is the part which shows up in profiles.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 6, 2021
bypass auto_da_alloc for metadata files

This saves about 0.7% when rerunning the UI test suite. I.e. when the metadata files exist and will be overwritten. No improvements expected for a clean build. So it might show up in incr-patched perf results.
```
regular rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     47.305 s ±  0.170 s    [User: 1631.540 s, System: 412.648 s]
  Range (min … max):   47.125 s … 47.856 s    20 runs

non-durable rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     46.930 s ±  0.064 s    [User: 1634.344 s, System: 396.038 s]
  Range (min … max):   46.759 s … 47.043 s    20 runs
```

There are more places that trigger auto_da_alloc behavior by overwriting existing files with O_TRUNC, but those are much harder to locate because `O_TRUNC` is set on `open()` but the writeback is triggered on `close()`. The latter is the part which shows up in profiles.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
bypass auto_da_alloc for metadata files

This saves about 0.7% when rerunning the UI test suite. I.e. when the metadata files exist and will be overwritten. No improvements expected for a clean build. So it might show up in incr-patched perf results.
```
regular rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     47.305 s ±  0.170 s    [User: 1631.540 s, System: 412.648 s]
  Range (min … max):   47.125 s … 47.856 s    20 runs

non-durable rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     46.930 s ±  0.064 s    [User: 1634.344 s, System: 396.038 s]
  Range (min … max):   46.759 s … 47.043 s    20 runs
```

There are more places that trigger auto_da_alloc behavior by overwriting existing files with O_TRUNC, but those are much harder to locate because `O_TRUNC` is set on `open()` but the writeback is triggered on `close()`. The latter is the part which shows up in profiles.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
bypass auto_da_alloc for metadata files

This saves about 0.7% when rerunning the UI test suite. I.e. when the metadata files exist and will be overwritten. No improvements expected for a clean build. So it might show up in incr-patched perf results.
```
regular rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     47.305 s ±  0.170 s    [User: 1631.540 s, System: 412.648 s]
  Range (min … max):   47.125 s … 47.856 s    20 runs

non-durable rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     46.930 s ±  0.064 s    [User: 1634.344 s, System: 396.038 s]
  Range (min … max):   46.759 s … 47.043 s    20 runs
```

There are more places that trigger auto_da_alloc behavior by overwriting existing files with O_TRUNC, but those are much harder to locate because `O_TRUNC` is set on `open()` but the writeback is triggered on `close()`. The latter is the part which shows up in profiles.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
bypass auto_da_alloc for metadata files

This saves about 0.7% when rerunning the UI test suite. I.e. when the metadata files exist and will be overwritten. No improvements expected for a clean build. So it might show up in incr-patched perf results.
```
regular rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     47.305 s ±  0.170 s    [User: 1631.540 s, System: 412.648 s]
  Range (min … max):   47.125 s … 47.856 s    20 runs

non-durable rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     46.930 s ±  0.064 s    [User: 1634.344 s, System: 396.038 s]
  Range (min … max):   46.759 s … 47.043 s    20 runs
```

There are more places that trigger auto_da_alloc behavior by overwriting existing files with O_TRUNC, but those are much harder to locate because `O_TRUNC` is set on `open()` but the writeback is triggered on `close()`. The latter is the part which shows up in profiles.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 8, 2021
bypass auto_da_alloc for metadata files

This saves about 0.7% when rerunning the UI test suite. I.e. when the metadata files exist and will be overwritten. No improvements expected for a clean build. So it might show up in incr-patched perf results.
```
regular rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     47.305 s ±  0.170 s    [User: 1631.540 s, System: 412.648 s]
  Range (min … max):   47.125 s … 47.856 s    20 runs

non-durable rename:

Benchmark rust-lang#1: touch src/tools/compiletest/src/main.rs ; RUSTC_WRAPPER="" schedtool -B -e ./x.py test src/test/ui
  Time (mean ± σ):     46.930 s ±  0.064 s    [User: 1634.344 s, System: 396.038 s]
  Range (min … max):   46.759 s … 47.043 s    20 runs
```

There are more places that trigger auto_da_alloc behavior by overwriting existing files with O_TRUNC, but those are much harder to locate because `O_TRUNC` is set on `open()` but the writeback is triggered on `close()`. The latter is the part which shows up in profiles.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#82047 (bypass auto_da_alloc for metadata files)
 - rust-lang#82415 (expand: Refactor module loading)
 - rust-lang#82557 (Add natvis for Result, NonNull, CString, CStr, and Cow)
 - rust-lang#82613 (Remove Item::kind, use tagged enum. Rename variants to match)
 - rust-lang#82642 (Fix jemalloc usage on OSX)
 - rust-lang#82682 (Implement built-in attribute macro `#[cfg_eval]` + some refactoring)
 - rust-lang#82684 (Disable destination propagation on all mir-opt-levels)
 - rust-lang#82755 (Refactor confirm_builtin_call, remove partial if)
 - rust-lang#82857 (Edit ructc_ast_lowering docs)
 - rust-lang#82862 (Generalize Write impl for Vec<u8> to Vec<u8, A>)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit da74a77 into rust-lang:master Mar 8, 2021
@cuviper cuviper added this to the 1.52.0 milestone Nov 16, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants