Deduplicate some code and compile-time values around vtables by oli-obk · Pull Request #55016 · rust-lang/rust

oli-obk · 2018-10-12T15:17:15Z

oli-obk · 2018-10-12T15:17:24Z

bors · 2018-10-12T15:17:35Z

⌛ Trying commit ca8c9576ad36b9556102818a978182ef9413c53f with merge aaaada8d4890723455ce6f8fcb02f4af269ff2d5...

rust-highfive · 2018-10-12T15:51:17Z

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:16ffcbc0
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
[00:14:10] Compiling rustc_allocator v0.0.0 (/checkout/src/librustc_allocator)
[00:14:12] error: doc comment not used by rustdoc
[00:14:12] --> librustc_mir/interpret/traits.rs:52:9
[00:14:12] |
[00:14:12] 52 | /// If you touch this code, be sure to also make the corresponding changes to
[00:14:12] |
[00:14:12] = note: `-D unused-doc-comments` implied by `-D warnings`
[00:14:12] [00:14:23] error: aborting due to previous error
---
[00:15:23] travis_fold:end:stage0-rustc
[00:15:23] travis_time:end:stage0-rustc:start=1539358925796933868,finish=1539359437932254659,duration=512135320791
[00:15:23] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1112:9
[00:15:23] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
[00:15:23] Build completed unsuccessfully in 0:09:38
travis_time:end:0a6c36df:start=1539358513974048904,finish=1539359438194569767,duration=924220520863
---
travis_time:end:03bf0fca:start=1539359438672141567,finish=1539359438680370141,duration=8228574
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:009e5e46
$ 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:0fc6de26
travis_time:start:0fc6de26
$ 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:2978f7c0
$ 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)

oli-obk · 2018-10-12T16:06:10Z

ups, ordering issues

@bors try

bors · 2018-10-12T16:08:17Z

⌛ Trying commit fce7dc2 with merge 178f3ba...

@RalfJung

Deduplicate some code and compile-time values around vtables r? @RalfJung

bors · 2018-10-12T18:27:55Z

☀️ Test successful - status-travis
State: approved= try=True

oli-obk · 2018-10-12T20:58:15Z

@rust-timer build fce7dc2

rust-timer · 2018-10-12T20:58:15Z

Insufficient permissions to issue commands to rust-timer.

Mark-Simulacrum · 2018-10-12T21:00:59Z

@rust-timer build fce7dc2

Bot should now recognize @oli-obk .

rust-timer · 2018-10-12T21:01:00Z

Bors try commit fce7dc2 unexpectedly has 1 parents.

Mark-Simulacrum · 2018-10-12T21:01:20Z

@rust-timer build 178f3ba

rust-timer · 2018-10-12T21:01:21Z

Success: Queued 178f3ba with parent e9e27e6, comparison URL.

rust-timer · 2018-10-12T22:41:26Z

Finished benchmarking try commit 178f3ba

oli-obk · 2018-10-13T06:41:52Z

https://perf.rust-lang.org/compare.html?start=e9e27e6a6258b3adf00a5dd35d2676656224880d&end=178f3ba362979973aef288e95b6730595586ce00&stat=max-rss undoes the memory regression introduced in #54461 (comment) but seems to regress load of others?!

Ok I'm doing more in this PR than just undoing the regression, I'll split out the changes of this PR into separate PRs so we can observe them in isolation

oli-obk · 2018-10-13T07:41:00Z

@bors try

bors · 2018-10-13T07:41:11Z

⌛ Trying commit faa688b with merge b4b95a3...

@RalfJung

Deduplicate some code and compile-time values around vtables r? @RalfJung

RalfJung · 2018-10-13T08:48:16Z

 pub(crate) stack: Vec<Frame<'mir, 'tcx, M::PointerTag>>,
+
+ /// A cache for deduplicating vtables
+ pub(super) vtables: FxHashMap<(Ty<'tcx>, ty::PolyTraitRef<'tcx>), AllocId>,


