Deduplicate some code and compile-time values around vtables #55016

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

src/librustc_mir/interpret/eval_context.rs

@@ -50,6 +51,9 @@ pub struct EvalContext<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'a, 'mir, 'tcx>> {

    /// The virtual call stack.
    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

src/librustc_mir/interpret/eval_context.rs

@@ -207,6 +211,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
            param_env,
            memory: Memory::new(tcx, memory_data),
            stack: Vec::new(),
+            vtables: FxHashMap(),


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

src/librustc_mir/interpret/traits.rs

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 fce7dc2 Compare 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 faa688b Compare 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

src/librustc_mir/interpret/traits.rs Show resolved Hide resolved

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 c95ee9e Compare 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 of rust-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:master Oct 18, 2018

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deduplicate some code and compile-time values around vtables #55016

Deduplicate some code and compile-time values around vtables #55016

oli-obk commented Oct 12, 2018

oli-obk commented Oct 12, 2018

bors commented Oct 12, 2018

rust-highfive commented Oct 12, 2018

oli-obk commented Oct 12, 2018

bors commented Oct 12, 2018

bors commented Oct 12, 2018

oli-obk commented Oct 12, 2018

rust-timer commented Oct 12, 2018

Mark-Simulacrum commented Oct 12, 2018

rust-timer commented Oct 12, 2018

Mark-Simulacrum commented Oct 12, 2018

rust-timer commented Oct 12, 2018

rust-timer commented Oct 12, 2018

oli-obk commented Oct 13, 2018

oli-obk commented Oct 13, 2018

bors commented Oct 13, 2018

RalfJung Oct 13, 2018

RalfJung Oct 13, 2018

oli-obk Oct 16, 2018

RalfJung commented Oct 13, 2018 •

edited

Loading

bors commented Oct 13, 2018

RalfJung commented Oct 13, 2018

rust-timer commented Oct 13, 2018

rust-timer commented Oct 13, 2018

oli-obk commented Oct 13, 2018

RalfJung commented Oct 16, 2018

bors commented Oct 16, 2018

bors commented Oct 16, 2018

bors commented Oct 16, 2018

rust-highfive commented Oct 16, 2018

RalfJung commented Oct 16, 2018 •

edited

Loading

Deduplicate some code and compile-time values around vtables #55016

Deduplicate some code and compile-time values around vtables #55016

Conversation

oli-obk commented Oct 12, 2018

oli-obk commented Oct 12, 2018

bors commented Oct 12, 2018

rust-highfive commented Oct 12, 2018

oli-obk commented Oct 12, 2018

bors commented Oct 12, 2018

bors commented Oct 12, 2018

oli-obk commented Oct 12, 2018

rust-timer commented Oct 12, 2018

Mark-Simulacrum commented Oct 12, 2018

rust-timer commented Oct 12, 2018

Mark-Simulacrum commented Oct 12, 2018

rust-timer commented Oct 12, 2018

rust-timer commented Oct 12, 2018

oli-obk commented Oct 13, 2018

oli-obk commented Oct 13, 2018

bors commented Oct 13, 2018

RalfJung Oct 13, 2018

Choose a reason for hiding this comment

RalfJung Oct 13, 2018

Choose a reason for hiding this comment

oli-obk Oct 16, 2018

Choose a reason for hiding this comment

RalfJung commented Oct 13, 2018 • edited Loading

bors commented Oct 13, 2018

RalfJung commented Oct 13, 2018

rust-timer commented Oct 13, 2018

rust-timer commented Oct 13, 2018

oli-obk commented Oct 13, 2018

RalfJung commented Oct 16, 2018

bors commented Oct 16, 2018

bors commented Oct 16, 2018

bors commented Oct 16, 2018

rust-highfive commented Oct 16, 2018

RalfJung commented Oct 16, 2018 • edited Loading

RalfJung commented Oct 13, 2018 •

edited

Loading

RalfJung commented Oct 16, 2018 •

edited

Loading