Stream: t-compiler/wg-nll

Topic: #58178 performance issue


Eric Huss (Feb 05 2019 at 06:06, on Zulip):

The grammar wg is having an issue with NLL causing extremely long compile times (https://github.com/rust-lang/rust/issues/58178). I was wondering if there were any known issues along these lines? I searched around, but all the performance-related issues seem to be closed.

lqd (Feb 05 2019 at 13:59, on Zulip):

while I was initially surprised a lot of time would be spent in find_outlives_blame_span (as I thought it was related to errors) quick logging seems to indicate this is indeed on the problematic codepath. It's hard to reduce the 2MB test file, and the crate involves proc_macros (i.e dark magic unknown to me to make it work on anything but a 30mins full stage-2 build just to add logging), a 2K line macro, etc, but it seems to be pointing at check_universal_regions + check_universal_region being supralinear - the log shows some of these GLL closures propagating thousands of outlives_requirements (the highest I've seen was 9k of them, which was before stopping rustc trying to check a closure with 70K regions). Hopefully this will allow to reduce the problematic case from 45K lines to one of these :joy:

Matthew Jasper (Feb 05 2019 at 16:51, on Zulip):

@lqd if you have time, can you try replacing the call here: https://github.com/rust-lang/rust/blob/b2c6b8c29f13f8d1f242da89e587960b95337819/src/librustc_mir/borrow_check/nll/region_infer/mod.rs#L1173 with (ConstraintCategory::Boring, mir.span) and seeing how the perf looks (this will regress diagnostics, but there are less computationally expensive ways to determine them.

Matthew Jasper (Feb 05 2019 at 16:52, on Zulip):

That code was know to be possibly expensive, but I didn't think it would be worthwhile until there was a know problem

lqd (Feb 05 2019 at 16:52, on Zulip):

I was casually trying to move it closer to where it's actually needed to see if it made a difference (esp since there's a conditional lower so I thought maybe it'll filter the calls a bit but I'm not sure), and will try stubbing it out as you say afterwards yeah

Matthew Jasper (Feb 05 2019 at 16:53, on Zulip):

There's no small change that's going to fix this. It's too slow of a function to call thousands of times.

lqd (Feb 05 2019 at 16:54, on Zulip):

yeah

lqd (Feb 05 2019 at 16:55, on Zulip):

the 70k regions being checked almost with each other seemed an interesting point to investigate :)

Matthew Jasper (Feb 05 2019 at 16:56, on Zulip):

It's written to only be called on error paths, but this was an exception because "who writes enormous clsoures?" :rolling_on_the_floor_laughing:

lqd (Feb 05 2019 at 16:56, on Zulip):

it's likely (but would have to check) we're doing the same work multiple times here, since we're immediately going to check using the region's SCC

lqd (Feb 05 2019 at 16:57, on Zulip):

this closure's signature in the MIR is 50k characters :joy:

Matthew Jasper (Feb 05 2019 at 16:58, on Zulip):

yes, but hopefully this makes it a much smaller regression

lqd (Feb 05 2019 at 16:59, on Zulip):

I'll go launch the build now

lqd (Feb 05 2019 at 17:40, on Zulip):

@Matthew Jasper yay not calling find_outlives_blame_spanresults in the much smaller difference you expected: around 45s on AST, and 54s with MIR borrowck :)

Matthew Jasper (Feb 05 2019 at 17:47, on Zulip):

That's great thanks! I'll try to work out how to avoid diagnostic regressions over the next week.

lqd (Feb 05 2019 at 17:51, on Zulip):

np!

lqd (Feb 05 2019 at 17:53, on Zulip):

(IIRC less than 10 tests show a diagnostics difference here; and they're not terrible)

lqd (Feb 07 2019 at 11:31, on Zulip):

@Matthew Jasper btw if you need grunt work on this, experiments or whatever (as you're the one actually familiar with all this :) I'll be happy to assist just let me know

Matthew Jasper (Feb 09 2019 at 20:22, on Zulip):

@lqd did you find the source for the closure with 70k regions?

lqd (Feb 11 2019 at 09:29, on Zulip):

@Matthew Jasper unfortunately the source is a mystery wrapped in a riddle wrapped in a macro, so all I have is `DEBUG 2019-02-05T13:28:58Z: rustc_mir::borrow_check::nll::region_infer: check_universal_regions(mir_def_id=DefId(0/1:573 ~ rust_grammar[853b]::parse[0]::{{impl}}[67]::all[0]::{{closure}}[0]), propagated_outlives_requirements=Some([]), defns len=70860, errors_buffer len=0) and the MIR dump

lqd (Feb 11 2019 at 10:08, on Zulip):

ah I see you've already opened a PR in the meantime

qmx (Feb 20 2019 at 16:12, on Zulip):

hey, thanks y'all for looking into this issue :tada:

Last update: Nov 21 2019 at 14:55UTC