Self managing allocations#55293

oli-obk · 2018-10-23T19:16:36Z

No description provided.

rust-highfive · 2018-10-23T19:16:40Z

r? @zackmdavis

(rust_highfive has picked a reviewer for you, use r? to override)

rust-highfive · 2018-10-23T19:16:40Z

⚠️ Warning ⚠️

These commits modify submodules.

oli-obk · 2018-10-23T19:17:34Z

r? @RalfJung

pulled out of #55260 to keep that PR simpler

rust-highfive · 2018-10-23T20:51:38Z

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:01] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:01] tidy error: /checkout/src/librustc/mir/interpret/allocation.rs: missing trailing newline
[00:04:01] tidy error: /checkout/src/librustc/mir/interpret/mod.rs: missing trailing newline
[00:04:02] some tidy checks failed
[00:04:02] 
[00:04:02] 
[00:04:02] 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:04:02] 
[00:04:02] 
[00:04:02] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:02] Build completed unsuccessfully in 0:00:45
[00:04:02] Build completed unsuccessfully in 0:00:45
[00:04:02] Makefile:79: recipe for target 'tidy' failed
[00:04:02] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:2f10acce
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:15d7eb90:start=1540322595447864581,finish=1540322595451974711,duration=4110130
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:067f2660
$ 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:05b187c1
travis_time:start:05b187c1
$ 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:0bd56084
$ 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)

RalfJung

I expect that the miri submodule update will get messy because we don't know in which order our PRs will land. As long as we have so many miri-breaking PRs in flight, I think it is better to do the submodule updates separately.

src/librustc/mir/interpret/mod.rs

src/librustc/mir/interpret/allocation.rs

RalfJung · 2018-10-25T08:11:38Z

src/librustc/mir/interpret/allocation.rs

