Self managing allocations by oli-obk · Pull Request #55293 · rust-lang/rust

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.

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

+ ) -> 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.

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.

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

Comment threadsrc/librustc/mir/interpret/mod.rs Outdated

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

Comment threadsrc/librustc/mir/interpret/allocation.rs Outdated

oli-obk force-pushed the self_managing_allocations branch from fba8adc to a8b9eecCompare 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 0aa2aa8Compare 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!

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!

RalfJungOct 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