Skip to content
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

perf: Avoid creating a SmallVec if nothing changes during a fold #68031

Merged
merged 3 commits into from Jan 26, 2020

Conversation

@Marwes
Copy link
Contributor

Marwes commented Jan 8, 2020

Not sure if this helps but in theory it should be less work than what
the current micro optimization does for ty::Predicate lists.

(It would explain the overhead I am seeing from perf.)

Not sure if this helps but in theory it should be less work than what
the current micro optimization does for `ty::Predicate` lists.

(It would explain the overhead I am seeing from `perf`.)
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 8, 2020

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@jonas-schievink

This comment has been minimized.

Copy link
Member

jonas-schievink commented Jan 8, 2020

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Jan 8, 2020

Awaiting bors try build completion

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 8, 2020

⌛️ Trying commit a5657db with merge 5193414...

bors added a commit that referenced this pull request Jan 8, 2020
perf: Avoid creating a SmallVec if nothing changes during a fold

Not sure if this helps but in theory it should be less work than what
the current micro optimization does for `ty::Predicate` lists.

(It would explain the overhead I am seeing from `perf`.)
@@ -1073,3 +1060,29 @@ impl<'tcx> TypeFoldable<'tcx> for InferConst<'tcx> {
false
}
}

fn fold_list<'tcx, F, T>(

This comment has been minimized.

Copy link
@jonas-schievink

jonas-schievink Jan 8, 2020

Member

Might be worth adding a comment here that explain that this is an optimization over .iter().map(...).collect()

This comment has been minimized.

Copy link
@Marwes

Marwes Jan 17, 2020

Author Contributor

Done in a1586f1

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 8, 2020

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-08T22:27:12.9716594Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-08T22:27:12.9731229Z ##[command]git config gc.auto 0
2020-01-08T22:27:12.9734455Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-08T22:27:12.9781187Z ##[command]git config --get-all http.proxy
2020-01-08T22:27:12.9786975Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/68031/merge:refs/remotes/pull/68031/merge
---
2020-01-08T22:53:50.1390775Z    Compiling cc v1.0.49
2020-01-08T22:53:50.1391190Z    Compiling core v0.0.0 (/checkout/src/libcore)
2020-01-08T22:53:55.4406678Z thread 'rustc' panicked at 'assertion failed: `(left == right)`
2020-01-08T22:53:55.4406906Z   left: `0`,
2020-01-08T22:53:55.4408055Z  right: `1`: destination and source slices have different lengths', /checkout/src/libcore/macros/mod.rs:18:13
2020-01-08T22:53:55.4408233Z 
2020-01-08T22:53:55.4408279Z error: internal compiler error: unexpected panic
2020-01-08T22:53:55.4408553Z 
2020-01-08T22:53:55.4424215Z note: the compiler unexpectedly panicked. this is a bug.
2020-01-08T22:53:55.4424215Z note: the compiler unexpectedly panicked. this is a bug.
2020-01-08T22:53:55.4424268Z 
2020-01-08T22:53:55.4425036Z note: we would appreciate a bug report: ***/blob/master/CONTRIBUTING.md#bug-reports
2020-01-08T22:53:55.4425093Z 
2020-01-08T22:53:55.4425453Z note: rustc 1.42.0-nightly (0ea29ce3e 2020-01-08) running on x86_64-unknown-linux-gnu
2020-01-08T22:53:55.4428839Z 
2020-01-08T22:53:55.4430932Z note: compiler flags: -Z external-macro-backtrace -Z binary-dep-depinfo -Z force-unstable-if-unmarked -C opt-level=2 -C codegen-units=1 -C debuginfo=0 -C link-args=-Wl,-rpath,$ORIGIN/../lib -C prefer-dynamic -C llvm-args=-import-instr-limit=10 -C debug-assertions=n --crate-type lib
2020-01-08T22:53:55.4434880Z note: some of the compiler flags provided by cargo are hidden
2020-01-08T22:53:55.4436013Z 
2020-01-08T22:53:55.4951605Z error: could not compile `core`.
2020-01-08T22:53:55.4970798Z warning: build failed, waiting for other jobs to finish...
---
2020-01-08T22:53:56.4329272Z   local time: Wed Jan  8 22:53:56 UTC 2020
2020-01-08T22:53:56.5279467Z   network time: Wed, 08 Jan 2020 22:53:56 GMT
2020-01-08T22:53:56.5283287Z == end clock drift check ==
2020-01-08T22:53:57.7401012Z 
2020-01-08T22:53:57.7528531Z ##[error]Bash exited with code '1'.
2020-01-08T22:53:57.7572343Z ##[section]Starting: Checkout
2020-01-08T22:53:57.7574427Z ==============================================================================
2020-01-08T22:53:57.7574508Z Task         : Get sources
2020-01-08T22:53:57.7574562Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 8, 2020

The job dist-x86_64-linux-alt of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-08T23:34:31.9457483Z    Compiling unwind v0.0.0 (/checkout/src/libunwind)
2020-01-08T23:34:33.6038547Z    Compiling backtrace-sys v0.1.32
2020-01-08T23:34:33.6724638Z thread 'rustc' panicked at 'assertion failed: `(left == right)`
2020-01-08T23:34:33.6732133Z   left: `0`,
2020-01-08T23:34:33.6732674Z  right: `1`: destination and source slices have different lengths', /rustc/5193414545ba1db55e954d1a68f1943cf4f258df/src/libcore/macros/mod.rs:18:13
2020-01-08T23:34:33.6732867Z 
2020-01-08T23:34:33.6732962Z error: internal compiler error: unexpected panic
2020-01-08T23:34:33.6733005Z 
2020-01-08T23:34:33.6733242Z note: the compiler unexpectedly panicked. this is a bug.
2020-01-08T23:34:33.6733242Z note: the compiler unexpectedly panicked. this is a bug.
2020-01-08T23:34:33.6733284Z 
2020-01-08T23:34:33.6734120Z note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
2020-01-08T23:34:33.6734180Z 
2020-01-08T23:34:33.6734420Z note: rustc 1.42.0-nightly (519341454 2020-01-08) running on x86_64-unknown-linux-gnu
2020-01-08T23:34:33.6734672Z 
2020-01-08T23:34:33.6735660Z note: compiler flags: -Z external-macro-backtrace -Z save-analysis -Z binary-dep-depinfo -Z force-unstable-if-unmarked -C opt-level=2 -C codegen-units=1 -C debuginfo=1 -C linker=clang -C link-args=-Wl,-rpath,$ORIGIN/../lib -C prefer-dynamic -C debug-assertions=n --crate-type lib
2020-01-08T23:34:33.6735893Z note: some of the compiler flags provided by cargo are hidden
2020-01-08T23:34:33.6735966Z 
2020-01-08T23:34:33.7603833Z    Compiling rustc_msan v0.0.0 (/checkout/src/librustc_msan)
2020-01-08T23:34:33.8232166Z    Compiling profiler_builtins v0.0.0 (/checkout/src/libprofiler_builtins)
---
2020-01-08T23:34:42.9178281Z   local time: Wed Jan  8 23:34:42 UTC 2020
2020-01-08T23:34:43.2460208Z   network time: Wed, 08 Jan 2020 23:34:43 GMT
2020-01-08T23:34:43.2461324Z == end clock drift check ==
2020-01-08T23:34:47.3871449Z 
2020-01-08T23:34:47.3989400Z ##[error]Bash exited with code '1'.
2020-01-08T23:34:47.4032290Z ##[section]Starting: Checkout
2020-01-08T23:34:47.4053823Z ==============================================================================
2020-01-08T23:34:47.4053930Z Task         : Get sources
2020-01-08T23:34:47.4054037Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 8, 2020

💔 Test failed - checks-azure

@Marwes

This comment has been minimized.

Copy link
Contributor Author

Marwes commented Jan 15, 2020

This could use a perf run now.

@estebank

This comment has been minimized.

Copy link
Contributor

estebank commented Jan 16, 2020

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Jan 16, 2020

Awaiting bors try build completion

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 16, 2020

⌛️ Trying commit a1586f1 with merge 64c086d...

bors added a commit that referenced this pull request Jan 16, 2020
perf: Avoid creating a SmallVec if nothing changes during a fold

Not sure if this helps but in theory it should be less work than what
the current micro optimization does for `ty::Predicate` lists.

(It would explain the overhead I am seeing from `perf`.)
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 17, 2020

☀️ Try build successful - checks-azure
Build commit: 64c086d (64c086d8ce1317f2d12547b9ea04989363c0cae6)

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Jan 17, 2020

Queued 64c086d with parent 4884061, future comparison URL.

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Jan 17, 2020

Finished benchmarking try commit 64c086d, comparison URL.

@Marwes

This comment has been minimized.

Copy link
Contributor Author

Marwes commented Jan 22, 2020

Perf results look good. Should be good to merge?

@estebank

This comment has been minimized.

Copy link
Contributor

estebank commented Jan 26, 2020

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 26, 2020

📌 Commit a1586f1 has been approved by estebank

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 26, 2020

⌛️ Testing commit a1586f1 with merge 086b2a4...

bors added a commit that referenced this pull request Jan 26, 2020
perf: Avoid creating a SmallVec if nothing changes during a fold

Not sure if this helps but in theory it should be less work than what
the current micro optimization does for `ty::Predicate` lists.

(It would explain the overhead I am seeing from `perf`.)
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 26, 2020

☀️ Test successful - checks-azure
Approved by: estebank
Pushing 086b2a4 to master...

@bors bors added the merged-by-bors label Jan 26, 2020
@bors bors merged commit a1586f1 into rust-lang:master Jan 26, 2020
5 checks passed
5 checks passed
homu Test successful
Details
pr Build #20200113.19 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-7) Linux x86_64-gnu-llvm-7 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
@Marwes Marwes deleted the Marwes:fold_list branch Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.