Replace syntax::opt_vec with syntax::owned_slice #13016
Conversation
|
(I'll rebase later.) @cmr a mem-bench of this would be cool too (after I rebase, of course). |
|
Tiny win. |
|
Could we just use |
|
I was thinking that having a type exposing only the operations that |
|
I'm not a big fan of having large amounts of transitionary code lying around. This doesn't actually give us the benefits of a 2-word vector as opposed to a 3-word vector, so why not just deal with the migration later? Migrating libsyntax/librustc to using |
|
I solved my segfaulting issues (#13032 :/ ) and have just pushed a version that is only 2 words. It uses a small(-ish) amount of unsafe code, but I've tried to keep it well contained, and comment the non-obvious things. In any case, there is not much of this transitionary code at all: the diff stat is negative. If this is approved I'll squash (and rebase, etc).
I don't understand what you mean by the standard library "needing much Also, as @cmr's mem-bench demonstrated, even shaving one word off this type apparently saves ~100 MB (and with this extra word removed that saving will presumably double), which I think is a reasonably large concern, given how memory hungry rustc is and the fact this is a nearly -10% reduction. (@cmr another mem-bench?) |
|
|
|
ignore everything after ~50s, accidentally had an |
This is the first step to replacing OptVec with a new representation: remove all mutability. Any mutations have to go via `Vec` and then make to `OptVec`. Many of the uses of OptVec are unnecessary now that Vec has no-alloc emptiness (and have been converted to Vec): the only ones that really need it are the AST and sty's (and so on) where there are a *lot* of instances of them, and they're (mostly) immutable.
| if self.data.is_null() { return } | ||
|
|
||
| // zero self | ||
| let v = mem::replace(self, OwnedSlice::empty()); |
There was a problem hiding this comment.
You don't need to do this, it happens automatically. The null check is all you have to do.
There was a problem hiding this comment.
I know it happens automatically in general, but this will make it still correct when we stop zeroing everything.
There was a problem hiding this comment.
Zeroing in the destructor won't necessarily be seen from locations it was moved from anyway. If we stop zeroing, it will mean Rust needs to statically prevent a call if a type was ever possibly moved from.
There was a problem hiding this comment.
Oh, wait; I was wrong. The replace here is to be able to call the by-value .into_vec function.
There was a problem hiding this comment.
(Updated the comment to remove the reference to zeroing)
This is a stand-in until we have a saner `~[T]` type (i.e. a proper owned slice). It's a library version of what `~[T]` will be, i.e. an owned pointer and a length.
syntax::opt_vec is now entirely unused, and so can go.
Replace syntax::opt_vec with syntax::owned_slice The `owned_slice::OwnedSlice` is `(*T, uint)` (i.e. a direct equivalent to DSTs `~[T]`). This shaves two words off the old OptVec type; and also makes substituting in other implementations easy, by removing all the mutation methods. (And also everything that's very rarely/never used.)
…kril Move VSCode diagnostics workaroudn into client code
Replace syntax::opt_vec with syntax::owned_slice
The
owned_slice::OwnedSliceis(*T, uint)(i.e. a direct equivalent to DSTs~[T]).This shaves two words off the old OptVec type; and also makes substituting in other implementations easy, by removing all the mutation methods. (And also everything that's very rarely/never used.)