Skip to content

Avoid the Box in TyCtxt::associated_items. #55604

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 12, 2018
Merged

Avoid the Box in TyCtxt::associated_items. #55604

merged 1 commit into from
Nov 12, 2018

Conversation

nnethercote
Copy link
Contributor

This reduces instruction counts on packed_simd by 2%.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2018
@nnethercote
Copy link
Contributor Author

@nikomatsakis: a better approach would be to remove the box in TyCtxt::associated_items. I tried doing that but I couldn't satisfy the borrow checker, even after adding various uses of Captures. You added this box yourself here. Any ideas on how to remove it?

@nikomatsakis
Copy link
Contributor

I'll take a look. I can't see any deep reason for it to be there.

@nikomatsakis
Copy link
Contributor

(Oh, I think I know why it's there, that's so annoying. Yeah, I think I added it as a sort of workaround, but if it's perf sensitive I should be able to fix it.)

@nikomatsakis
Copy link
Contributor

@nnethercote -- what do you think of the commits I just pushed? I guess that achieves what you were going for.

@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (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. 
travis_time:end:05d04fdc:start=1541181908403189096,finish=1541182110486416912,duration=202083227816
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---

[00:03:25] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:26] tidy error: /checkout/src/librustc/ty/mod.rs:2684: line longer than 100 chars
[00:03:27] some tidy checks failed
[00:03:27] 
[00:03:27] 
[00:03:27] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:27] 
[00:03:27] 
[00:03:27] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:27] Build completed unsuccessfully in 0:00:47
[00:03:27] Build completed unsuccessfully in 0:00:47
[00:03:27] make: *** [tidy] Error 1
[00:03:27] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:18979efc
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:0d50a056:start=1541182328538817435,finish=1541182328544269505,duration=5452070
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2b32079e
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:1bc506a0
travis_time:start:1bc506a0
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:056bc524
$ dmesg | grep -i kill

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)

@nnethercote
Copy link
Contributor Author

Looks reasonable. I will profile them on Monday to make sure they work as expected. Thanks!

@nnethercote
Copy link
Contributor Author

@nikomatsakis: I re-measured and your patch has the same effect as mine, yay.

Do you want to file a new PR with just your patch so we don't pollute the repo history unnecessarily?

@nnethercote nnethercote force-pushed the avoid-associated_items-Box branch from 433635f to 54409b6 Compare November 12, 2018 02:48
@nnethercote
Copy link
Contributor Author

@nikomatsakis: I changed things so just your last patch is here.

@nnethercote
Copy link
Contributor Author

@bors try

@bors
Copy link
Collaborator

bors commented Nov 12, 2018

⌛ Trying commit 54409b665a8d5ac560482222dcc7ba54326abf54 with merge eaa94a5f22f00315e4c696c94af9efaa915e534f...

@nnethercote
Copy link
Contributor Author

Ugh, I didn't mean to do a try.

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 12, 2018

📌 Commit 54409b665a8d5ac560482222dcc7ba54326abf54 has been approved by nnethercote

@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 Nov 12, 2018
@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (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. 
travis_time:end:07e756c4:start=1541990997871281818,finish=1541991050963426870,duration=53092145052
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---

[00:03:53] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:54] tidy error: /checkout/src/librustc/ty/mod.rs:2682: line longer than 100 chars
[00:03:55] some tidy checks failed
[00:03:55] 
[00:03:55] 
[00:03:55] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:55] 
[00:03:55] 
[00:03:55] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:55] Build completed unsuccessfully in 0:00:52
[00:03:55] Build completed unsuccessfully in 0:00:52
[00:03:55] Makefile:79: recipe for target 'tidy' failed
[00:03:55] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:01e7c400
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Mon Nov 12 02:54:57 UTC 2018
---
travis_time:end:0e2759b8:start=1541991297743390759,finish=1541991297749524773,duration=6134014
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:039d7204
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0efbd71f
travis_time:start:0efbd71f
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:35e74b10
$ dmesg | grep -i kill

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
Copy link
Collaborator

bors commented Nov 12, 2018

⌛ Testing commit 54409b665a8d5ac560482222dcc7ba54326abf54 with merge c6c23ed659615a8167468daf935ec930a5298bdd...

@bors
Copy link
Collaborator

bors commented Nov 12, 2018

💔 Test failed - status-travis

@bors bors 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 Nov 12, 2018
@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (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. 
[00:04:07] [TIMING] ToolBuild { compiler: Compiler { stage: 0, host: "x86_64-unknown-linux-gnu" }, target: "x86_64-unknown-linux-gnu", tool: "tidy", path: "src/tools/tidy", mode: ToolBootstrap, is_optional_tool: false, source_type: InTree, extra_features: [] } -- 49.294
[00:04:07] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:07] tidy error: /checkout/src/librustc/ty/mod.rs:2682: line longer than 100 chars
[00:04:08] some tidy checks failed
[00:04:08] 
[00:04:08] 
[00:04:08] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
[00:04:08] 
[00:04:08] 
[00:04:08] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:08] Build completed unsuccessfully in 0:00:52
[00:04:08] Build completed unsuccessfully in 0:00:52
[00:04:08] Makefile:79: recipe for target 'tidy' failed
[00:04:08] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1022e9b6
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Mon Nov 12 04:10:45 UTC 2018
---
travis_time:end:02e37658:start=1541995846311874544,finish=1541995846318146814,duration=6272270
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:19e72430
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:129e9d29
travis_time:start:129e9d29
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:20912290
$ dmesg | grep -i kill

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)

@nikomatsakis @nnethercote
Avoid the Box in TyCtxt::associated_items.
e927a24 
This reduces instruction counts on packed_simd by 2%.
@nnethercote nnethercote force-pushed the avoid-associated_items-Box branch from 54409b6 to e927a24 Compare November 12, 2018 05:26
@nnethercote
Copy link
Contributor Author

@bors retry

@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 Nov 12, 2018
@nnethercote
Copy link
Contributor Author

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 12, 2018

📌 Commit e927a24 has been approved by nnethercote

@bors
Copy link
Collaborator

bors commented Nov 12, 2018

⌛ Testing commit e927a24 with merge 0195812...

bors added a commit that referenced this pull request Nov 12, 2018
@bors
Auto merge of #55604 - nnethercote:avoid-associated_items-Box, r=nnet…
0195812 
…hercote

Avoid the Box in `TyCtxt::associated_items`.

This reduces instruction counts on `packed_simd` by 2%.

r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Nov 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nnethercote
Pushing 0195812 to master...

@bors bors merged commit e927a24 into rust-lang:master Nov 12, 2018
@nnethercote nnethercote deleted the avoid-associated_items-Box branch November 12, 2018 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nnethercote @nikomatsakis @rust-highfive @bors