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

[do not merge] Measure upper limit for performance of 32 bit Span #59749

Closed
wants to merge 3 commits into from

Conversation

petrochenkov
Copy link
Contributor

What would happen if span interner accesses were cheap and didn't require any synchronization?

r? @ghost

@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Apr 6, 2019

⌛ Trying commit 6930ecbb0c927fb458bfde7fa6bb82be26ddef18 with merge 79b69238624df985b4415688efea25470ba8a824...

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 6, 2019
@petrochenkov
Copy link
Contributor Author

TODO: Exclude rustdoc from bors try for this PR.

@nnethercote
Copy link
Contributor

This will be interesting. #59693 (comment) suggests that roughly half the speedup of 64-bit spans comes from fewer TLS accesses, and the other half comes from fewer hash table accesses.

@petrochenkov petrochenkov force-pushed the upspan branch 2 times, most recently from 248ccec to 7c2ab8d Compare April 8, 2019 23:49
@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Apr 8, 2019

⌛ Trying commit 7c2ab8d with merge dc7e5bd...

bors added a commit that referenced this pull request Apr 8, 2019
[do not merge] Measure upper limit for performance of 32 bit `Span`

What would happen if span interner accesses were cheap and didn't require any synchronization?

r? @ghost
@rust-lang rust-lang deleted a comment from rust-highfive Apr 8, 2019
@rust-lang rust-lang deleted a comment from rust-highfive Apr 8, 2019
@rust-lang rust-lang deleted a comment from rust-highfive Apr 8, 2019
@rust-lang rust-lang deleted a comment from bors Apr 8, 2019
@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@petrochenkov
Copy link
Contributor Author

Oh, come on, why x.py dist tests rustbook, I disabled it.

@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Apr 9, 2019

⌛ Trying commit ed9912de2f7fc10c1f3ec969d9c835c833c374b8 with merge 3691212ca5933a51857de07b5a62084dabe81b82...

@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Apr 9, 2019

⌛ Trying commit af6e2d7 with merge 026939b...

@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Apr 11, 2019

⌛ Trying commit 4f208af with merge aff596d...

bors added a commit that referenced this pull request Apr 11, 2019
[do not merge] Measure upper limit for performance of 32 bit `Span`

What would happen if span interner accesses were cheap and didn't require any synchronization?

r? @ghost
@rust-highfive
Copy link
Collaborator

The job mingw-check 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:2f0f8140:start=1554970395325370316,finish=1554970397671287397,duration=2345917081
$ 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
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[00:03:16] configure: build.locked-deps    := True
[00:03:16] configure: llvm.ccache          := sccache
[00:03:16] configure: build.cargo-native-static := True
[00:03:16] configure: dist.missing-tools   := True
[00:03:16] configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
[00:03:16] configure: writing `config.toml` in current directory
[00:03:16] configure: 
[00:03:16] configure: run `python /checkout/x.py --help`
[00:03:16] configure: 

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)

@petrochenkov
Copy link
Contributor Author

cc @Mark-Simulacrum
Any hints on how to "disable running any rustdoc" in rustbuild on bors try?
So far bors try was behaving entirely differently from what I see on local attemts to run dist with or without docker/run.sh.

@bors
Copy link
Contributor

bors commented Apr 11, 2019

💔 Test failed - checks-travis

@rust-highfive
Copy link
Collaborator

The job dist-x86_64-linux 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_fold:end:services

travis_fold:start:git.checkout
travis_time:start:0e9b073d
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---

[01:44:09] travis_time:end:stage2-cargo-miri:start=1554976672979278033,finish=1554976673467240019,duration=487961986

[01:44:11] Dist docs (x86_64-unknown-linux-gnu)
[01:44:11] thread 'main' panicked at 'fs::read_dir(src) failed with No such file or directory (os error 2)', src/bootstrap/lib.rs:1228:18
[01:44:11] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
[01:44:11] Build completed unsuccessfully in 1:38:17
travis_time:end:2dff1b77:start=1554970423575529284,finish=1554976676247734959,duration=6252672205675
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 1.
---
travis_time:end:05a5f348:start=1554976678854571109,finish=1554976678877608078,duration=23036969
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:00aa6bf8
$ 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:0d29359a
travis_time:start:0d29359a
$ 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:057f9960
$ 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)

@petrochenkov
Copy link
Contributor Author

@bors try

@petrochenkov petrochenkov added S-waiting-on-perf Status: Waiting on a perf run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 11, 2019
@bors
Copy link
Contributor

bors commented Apr 11, 2019

⌛ Trying commit 955ca4c with merge d806815...

bors added a commit that referenced this pull request Apr 11, 2019
[do not merge] Measure upper limit for performance of 32 bit `Span`

What would happen if span interner accesses were cheap and didn't require any synchronization?

r? @ghost
@rust-highfive
Copy link
Collaborator

The job mingw-check 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:0004aec0:start=1555009905052951606,finish=1555009907088588091,duration=2035636485
$ 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
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[00:04:41]     Checking rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
[00:04:43]     Checking arena v0.0.0 (/checkout/src/libarena)
[00:04:43]     Checking syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:04:45]     Checking rustc_errors v0.0.0 (/checkout/src/librustc_errors)
[00:04:51] error[E0599]: no method named `as_ptr` found for type `lock_api::mutex::Mutex<parking_lot::raw_mutex::RawMutex, syntax_pos::span_encoding::SpanInterner>` in the current scope
[00:04:51]    --> src/libsyntax/lib.rs:105:64
[00:04:51]     |
[00:04:51] 105 |                     globals.syntax_pos_globals.span_interner.0.as_ptr();
[00:04:51] 
[00:04:57] error: aborting due to previous error
[00:04:57] 
[00:04:57] For more information about this error, try `rustc --explain E0599`.
---
travis_time:end:013531b4:start=1555010228105362502,finish=1555010228109808459,duration=4445957
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0d01932e
$ 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:000f4746
travis_time:start:000f4746
$ 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:09077760
$ 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
Contributor

bors commented Apr 11, 2019

☀️ Try build successful - checks-travis
Build commit: d806815

@petrochenkov
Copy link
Contributor Author

@rust-timer build d806815

@rust-timer
Copy link
Collaborator

Success: Queued d806815 with parent 3de0106, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit d806815

@nnethercote
Copy link
Contributor

The comparison URL is empty, showing an error message about "could not find commit..." :(

@Mark-Simulacrum
Copy link
Member

Yeah, that's a known bug. The parent commit is still building.

@matprec
Copy link
Contributor

matprec commented Apr 12, 2019

Parent commit is finished

@nnethercote
Copy link
Contributor

This gets roughly 75% of the Span64 improvements (#59693).

@petrochenkov
Copy link
Contributor Author

@nnethercote
On the largest crates that's true, but I wouldn't say that in general.
Here's the diff between this PR (the first column) and that PR (the second column).
https://perf.rust-lang.org/compare.html?start=d80681574370cb24650e053d4f30bf0e28f8b923&end=cfa77658d2ba937d22801e3a5222fcfc7f63ca47

@petrochenkov
Copy link
Contributor Author

I'd say that if we had perfectly inline-able TLS accesses or explicitly passed interner, then 64 bit spans would rather be a regression for most of crates beside script-servo.

@petrochenkov petrochenkov deleted the upspan branch June 5, 2019 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-perf Status: Waiting on a perf run to be completed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants