Refactor and improve: Arena, TypedArena by pczarn · Pull Request #27807 · rust-lang/rust

pczarn · 2015-08-13T16:00:37Z

Fixes#18037 "TypedArena cannot handle zero-sized types".
Closes#17931 "improve chunk allocation scheme used by Arena / TypedArena".
Closes#22847 "TypedArena should implement Send". - N.B. Arena cannot implement Send, since it may contain non-Send values.
Closes#18471 "Arena::alloc_copy_inner (at least) should be renamed and made public." - Added Arena::alloc_bytes.
Closes#18261 "support clearing TypedArena with the chunks preserved". - Only the largest chunk is preserved.

rust-highfive · 2015-08-13T16:00:50Z

r? @huonw

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

huonw · 2015-08-13T16:07:02Z

cc @gankro particularly for the RawVec changes.

huonw · 2015-08-13T16:15:12Z

I think this possibly needs to be RawVec<UnsafeCell<u8>> since there's effectively a transmute from &Chunk -> &mut [u8] in the various allocation methods; however I'm not totally sure it's needed because the Arena ensures non-aliased-ness of everything it hands out.

I think UnsafeCell can be left out as long as we only use non-aliased or raw pointers. UnsafeCell's docs say:
In general, transmuting an &T type into an &mut T is considered undefined behavior.

huonw · 2015-08-13T16:25:57Z

I idly wonder if this will slow the typedarena down noticably: it is adding an extra allocation (the Vec of chunks) and more indirection to get to the elements of the last chunk (have to index the vector and then dereference, instead of just a straight dereference).

That said I imagine it's not noticable.

bors · 2015-09-08T16:02:01Z

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

Gankra · 2015-09-08T18:52:45Z

explicit return not needed

Gankra · 2015-09-08T19:01:31Z

\o/ for RawVec helping out even more!

All the RawVec-internal stuff seems good, but I don't have the bandwidth to review the Arena stuff, sadly :(

pczarn · 2015-09-08T19:36:07Z

Thanks. Changes are pushed.

bors · 2015-09-11T23:06:50Z

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

bors · 2015-09-25T21:57:48Z

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

bors · 2015-10-14T04:24:30Z

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

pczarn · 2015-10-26T13:31:07Z

Updated

Gankra · 2015-10-26T21:31:29Z

rustc: x86_64-unknown-linux-gnu/stage2/test/arenatest-x86_64-unknown-linux-gnu
src/libarena/lib.rs:610:29: 610:59 error: box expression syntax is experimental; you can call `Box::new` instead. (see issue #27779)
src/libarena/lib.rs:610 let _: Box<_> = box Point { x: 1, y: 2, z: 3 };
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libarena/lib.rs:610:29: 610:59 help: add #![feature(box_syntax)] ```to the crate attributes to enable
error: aborting due to previous error

pczarn · 2015-11-18T12:40:12Z

Corrected

bors · 2015-11-25T22:32:48Z

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

shepmaster · 2016-01-06T17:01:47Z

@pczarn great, I appreciate that! Although I'm totally cool with it having well-tested unsafe code ;-) I could always do some head-to-head perf testing between the two implementations to see if it's useful.

pczarn · 2016-01-06T17:08:23Z

@bluss: It's deprecated. 🎉

aturon · 2016-01-06T17:18:20Z

Nominating for libs team discussion re: deprecation.

aturon · 2016-01-08T20:10:34Z

Libs team consensus: the deprecation here isn't really relevant, as the crate is already gated by rustc_private.

bluss · 2016-01-08T20:23:26Z

Thank you @aturon. Let's go again!

@bors r+

bors · 2016-01-08T20:23:28Z

📌 Commit e2ccc4f has been approved by bluss

bors · 2016-01-08T23:51:13Z

⌛ Testing commit e2ccc4f with merge 927d22d...

bors · 2016-01-08T23:53:44Z

💔 Test failed - auto-linux-64-nopt-t

bluss · 2016-01-10T17:39:46Z

@bors retry

bors · 2016-01-11T01:58:44Z

⌛ Testing commit e2ccc4f with merge d3e5b72...

bors · 2016-01-11T02:07:43Z

💔 Test failed - auto-mac-64-opt

alexcrichton · 2016-01-11T04:11:33Z

@bors: retry

On Mon, Jan 11, 2016 at 3:07 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-mac-64-opt
http://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/7619
—
Reply to this email directly or view it on GitHub
#27807 (comment).

bors · 2016-01-11T04:12:50Z

⌛ Testing commit e2ccc4f with merge 02dc12b...

bors · 2016-01-11T06:40:57Z

💔 Test failed - auto-linux-64-nopt-t

alexcrichton · 2016-01-11T07:26:15Z

@bors: retry

On Mon, Jan 11, 2016 at 7:41 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-64-nopt-t
http://buildbot.rust-lang.org/builders/auto-linux-64-nopt-t/builds/7595
—
Reply to this email directly or view it on GitHub
#27807 (comment).

bors · 2016-01-11T08:32:47Z

⌛ Testing commit e2ccc4f with merge dfaddb7...

Fixes#18037 "TypedArena cannot handle zero-sized types". Closes#17931 "improve chunk allocation scheme used by Arena / TypedArena". Closes#22847 "TypedArena should implement Send". - N.B. Arena cannot implement Send, since it may contain non-Send values. Closes#18471 "`Arena::alloc_copy_inner` (at least) should be renamed and made public." - Added `Arena::alloc_bytes`. Closes#18261 "support clearing TypedArena with the chunks preserved". - Only the largest chunk is preserved.

bors · 2016-01-11T10:19:42Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

SimonSapin · 2016-01-12T08:29:14Z

This comment should be removed, right?

Gankra · 2016-01-12T20:01:40Z

🎊

Thanks so much for persevering on this, @pczarn!

jethrogb · 2016-05-08T18:29:15Z

+#[unstable(feature = "rustc_private",
+ reason = "Private to rustc", issue = "0")]
+#[rustc_deprecated(since = "1.6.0-dev", reason =
+"The reflection-based arena is superseded by the any-arena crate")]


