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

Avoid the Box in `TyCtxt::associated_items`. #55604

Merged
merged 1 commit into from Nov 12, 2018

Conversation

@nnethercote
Copy link
Contributor

commented Nov 2, 2018

This reduces instruction counts on packed_simd by 2%.

r? @nikomatsakis

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

@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

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

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

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

(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

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

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

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Nov 2, 2018

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

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

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

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Nov 4, 2018

@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 nnethercote:avoid-associated_items-Box branch from 433635f to 54409b6 Nov 12, 2018

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2018

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

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2018

@bors try

@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

⌛️ Trying commit 54409b6 with merge eaa94a5...

bors added a commit that referenced this pull request Nov 12, 2018

Auto merge of #55604 - nnethercote:avoid-associated_items-Box, r=<try>
Avoid the Box in `TyCtxt::associated_items`.

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

r? @nikomatsakis
@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2018

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

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

📌 Commit 54409b6 has been approved by nnethercote

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Nov 12, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

⌛️ Testing commit 54409b6 with merge c6c23ed...

bors added a commit that referenced this pull request Nov 12, 2018

Auto merge of #55604 - nnethercote:avoid-associated_items-Box, r=nnet…
…hercote

Avoid the Box in `TyCtxt::associated_items`.

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

r? @nikomatsakis
@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

💔 Test failed - status-travis

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Nov 12, 2018

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)

Avoid the Box in `TyCtxt::associated_items`.
This reduces instruction counts on packed_simd by 2%.

@nnethercote nnethercote force-pushed the nnethercote:avoid-associated_items-Box branch from 54409b6 to e927a24 Nov 12, 2018

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2018

@bors retry

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2018

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

📌 Commit e927a24 has been approved by nnethercote

@bors

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

⌛️ Testing commit e927a24 with merge 0195812...

bors added a commit that referenced this pull request Nov 12, 2018

Auto merge of #55604 - nnethercote:avoid-associated_items-Box, r=nnet…
…hercote

Avoid the Box in `TyCtxt::associated_items`.

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

r? @nikomatsakis
@bors

This comment has been minimized.

Copy link
Contributor

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

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@nnethercote nnethercote deleted the nnethercote:avoid-associated_items-Box branch Nov 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.