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 calling `unroll_place()` in a common case. #53733

Merged
merged 1 commit into from Aug 30, 2018

Conversation

Projects
None yet
6 participants
@nnethercote
Contributor

nnethercote commented Aug 27, 2018

This reduces the execution time for ucd-check by 25%.

r? @nikomatsakis

Avoid calling `unroll_place()` in a common case.
This reduces the execution time for `ucd-check` by 25%.
@arielb1

This comment has been minimized.

Contributor

arielb1 commented Aug 27, 2018

Borrows that start from 2 separate locals can never overlap (and of course legal programs can never have borrow overlap).

Why isn't it possible to perform binning by place (by changing each_borrow_involving_path to do an "intersection query")?

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Aug 27, 2018

@arielb1 that is what I was proposing in #53159 -- it takes some refactoring though to achieve.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Aug 27, 2018

Hmm. For the case of ucd-check in particular, I was hoping to just eliminate the borrows (that's what was proposed in #53643) and -- more generally -- to eventually refactor to a hashing-like scheme (as described in #53328 and #53643).

But, this is a super simple fix of course that seems worth landing regardless. I wouldn't mind putting off particularly the latter two refactorings if we can, as I might rather just try to do it as part of a move to a polonius based system. (Which will still want to solve similar problems, of course)

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Aug 27, 2018

Also true: this was also a motivator for the refactoring happening in #53247 -- it would allow us to do this check for a broader range of places (since we can access the base place in O(1) time)

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Aug 27, 2018

@bors r+

Anyway, as I said, seems worth landing this quick hack for now, why not?

@bors

This comment has been minimized.

Contributor

bors commented Aug 27, 2018

📌 Commit 130e556 has been approved by nikomatsakis

@memoryruins memoryruins added the A-NLL label Aug 28, 2018

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Aug 28, 2018

Rollup merge of rust-lang#53733 - nnethercote:avoid-unroll_place, r=n…
…ikomatsakis

Avoid calling `unroll_place()` in a common case.

This reduces the execution time for `ucd-check` by 25%.

r? @nikomatsakis
@bors

This comment has been minimized.

Contributor

bors commented Aug 29, 2018

⌛️ Testing commit 130e556 with merge 7d32905...

bors added a commit that referenced this pull request Aug 29, 2018

Auto merge of #53733 - nnethercote:avoid-unroll_place, r=nikomatsakis
Avoid calling `unroll_place()` in a common case.

This reduces the execution time for `ucd-check` by 25%.

r? @nikomatsakis
@bors

This comment has been minimized.

Contributor

bors commented Aug 29, 2018

💔 Test failed - status-travis

@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Aug 29, 2018

The job dist-i686-apple 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:05:10]       Memory: 8 GB
[00:05:10]       Boot ROM Version: VMW71.00V.0.B64.1704110547
[00:05:10]       Apple ROM Info: [MS_VM_CERT/SHA1/27d66596a61c48dd3dc7216fd715126e33f59ae7]Welcome to the Virtual Machine
[00:05:10]       SMC Version (system): 2.8f0
[00:05:10]       Serial Number (system): VMl7OBTl7RfY
[00:05:10] 
[00:05:10] hw.ncpu: 4
[00:05:10] hw.byteorder: 1234
[00:05:10] hw.memsize: 8589934592
---
[01:13:59] thread 'main' panicked at 'failed to download openssl source: exit code: 6', bootstrap/native.rs:611:17
[01:13:59] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:13:59] failed to run: /Users/travis/build/rust-lang/rust/build/bootstrap/debug/bootstrap build
[01:13:59] Build completed unsuccessfully in 1:07:18
[01:13:59] make: *** [all] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:00a93c74
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_fold:start:after_failure.2
travis_time:start:070f5b28
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
total 0
drwx------+ 15 travis  staff  510 Jan 25  2018 ..
drwx------   2 travis  staff   68 Dec  6  2017 .
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:157ad010
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
travis_time:end:157ad010:start=1535563661128347000,finish=1535563661164061000,duration=35714000
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:015a1c9a
$ 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 -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:11034564
travis_time:start:11034564
$ 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:1f8f916f
$ dmesg | grep -i kill
$ dmesg | grep -i kill
Unable to obtain kernel buffer: Operation not permitted
usage: sudo dmesg
travis_fold:end:after_failure.6

Done. Your build exited with 1.

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.

Contributor

nnethercote commented Aug 29, 2018

@bors retry

@bors

This comment has been minimized.

Contributor

bors commented Aug 30, 2018

⌛️ Testing commit 130e556 with merge c096519...

bors added a commit that referenced this pull request Aug 30, 2018

Auto merge of #53733 - nnethercote:avoid-unroll_place, r=nikomatsakis
Avoid calling `unroll_place()` in a common case.

This reduces the execution time for `ucd-check` by 25%.

r? @nikomatsakis
@bors

This comment has been minimized.

Contributor

bors commented Aug 30, 2018

💔 Test failed - status-travis

@bors bors removed the S-waiting-on-bors label Aug 30, 2018

@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Aug 30, 2018

The job dist-i686-freebsd 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:0f0609e9:start=1535612258616811800,finish=1535612258624815133,duration=8003333
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1adf8184
$ 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 -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:191a7834
travis_time:start:191a7834
$ 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:25869231
$ 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.

Contributor

nnethercote commented Aug 30, 2018

@bors retry

@bors

This comment has been minimized.

Contributor

bors commented Aug 30, 2018

⌛️ Testing commit 130e556 with merge 03fe4c7...

bors added a commit that referenced this pull request Aug 30, 2018

Auto merge of #53733 - nnethercote:avoid-unroll_place, r=nikomatsakis
Avoid calling `unroll_place()` in a common case.

This reduces the execution time for `ucd-check` by 25%.

r? @nikomatsakis
@bors

This comment has been minimized.

Contributor

bors commented Aug 30, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 03fe4c7 to master...

@bors bors merged commit 130e556 into rust-lang:master Aug 30, 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-unroll_place branch Aug 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment