Stream: t-compiler/wg-nll

Topic: issue-52713-escaping-into-static


lqd (Jul 26 2018 at 13:51, on Zulip):

hello @nikomatsakis sorry to bother you, I was looking at this and wanted to check: to check which variables refer to which, I should do a mir walk, looking for statements / assign ?

nikomatsakis (Jul 26 2018 at 14:44, on Zulip):

@lqd yes, I think you could use a MIR visitor

lqd (Jul 26 2018 at 14:47, on Zulip):

do you by any chance remember an area of the compiler where we look for similar kinds of information, so that I don't bother you for trivial questions ?

lqd (Jul 26 2018 at 14:48, on Zulip):

(mostly to see the kinds of visit callbacks I'd need to look for, but I guess, assignments, statements, maybe locals and places, etc)

nikomatsakis (Jul 26 2018 at 14:50, on Zulip):

hmm, I can't think of anything quite comparable -- but I imagined that you would probably visit assignments

nikomatsakis (Jul 26 2018 at 14:50, on Zulip):

and then match on the rvalue

lqd (Jul 26 2018 at 14:51, on Zulip):

alright thank you, it'll be a good start already

lqd (Jul 27 2018 at 17:41, on Zulip):

alrighty, here's a quick WIP try/hack? at finding which locals flow into statics . Is this direction how you envisioned it or am I off base ?

lqd (Jul 27 2018 at 17:44, on Zulip):

and if that's approximately ok, how would I then go about forcing the escaping locals' regions to outlive 'static ?

lqd (Jul 27 2018 at 17:51, on Zulip):

(as another data point, completely disabling liveness in our html5ever big static benchmark case ended up improving around 25% so it's likely the upper bound of this future fix, and it's also likely that time would now go into dataflow/moveck like we recently talked about wrt to tuple-stress)

nikomatsakis (Jul 28 2018 at 07:00, on Zulip):

@lqd I am off to the beach just now ;) but from a 22-second glance looks right

nikomatsakis (Jul 28 2018 at 07:00, on Zulip):

will try to look more closely later

lqd (Jul 28 2018 at 07:02, on Zulip):

thanks :) enjoy the beach :beach_with_umbrella:

nikomatsakis (Jul 28 2018 at 13:11, on Zulip):

@lqd left a few comments now

lqd (Jul 28 2018 at 15:52, on Zulip):

thanks for the comments, I'll try to do as much as I can before leaving :)

lqd (Jul 28 2018 at 15:59, on Zulip):

I had started previously in the way the comments say, and testing on the "mini html5ever" version it seemed like it would stop a bit before getting to the big inner arrays. The MIR was indeed very similar to the const X: u32 = add(&22, &44); case except the references were to these big arrays instead, and my thoughts were that most of the liveness time was spent computing it for the array's elements, which I assumed I wouldn't prevent with this initial way. I'll expand checking to projections as well, and check the rvalues conservatively :)

lqd (Jul 28 2018 at 16:03, on Zulip):

(maybe the bigger wins / biggest time spent may be coming from the behaviour that is shared with tuple-stress in this benchmark :thinking_face: )

nikomatsakis (Jul 29 2018 at 03:42, on Zulip):

@lqd when do you leave btw?

nikomatsakis (Jul 29 2018 at 03:43, on Zulip):

are you saying there were fn calls in the MIR?

nikomatsakis (Jul 29 2018 at 03:44, on Zulip):

that said, I agree this will not be enough to "solve" html5ever...

nikomatsakis (Jul 29 2018 at 03:44, on Zulip):

I'm looking at the MIR for the smaller variant now (playground)

nikomatsakis (Jul 29 2018 at 03:46, on Zulip):

it seems like we need aggregates, ref expressions (&foo), unsizing casts, moves...

nikomatsakis (Jul 29 2018 at 03:46, on Zulip):

and that's it?

lqd (Jul 29 2018 at 08:10, on Zulip):

leaving tomorrow so timing is tight :)

lqd (Jul 29 2018 at 08:24, on Zulip):

oh no it wasn’t the function calls, just refs to inputs and I likely misunderstood these shouldn’t escape — but It seems the eg “_1 = const 22u32” shouldn’t escape but “_2 = &1” should, and how I did it absolutely accepts the inputs when it shouldn’t

lqd (Jul 29 2018 at 08:41, on Zulip):

specifically it wasn't immediately clear to me in the comment's example playground how the MIR for the static X was to be handled differently from the static NAMED_ENTITIES

lqd (Jul 29 2018 at 09:13, on Zulip):

ok now I think I understand the comment sorry :/ by looking at the rvalue directly I wouldn't mark locals which are visited as byproducts of eg the const fn call, which I surely get rn. I was looking at the MIR for the inputs while the problem would appear in the output...

nikomatsakis (Jul 29 2018 at 13:18, on Zulip):

@lqd if you wind up leaving we can always "hand off" the branch

lqd (Jul 29 2018 at 15:30, on Zulip):