I am a bit surprised to see this here and not in Memory, but I guess it does not matter much.

RalfJung · 2018-10-13T08:48:39Z

 param_env,
 memory: Memory::new(tcx, memory_data),
 stack: Vec::new(),
+ vtables: FxHashMap(),


I think we are supposed to use FxHashMap::default() these days.

RalfJung · 2018-10-13T08:54:09Z

r=me with the nits fixed and perf happy.

bors · 2018-10-13T09:59:11Z

☀️ Test successful - status-travis
State: approved= try=True

RalfJung · 2018-10-13T10:03:10Z

@rust-timer build b4b95a3

rust-timer · 2018-10-13T10:03:11Z

Success: Queued b4b95a3 with parent c47785f, comparison URL.

rust-timer · 2018-10-13T12:51:59Z

Finished benchmarking try commit b4b95a3

oli-obk · 2018-10-13T17:40:05Z

So my changes now are not causing perf regressions anymore (actually improving perf on some tests), but there are still some peak memory usage regressions.

Most notably

webrender-debug baseline incremental (6%)
- not sure how strongly this fluctuates, the previous run had 3%
coercions-debug baseline incremental (3%, marked with ? for fluctuations)
ripgrep-debug clean (3%, no regression in the previous run)
inflate-debug patched incremental: println (2%)

There's also a big swath of small (1%) improvements

RalfJung · 2018-10-16T08:47:01Z

@bors r+

bors · 2018-10-16T08:47:02Z

📌 Commit b1d3111 has been approved by RalfJung

bors · 2018-10-16T13:02:11Z

⌛ Testing commit b1d3111 with merge 3fd8be694dd129619f81b46944f5671e31359b8d...

bors · 2018-10-16T15:16:06Z

💔 Test failed - status-travis

rust-highfive · 2018-10-16T15:16:08Z

The job arm-android 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.

[01:39:03] test time::tests::system_time_elapsed ... ok
[01:39:03] test time::tests::system_time_math ... ok
[01:39:09] test sync::mpsc::tests::stress_recv_timeout_two_threads ... ok
[01:39:11] test collections::hash::map::test_map::test_lots_of_insertions ... ok
[01:39:44] test process::tests::test_process_output_fail_to_start ... test process::tests::test_process_output_fail_to_start has been running for over 60 seconds
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

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)

RalfJung · 2018-10-16T16:37:44Z

Timeout, guessing spurious.

@bors retry

@RalfJung

…, r=RalfJung Deduplicate some code and compile-time values around vtables r? @RalfJung

Rollup of 18 pull requests Successful merges: - #54646 (improve documentation on std::thread::sleep) - #54933 (Cleanup the rest of codegen_llvm) - #54964 (Run both lldb and gdb tests) - #55016 (Deduplicate some code and compile-time values around vtables) - #55031 (Improve verify_llvm_ir config option) - #55050 (doc std::fmt: the Python inspiration is already mentioned in precedin…) - #55077 (rustdoc: Use dyn keyword when rendering dynamic traits) - #55080 (Detect if access to localStorage is forbidden by the user's browser) - #55090 (regression test for move out of borrow via pattern) - #55102 (resolve: Do not skip extern prelude during speculative resolution) - #55104 (Add test for #34229) - #55111 ([Rustc Book] Explain --cfg's arguments) - #55122 (Cleanup mir/borrowck) - #55127 (Remove HybridBitSet::dummy) - #55128 (Fix LLVMRustInlineAsmVerify return type mismatch) - #55142 (miri: layout should not affect CTFE checks (outside of validation)) - #55151 (Cleanup nll) - #55161 ([librustdoc] Disable spellcheck for search field)

rust-highfive assigned RalfJung Oct 12, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2018

oli-obk force-pushed the vtables branch from ca8c957 to fce7dc2Compare October 12, 2018 15:20