Where is this crate?

@jethrogb I published it now: https://crates.io/crates/any-arena

rust-highfive assigned huonw Aug 13, 2015

pczarn force-pushed the arena-internals branch from f515cd8 to b7410d6Compare August 13, 2015 16:11

huonw reviewed Aug 13, 2015
View reviewed changes

pczarn force-pushed the arena-internals branch 5 times, most recently from ef5f6e9 to 8a08f17Compare August 14, 2015 17:49

Gankra reviewed Sep 8, 2015
View reviewed changes

Comment threadsrc/liballoc/raw_vec.rs Outdated
GankraSep 8, 2015
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explicit return not needed

pczarn force-pushed the arena-internals branch from 8a08f17 to 97a168bCompare September 8, 2015 19:32

pczarn force-pushed the arena-internals branch from 97a168b to afa449cCompare September 16, 2015 07:44

pczarn force-pushed the arena-internals branch from afa449c to 92df7ffCompare September 27, 2015 17:36

pczarn force-pushed the arena-internals branch from 92df7ff to e682b3cCompare October 26, 2015 13:31

pczarn force-pushed the arena-internals branch from e682b3c to c765391Compare November 18, 2015 12:19

Deprecate reflection-based Arena
e2ccc4f

pczarn force-pushed the arena-internals branch from 270ed4c to e2ccc4fCompare January 6, 2016 17:07

aturon added I-nominated T-libs-api [DEPRECATED; DO NOT USE] and removed I-nominated labels Jan 6, 2016

bors merged commit e2ccc4f into rust-lang:masterJan 11, 2016

SimonSapin reviewed Jan 12, 2016
View reviewed changes

Comment threadsrc/libarena/lib.rs
SimonSapinJan 12, 2016
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment should be removed, right?

jethrogb reviewed May 8, 2016
View reviewed changes

Uh oh!

Uh oh!

Conversation

pczarn commented Aug 13, 2015

Uh oh!

rust-highfive commented Aug 13, 2015

Uh oh!

huonw commented Aug 13, 2015

Uh oh!

huonwAug 13, 2015

Choose a reason for hiding this comment

Uh oh!

pczarnDec 21, 2015

Choose a reason for hiding this comment

Uh oh!

huonw commented Aug 13, 2015

Uh oh!

bors commented Sep 8, 2015

Uh oh!

GankraSep 8, 2015

Choose a reason for hiding this comment

Uh oh!

Gankra commented Sep 8, 2015

Uh oh!

pczarn commented Sep 8, 2015

Uh oh!

bors commented Sep 11, 2015

Uh oh!

bors commented Sep 25, 2015

Uh oh!

bors commented Oct 14, 2015

Uh oh!

pczarn commented Oct 26, 2015

Uh oh!

Gankra commented Oct 26, 2015

Uh oh!

pczarn commented Nov 18, 2015

Uh oh!

bors commented Nov 25, 2015

Uh oh!

shepmaster commented Jan 6, 2016

Uh oh!

pczarn commented Jan 6, 2016

Uh oh!

aturon commented Jan 6, 2016

Uh oh!

aturon commented Jan 8, 2016

Uh oh!

bluss commented Jan 8, 2016

Uh oh!

bors commented Jan 8, 2016

Uh oh!

bors commented Jan 8, 2016

Uh oh!

bors commented Jan 8, 2016

Uh oh!

bluss commented Jan 10, 2016

Uh oh!

bors commented Jan 11, 2016

Uh oh!

bors commented Jan 11, 2016

Uh oh!

alexcrichton commented Jan 11, 2016

Uh oh!

bors commented Jan 11, 2016

Uh oh!

bors commented Jan 11, 2016

Uh oh!

alexcrichton commented Jan 11, 2016

Uh oh!

bors commented Jan 11, 2016

Uh oh!

bors commented Jan 11, 2016

Uh oh!

SimonSapinJan 12, 2016

Choose a reason for hiding this comment

Uh oh!

Gankra commented Jan 12, 2016

Uh oh!

jethrogbMay 8, 2016