Stream: t-compiler/wg-mir-opt

Topic: Place size


oli (Jul 22 2019 at 18:28, on Zulip):

Places have increased in size since we moved to having a base field and a projection field. If we move further to &[PlaceElem] for the projection field, we don't gain any bytes, because a fat pointer needs 16 bytes. I remember some discussion about reducing Place back to a total of 16 bytes or so, because it's used so much in MIR

oli (Jul 22 2019 at 18:30, on Zulip):

What we could also do is intern all Places, so we'd have &'tcx Place<'tcx> everywhere and the body is

struct Place<'tcx> {
    base: PlaceBase<'tcx>, // and remove all `Box`es from `PlaceBase`
    projections: SmallVec<Projection<'tcx>>,
}
oli (Jul 22 2019 at 18:32, on Zulip):

cc @WG-mir-opt opinions? The current "plan" without considering the above is either

struct Place<'tcx> {
    base: PlaceBase<'tcx>, // 16 bytes
    projection: &'tcx [Projection<'tcx>], // 16 bytes
}

or

struct Place<'tcx> {
    base: PlaceBase<'tcx>, // 16 bytes (or reduce to 8 bytes by putting it behind a `Box` or so)
    projection: &'tcx Slice<Projection<'tcx>>, // 8 bytes
}
Wesley Wiser (Jul 22 2019 at 19:20, on Zulip):

I don't think I have a strong opinion either way. I'd love to see some empirical data demonstrating better performance/lower memory usage before we make the code uglier. So I'd vote for whichever one leads to nicer code and then we can tune it for performance later.

oli (Jul 25 2019 at 15:18, on Zulip):

cc @eddyb opinions on how to get Place's size down?

oli (Jul 25 2019 at 15:19, on Zulip):

interning seems practical for all the Places that just reference a local (especially the ones with small ids, which there are a lot of)

Wesley Wiser (Jul 25 2019 at 15:35, on Zulip):

Would it make sense to have a intern cache per mir::Body? Naively, I assume there's a great deal of duplication of Places within a given body unless it's just like a 2 or 3 line function.

oli (Jul 25 2019 at 15:36, on Zulip):

like "there are a maximum of 2^32 distinguishable Places that may show up in a mir::Body"?

oli (Jul 25 2019 at 15:36, on Zulip):

and just use such an index into that cache?

Wesley Wiser (Jul 25 2019 at 15:36, on Zulip):

Yeah, exactly

oli (Jul 25 2019 at 15:37, on Zulip):

hmm... I like it, this has potential for some fun optimizations

oli (Jul 25 2019 at 15:37, on Zulip):

like the indices below 256 are 1:1 mappings for local ids :D

oli (Jul 25 2019 at 15:38, on Zulip):

hmm... wait, this doesn't help. The array element size would be the full 32 bytes

oli (Jul 25 2019 at 15:38, on Zulip):

we'd need an indirection

oli (Jul 25 2019 at 15:38, on Zulip):

or sth

Wesley Wiser (Jul 25 2019 at 15:39, on Zulip):

You mean 32 bits right?

oli (Jul 25 2019 at 15:39, on Zulip):

Place is 32 bytes

Wesley Wiser (Jul 25 2019 at 15:40, on Zulip):

Ah yes

oli (Jul 25 2019 at 15:40, on Zulip):

so the cache would have to have a 32 byte entry per Place that it stores

oli (Jul 25 2019 at 15:40, on Zulip):

a naive implementation would not gain us much

Wesley Wiser (Jul 25 2019 at 15:40, on Zulip):

It wouldn't shrink Place any but it would reduce the number of instances of Place

oli (Jul 25 2019 at 15:40, on Zulip):

which would shrink Statement... ok

Wesley Wiser (Jul 25 2019 at 15:40, on Zulip):

And any place (hah) which currently has an inline place would shrink in size

Wesley Wiser (Jul 25 2019 at 15:40, on Zulip):

Yeah

oli (Jul 25 2019 at 15:40, on Zulip):

I keep forgetting the "fallout" that shrinking the type has

oli (Jul 25 2019 at 15:41, on Zulip):

we can start by just making a few relevant inline Places Box<Place>

oli (Jul 25 2019 at 15:41, on Zulip):

I'm not sure how fast we can get such an indexing scheme done

oli (Jul 25 2019 at 15:42, on Zulip):

we talked about it at the all hands for several other things, too iirc

Wesley Wiser (Jul 25 2019 at 15:43, on Zulip):

We should keep track of this stuff somewhere. Fairly regularly I go looking for stuff to work on and can't find any of that stuff.

oli (Jul 25 2019 at 15:43, on Zulip):

yea :confused: I know what you mean

oli (Jul 25 2019 at 15:43, on Zulip):

we can put it into the wg-mir-opt pages in the compiler-team repo

Wesley Wiser (Jul 25 2019 at 15:43, on Zulip):

So then I have to bug either you or @eddyb :slight_smile:

oli (Jul 25 2019 at 15:43, on Zulip):

I think we already have a long-term TODO list there

Santiago Pastorino (Jul 25 2019 at 16:02, on Zulip):

where is there a long list of things to do?

Santiago Pastorino (Jul 25 2019 at 16:02, on Zulip):

sorry didn't get that quite well

Santiago Pastorino (Jul 25 2019 at 16:02, on Zulip):

I still have a bunch of stuff to do with all this Place 2.0 stuff, not looking for more for now but interested in knowing anyway :)

Santiago Pastorino (Jul 25 2019 at 16:04, on Zulip):

@Wesley Wiser after I finish migrating Place to a slice if you're interested we can talk about future steps and maybe even share work or pair program on stuff

Wesley Wiser (Jul 25 2019 at 16:04, on Zulip):

@Santiago Pastorino Sounds good!

Last update: Nov 17 2019 at 07:30UTC