bors added a commit that referenced this pull request Oct 12, 2018
Auto merge of#55016- oli-obk:vtables💥_vtables_everywhere, r=<try>
178f3ba
Deduplicate some code and compile-time values around vtables r? @RalfJung

oli-obk force-pushed the vtables branch from fce7dc2 to faa688bCompare October 13, 2018 07:40

bors added a commit that referenced this pull request Oct 13, 2018
Auto merge of#55016- oli-obk:vtables💥_vtables_everywhere, r=<try>
b4b95a3
Deduplicate some code and compile-time values around vtables r? @RalfJung

RalfJung reviewed Oct 13, 2018
View reviewed changes

Comment threadsrc/librustc_mir/interpret/traits.rs

oli-obk added 4 commits October 15, 2018 20:54

Deduplicate vtables within a single evaluation
80feed3

Synchronize get_vtable with the codegen_llvm one
8ac7fa4

Uplift some comments to doc comments
ef3ece7

Add comments to remind everyone to keep the get_vtable impls in sync
c95ee9e

oli-obk force-pushed the vtables branch from faa688b to c95ee9eCompare October 15, 2018 19:05

Use forward compatible FxHashMap initialization
b1d3111

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 Oct 16, 2018

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 Oct 16, 2018

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 Oct 16, 2018

kennytm mentioned this pull request Oct 18, 2018
Rollup of 18 pull requests #55171
Merged

kennytm added a commit to kennytm/rust that referenced this pull request Oct 18, 2018
Rollup merge ofrust-lang#55016- oli-obk:vtables💥_vtables_everywhere…
9ee49bb
…, r=RalfJung Deduplicate some code and compile-time values around vtables r? @RalfJung

bors merged commit b1d3111 into rust-lang:masterOct 18, 2018

oli-obk deleted the vtables💥_vtables_everywhere branch June 15, 2020 15:29

Uh oh!

Uh oh!

Hidden character warning

Conversation

oli-obk commented Oct 12, 2018

Uh oh!

oli-obk commented Oct 12, 2018

Uh oh!

bors commented Oct 12, 2018

Uh oh!

rust-highfive commented Oct 12, 2018

Uh oh!

oli-obk commented Oct 12, 2018

Uh oh!

bors commented Oct 12, 2018

Uh oh!

bors commented Oct 12, 2018

Uh oh!

oli-obk commented Oct 12, 2018

Uh oh!

rust-timer commented Oct 12, 2018

Uh oh!

Mark-Simulacrum commented Oct 12, 2018

Uh oh!

rust-timer commented Oct 12, 2018

Uh oh!

Mark-Simulacrum commented Oct 12, 2018

Uh oh!

rust-timer commented Oct 12, 2018

Uh oh!

rust-timer commented Oct 12, 2018

Uh oh!

oli-obk commented Oct 13, 2018

Uh oh!

oli-obk commented Oct 13, 2018

Uh oh!

bors commented Oct 13, 2018

Uh oh!

RalfJungOct 13, 2018

Choose a reason for hiding this comment

Uh oh!

RalfJungOct 13, 2018

Choose a reason for hiding this comment

Uh oh!

oli-obkOct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RalfJung commented Oct 13, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Oct 13, 2018

Uh oh!

RalfJung commented Oct 13, 2018

Uh oh!

rust-timer commented Oct 13, 2018

Uh oh!

rust-timer commented Oct 13, 2018

Uh oh!

oli-obk commented Oct 13, 2018

Uh oh!

RalfJung commented Oct 16, 2018

Uh oh!

bors commented Oct 16, 2018

Uh oh!

bors commented Oct 16, 2018

Uh oh!

bors commented Oct 16, 2018

Uh oh!

rust-highfive commented Oct 16, 2018

Uh oh!

RalfJung commented Oct 16, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

RalfJung commented Oct 13, 2018•
edited
Loading

RalfJung commented Oct 16, 2018•
edited
Loading