yeah I hope I can finish this first step of computing the escaping locals during the night, so it’s at least done before handing it off to incorporate into liveness

lqd (Jul 29 2018 at 22:43, on Zulip):

pushed this commit which should be more in-line with how we wanted to do the analysis; in my tests it seemed to correctly analyze the html5ever static, and detect all but the const fn call in our counter-example. There are things I lacked time to do/wasn't sure exactly under which form we wanted them:
- this needs tests
- maybe debug logs of the escaping locals gathering
- the big one I expected would not have enough time to finish: it still doesn't do anything with the escaping locals to filter them from liveness.
I can still do some fixups in the morning btw. Still, sorry I couldn't push this over the finish line :/

lqd (Jul 30 2018 at 08:05, on Zulip):

but otherwise, I think it should be in good enough shape to hand off

nikomatsakis (Jul 30 2018 at 11:35, on Zulip):

@lqd awesome, I have some plane rides coming up, maybe I'll pick it up

lqd (Jul 30 2018 at 14:10, on Zulip):

@nikomatsakis safe travels niko :)

nikomatsakis (Jul 30 2018 at 14:39, on Zulip):

@lqd what branch? (I was going to pull it locally and get the build process started)

lqd (Jul 30 2018 at 14:40, on Zulip):

@nikomatsakis https://github.com/lqd/rust/tree/the-MIRnistry-of-silly-walks

lqd (Jul 30 2018 at 14:41, on Zulip):

(the fork is a bit behind, like 6 days)

lqd (Jul 30 2018 at 14:42, on Zulip):

(@David Wood whistling + you're a brit right ? :D)

nikomatsakis (Jul 30 2018 at 14:44, on Zulip):

building...

davidtwco (Jul 30 2018 at 14:44, on Zulip):

(@lqd yeah, Scottish)

Jake Goulding (Jul 30 2018 at 15:29, on Zulip):

@lqd this is not a unique cultural reference ;-) I'd say many (most?) nerds would get it.

Jake Goulding (Jul 30 2018 at 15:30, on Zulip):

I love making fun branch names. They are usually non-obvious

lqd (Jul 30 2018 at 15:35, on Zulip):

@Jake Goulding :)

mikhail-m1 (Jul 31 2018 at 21:30, on Zulip):

hi, I can continue work on this issue if nobody on it rightnow

nikomatsakis (Aug 02 2018 at 11:09, on Zulip):

I'm still not able to really be online, but I did poke at this branch a bit actually @mikhail-m1 and I think I kind of got it working

nikomatsakis (Aug 06 2018 at 10:11, on Zulip):

argh I just realized a huge bug in this implementation -- the idea is sound, but overly aggressive

nikomatsakis (Aug 06 2018 at 12:09, on Zulip):

opened https://github.com/rust-lang/rust/pull/53109 but I'll post some notes for how to do this properly

nikomatsakis (Aug 06 2018 at 12:57, on Zulip):

@lqd or @mikhail-m1 still interested?

nikomatsakis (Aug 06 2018 at 12:59, on Zulip):

I left a comment here https://github.com/rust-lang/rust/issues/52713#issuecomment-410699523

mikhail-m1 (Aug 06 2018 at 12:59, on Zulip):

Hi @nikomatsakis , I don't mind to continue work on it, if @lqd don't have time...

nikomatsakis (Aug 06 2018 at 13:00, on Zulip):

I predict I will be pretty caught up :)

nikomatsakis (Aug 06 2018 at 13:01, on Zulip):

take a look at my comment — I think it's not a major change that is needed

nikomatsakis (Aug 06 2018 at 14:35, on Zulip):

@mikhail-m1 shall I assign to you then?

nikomatsakis (Aug 06 2018 at 14:39, on Zulip):

I'm leaning towards this alternative approach now

nikomatsakis (Aug 06 2018 at 14:39, on Zulip):

I can also take a stab (as you prefer)

nikomatsakis (Aug 06 2018 at 14:39, on Zulip):

I had some other ideas for perf improvements I need to try and write up

lqd (Aug 06 2018 at 14:54, on Zulip):

@mikhail-m1 I'm still away until next week, feel free to take it :)

mikhail-m1 (Aug 06 2018 at 15:46, on Zulip):

@nikomatsakis yes, you can assign it to me, but it will take some time to understand how to implement the new approach

nikomatsakis (Aug 06 2018 at 16:06, on Zulip):

@mikhail-m1 if you can give me some high-level tips of what might need explanation, I can try to add more details

mikhail-m1 (Aug 06 2018 at 16:07, on Zulip):

@nikomatsakis yes, it will be helpful

nikomatsakis (Aug 06 2018 at 16:08, on Zulip):

ok let me try to leave comments that are a bit more in depth. As far as delays, seems ok, whenever possible I always prefer to mentor! =) Last week I just happened to be sort of unavailable ...

nikomatsakis (Aug 07 2018 at 13:36, on Zulip):

@mikhail-m1 i'm going to leave a few notes now

Last update: Nov 21 2019 at 14:10UTC