+    ) -> EvalResult<'tcx> {
+        // Empty accesses don't need to be valid pointers, but they should still be non-NULL
+        let align = Align::from_bytes(1, 1).unwrap();
+        if size.bytes() == 0 {


Now we check for emptiness twice. I think it should be fine to say that all the methods on an Allocation require an in-bounds ptr, even for size 0 -- then we'd have no special handling for size 0 here at all. All the handling would be in memory.rs where we need it anyway to handle integer pointers.

src/librustc/mir/interpret/allocation.rs

src/librustc/mir/interpret/mod.rs

oli-obk · 2018-10-25T15:30:27Z

before removing the size==0 checks I was seeing nice perf improvements. Lets see if this was me failing to run perfruns properly.

@bors try

bors · 2018-10-25T15:30:40Z

⌛ Trying commit 07e8233 with merge 80add95...

Self managing allocations

oli-obk · 2018-10-25T15:31:05Z

@rust-timer build 80add95

rust-timer · 2018-10-25T15:31:07Z

Success: Queued 80add95 with parent 3476ac0, comparison URL.

src/librustc/mir/interpret/allocation.rs

RalfJung · 2018-10-25T16:13:25Z

Do we also want to find a new home for Pointer? mod.rs is inadequate, I think. I just cannot decide between value.rs and alloc.rs.

oli-obk · 2018-10-25T16:16:27Z

Do we also want to find a new home for Pointer? mod.rs is inadequate, I think. I just cannot decide between value.rs and alloc.rs.

pointer.rs ?

RalfJung · 2018-10-25T16:17:34Z

What would be in there? Pointer and the pointer arithmetic stuff?

Sounds reasonable.

RalfJung · 2018-10-25T16:37:30Z

r=me if you remove the miri update and perf looks good

bors · 2018-10-25T17:57:54Z

☔ The latest upstream changes (presumably #55347) made this pull request unmergeable. Please resolve the merge conflicts.

rust-timer · 2018-10-25T20:11:40Z

Finished benchmarking try commit 80add95

oli-obk · 2018-10-25T20:24:07Z

Oh yes! improvements up to 6% across the board. Not fetching those allocations 4x for 4 checks does have an effect.

oli-obk · 2018-10-25T20:28:15Z

@bors r=RalfJung

bors · 2018-10-25T20:28:16Z

📌 Commit a8b9eec4d2ca87ea45edb4f53682394eca7c893d has been approved by RalfJung

…rough `.get(alloc_id)`

This reverts commit d50e401.

oli-obk · 2018-11-02T10:17:11Z

I am unable to reproduce on x86 linux. I also don't see how these changes could cause changes in the emitted machine code. It's just a refactoring moving functions between modules.

I could do this PR in steps by splitting it into smaller PRs

RalfJung · 2018-11-02T10:31:57Z

I don't have any better idea either. Maybe reduce this one to the most bitrotty "moving stuff around" part (Allocation, ScalarMaybeUndef, Pointer), and then have a separate PR for actually adding more methods to Allocation?

bors · 2018-11-02T15:51:20Z

☔ The latest upstream changes (presumably #55316) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2018-11-08T13:50:08Z

blocked on #55674

@RalfJung

Miri engine refactoring r? @RalfJung split out the "just moves stuff around" part of #55293

oli-obk · 2018-11-13T09:20:57Z

closing in favour of #55915

@RalfJung

Miri engine refactoring next small step of #55293 r? @RalfJung

rust-highfive assigned zackmdavis Oct 23, 2018

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

oli-obk mentioned this pull request Oct 23, 2018

Update to allocation method PR rust-lang/miri#493

Closed

rust-highfive assigned RalfJung and unassigned zackmdavis Oct 23, 2018

oli-obk mentioned this pull request Oct 24, 2018

Get rid of the Scalar and ScalarPair variants of ConstValue... #55260

Closed

RalfJung requested changes Oct 25, 2018

View reviewed changes

RalfJung reviewed Oct 25, 2018

View reviewed changes

src/librustc/mir/interpret/mod.rs Outdated Show resolved Hide resolved

bors added a commit that referenced this pull request Oct 25, 2018

Auto merge of #55293 - oli-obk:self_managing_allocations, r=<try>

80add95

Self managing allocations

RalfJung reviewed Oct 25, 2018

View reviewed changes

src/librustc/mir/interpret/allocation.rs Outdated Show resolved Hide resolved

oli-obk force-pushed the self_managing_allocations branch from fba8adc to a8b9eec Compare October 25, 2018 20:27

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 25, 2018

oli-obk added 15 commits October 31, 2018 10:38

Move the memory_accessed hook onto the Extra value

3dcd1c6

Fixup all code to work directly on the allocation instead of going th…

f5a5a71

…rough `.get(alloc_id)`

Forward some function calls of Memory to Allocation

9614f34

Update miri submodule

2d1d014

Tidy all the way

f08b117

We actually have the allocation available... it can't be dangling

d593a47

Move UndefMask and Relocations into allocation.rs

8a6f62b

Move ScalarMaybeUndef into value.rs

df3cabb

Move some functions around to keep order in sync with memory.rs

de462ba

Add doc comment to check_bytes

8cc3faa

Remove leftover comment

ed8b17e

Get rid of zst checks, the code works just fine without them

5c9b456

Move Pointer to its own module

9ce0803

Revert "Update miri submodule"

dd194ef

This reverts commit d50e401.

Rebase fallout

0aa2aa8

RalfJung mentioned this pull request Nov 1, 2018

Value visitors for miri #55549

Merged

oli-obk force-pushed the self_managing_allocations branch from 880ba86 to 0aa2aa8 Compare November 2, 2018 10:12

oli-obk mentioned this pull request Nov 4, 2018

Miri engine refactoring #55674

Merged

oli-obk added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Nov 8, 2018

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

Auto merge of #55674 - oli-obk:miri_engine_refactoring, r=RalfJung

a88613c

Miri engine refactoring r? @RalfJung split out the "just moves stuff around" part of #55293

oli-obk mentioned this pull request Nov 13, 2018

Miri engine refactoring #55915

Merged

oli-obk closed this Nov 13, 2018

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

Auto merge of #55915 - oli-obk:miri_engine_refactoring, r=RalfJung

2dd94c1

Miri engine refactoring next small step of #55293 r? @RalfJung

oli-obk deleted the self_managing_allocations branch March 16, 2021 12:13

Uh oh!

Conversation

oli-obk commented Oct 23, 2018

Uh oh!

rust-highfive commented Oct 23, 2018

Uh oh!

rust-highfive commented Oct 23, 2018

Uh oh!

oli-obk commented Oct 23, 2018

Uh oh!

rust-highfive commented Oct 23, 2018

Uh oh!

RalfJung left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

RalfJung Oct 25, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oli-obk commented Oct 25, 2018

Uh oh!

bors commented Oct 25, 2018

Uh oh!

oli-obk commented Oct 25, 2018

Uh oh!

rust-timer commented Oct 25, 2018

Uh oh!

Uh oh!

RalfJung commented Oct 25, 2018

Uh oh!

oli-obk commented Oct 25, 2018

Uh oh!

RalfJung commented Oct 25, 2018

Uh oh!

RalfJung commented Oct 25, 2018

Uh oh!

bors commented Oct 25, 2018

Uh oh!

rust-timer commented Oct 25, 2018

Uh oh!

oli-obk commented Oct 25, 2018

Uh oh!

oli-obk commented Oct 25, 2018

Uh oh!

bors commented Oct 25, 2018

Uh oh!

oli-obk commented Nov 2, 2018

Uh oh!

RalfJung commented Nov 2, 2018

Uh oh!

bors commented Nov 2, 2018

Uh oh!

oli-obk commented Nov 8, 2018

Uh oh!

oli-obk commented Nov 13, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants