Stream: t-compiler/wg-nll

Topic: making-more-plans


nikomatsakis (May 18 2018 at 12:47, on Zulip):

@pnkfelix if you want to chat, I'm around now btw

pnkfelix (May 18 2018 at 12:47, on Zulip):

okay I'll be ready in a minute

pnkfelix (May 18 2018 at 12:54, on Zulip):

okay, few more delays but here now.

pnkfelix (May 18 2018 at 12:55, on Zulip):

we doing this via Zulip, or via some video chat system?

nikomatsakis (May 18 2018 at 12:55, on Zulip):

either way

nikomatsakis (May 18 2018 at 12:55, on Zulip):

might as well chat over zulip I guess

pnkfelix (May 18 2018 at 12:55, on Zulip):

won't be able to include digressions about James Bond over zulip

nikomatsakis (May 18 2018 at 12:55, on Zulip):

oh, you can

pnkfelix (May 18 2018 at 12:55, on Zulip):

/me found a new podcast

nikomatsakis (May 18 2018 at 12:55, on Zulip):

:)

pnkfelix (May 18 2018 at 12:56, on Zulip):

Anyway

nikomatsakis (May 18 2018 at 12:56, on Zulip):

well, so, what I wanted to say is that I was talking to aturon about the Rust 2018 Edition plans overall

nikomatsakis (May 18 2018 at 12:56, on Zulip):

and when I step back for second

nikomatsakis (May 18 2018 at 12:56, on Zulip):

if we consider location-invariant NLL

nikomatsakis (May 18 2018 at 12:57, on Zulip):

it is essentially "feature complete", from a certain perspective :)

pnkfelix (May 18 2018 at 12:57, on Zulip):

heh

nikomatsakis (May 18 2018 at 12:57, on Zulip):

it still needs polish: perf isn't where we'd like it to be, and diagnostics are uncertain

nikomatsakis (May 18 2018 at 12:57, on Zulip):

plus there are some lingering soundness bugs

pnkfelix (May 18 2018 at 12:57, on Zulip):

Thats with an r+ on the changes to MIR codegen for match?

nikomatsakis (May 18 2018 at 12:57, on Zulip):

yes, assuming that

pnkfelix (May 18 2018 at 12:57, on Zulip):

diagnostics are the big remaining thing I bet, okay

nikomatsakis (May 18 2018 at 12:57, on Zulip):

which reminds me that I was thinking more about that RFC, and wanted to double check what you were thinking, but that's a topic for another day :)

pnkfelix (May 18 2018 at 12:57, on Zulip):

but interesting to decide whehter that would trump further perforamcne investgation

nikomatsakis (May 18 2018 at 12:57, on Zulip):

right, so, from a risk management perspective

nikomatsakis (May 18 2018 at 12:58, on Zulip):

but interesting to decide whehter that would trump further perforamcne investgation

I did some profiling this morning, but let's revisit it in a sec

nikomatsakis (May 18 2018 at 12:58, on Zulip):

anyway, from a risk mgmt perspetive, Rust 2018 could ship with NLL if we solved diagnostics + perf

nikomatsakis (May 18 2018 at 12:58, on Zulip):

er, with "location-insensitive NLL"

pnkfelix (May 18 2018 at 12:58, on Zulip):

heh

nikomatsakis (May 18 2018 at 12:58, on Zulip):

needs a snappier name

nikomatsakis (May 18 2018 at 12:59, on Zulip):

and then we could follow up with an improved precision analysis at some point :)

nikomatsakis (May 18 2018 at 12:59, on Zulip):

this is basically the same conclusion you and I reached

nikomatsakis (May 18 2018 at 12:59, on Zulip):

but I am re-reaching it :)

pnkfelix (May 18 2018 at 12:59, on Zulip):

might be good to have a succinct summary of which cases it regressed

pnkfelix (May 18 2018 at 12:59, on Zulip):

or

pnkfelix (May 18 2018 at 12:59, on Zulip):

hm

nikomatsakis (May 18 2018 at 12:59, on Zulip):

do you mean, where does location sensitive matter?

pnkfelix (May 18 2018 at 12:59, on Zulip):

I was going to say "for the release notes"

pnkfelix (May 18 2018 at 12:59, on Zulip):

yeah

pnkfelix (May 18 2018 at 12:59, on Zulip):

but actually

pnkfelix (May 18 2018 at 12:59, on Zulip):

who cares

nikomatsakis (May 18 2018 at 12:59, on Zulip):

both of those :)

pnkfelix (May 18 2018 at 12:59, on Zulip):

why draw attention to our short comings!

nikomatsakis (May 18 2018 at 12:59, on Zulip):

the answer is roughly "the get_default example"

nikomatsakis (May 18 2018 at 13:00, on Zulip):

specifically, cases where you take a borrow and return it

nikomatsakis (May 18 2018 at 13:00, on Zulip):

but only conditionally

nikomatsakis (May 18 2018 at 13:00, on Zulip):

and along other paths, the borrow ends

pnkfelix (May 18 2018 at 13:00, on Zulip):

right, you removed that test outright

nikomatsakis (May 18 2018 at 13:00, on Zulip):

yeah

nikomatsakis (May 18 2018 at 13:00, on Zulip):

there are other examples I think

nikomatsakis (May 18 2018 at 13:00, on Zulip):

it doesn't have to leave the fn

nikomatsakis (May 18 2018 at 13:00, on Zulip):

but I think in practice that's where this arises the most

nikomatsakis (May 18 2018 at 13:01, on Zulip):

probably vec-push-ref too

nikomatsakis (May 18 2018 at 13:01, on Zulip):

vec-push-ref is this one:

pnkfelix (May 18 2018 at 13:01, on Zulip):

hmm did we not have a NLL unit test for vec-push-ref in the rust repo

nikomatsakis (May 18 2018 at 13:01, on Zulip):
fn foo() {
  let mut v = vec![];
  let p = 22;
  let x = &p;
  if true {
    v.push(x);
  } else {
    //  in here, `p` is not borrowed
  }
  use(v);
}
nikomatsakis (May 18 2018 at 13:02, on Zulip):

hmm did we not have a NLL unit test for vec-push-ref in the rust repo

I was just wondering the same

pnkfelix (May 18 2018 at 13:02, on Zulip):

oh hmm

nikomatsakis (May 18 2018 at 13:02, on Zulip):

should double check

nikomatsakis (May 18 2018 at 13:02, on Zulip):

or maybe it still works for some reason

pnkfelix (May 18 2018 at 13:02, on Zulip):

(I don't think we could have, becasue your PR did not touch any such test)

nikomatsakis (May 18 2018 at 13:02, on Zulip):

I will have to check

pnkfelix (May 18 2018 at 13:02, on Zulip):

oh that's a solid alteratnive theory

pnkfelix (May 18 2018 at 13:02, on Zulip):

:)

nikomatsakis (May 18 2018 at 13:02, on Zulip):

anyway I definitely agree that — for our own purposes —

nikomatsakis (May 18 2018 at 13:02, on Zulip):

a good write-up of which patterns we support

nikomatsakis (May 18 2018 at 13:02, on Zulip):

and which we expect to support in the future

nikomatsakis (May 18 2018 at 13:03, on Zulip):

is a good idea

nikomatsakis (May 18 2018 at 13:03, on Zulip):

but not maybe top priority

nikomatsakis (May 18 2018 at 13:03, on Zulip):

anyway, so the question then comes to how to divide up the effort between "polish location insensitive" vs "continue pushing on the newer analysis"

pnkfelix (May 18 2018 at 13:03, on Zulip):

yeah. Improving the expected user experience

pnkfelix (May 18 2018 at 13:03, on Zulip):

takes priority over documenting the far-future expectations

nikomatsakis (May 18 2018 at 13:04, on Zulip):

I of course want to ensure we do the latter ... but @Aaron Turon pointed out that, really, would it be so bad if we didn't? I am forced to admit that .. it doesn't matter that much

pnkfelix (May 18 2018 at 13:04, on Zulip):

Well, if we have volunteers excited about working on the latter

pnkfelix (May 18 2018 at 13:04, on Zulip):

then let them?

nikomatsakis (May 18 2018 at 13:04, on Zulip):

yeah so roughly that is what I felt like:

nikomatsakis (May 18 2018 at 13:04, on Zulip):

let's you + I (at least) focus on polish

pnkfelix (May 18 2018 at 13:04, on Zulip):

hopefully we can pull in people to focus on the top priority items, even if they are new people

nikomatsakis (May 18 2018 at 13:04, on Zulip):

and try to spin up tasks

pnkfelix (May 18 2018 at 13:04, on Zulip):

yeah I'm down with that

nikomatsakis (May 18 2018 at 13:05, on Zulip):

and others can choose to do whichever strikes their fancy

nikomatsakis (May 18 2018 at 13:05, on Zulip):

I suispect e.g. @David Wood might be up for tackling some polish-like tasks :)

nikomatsakis (May 18 2018 at 13:05, on Zulip):

/me waves to @David Wood

pnkfelix (May 18 2018 at 13:06, on Zulip):

I would like to try to see how much further I can go with rustc integration before next tuesday mtg

davidtwco (May 18 2018 at 13:06, on Zulip):

I certainly could do that.

nikomatsakis (May 18 2018 at 13:06, on Zulip):

yeah I still think rustc integration is key

nikomatsakis (May 18 2018 at 13:06, on Zulip):

otherwise I feel like Polonius can't make progress

pnkfelix (May 18 2018 at 13:07, on Zulip):

I do have some high-level Q's for you on that topic

pnkfelix (May 18 2018 at 13:07, on Zulip):

but lets tie off this discussion first

nikomatsakis (May 18 2018 at 13:07, on Zulip):

ok so, we could turn to a bit more detailed look at what polish means

pnkfelix (May 18 2018 at 13:08, on Zulip):

maybe need to actually do that review of the ui/*.nll.stderr files?

nikomatsakis (May 18 2018 at 13:08, on Zulip):

yeah, I saw two things:

pnkfelix (May 18 2018 at 13:08, on Zulip):

categorize into "soundness bug", "ux bug", "all is well" ?

nikomatsakis (May 18 2018 at 13:08, on Zulip):

1. look at a profile (I'm doing that)
2. review the stderr files

pnkfelix (May 18 2018 at 13:09, on Zulip):

oh right, polish include perf issues

pnkfelix (May 18 2018 at 13:09, on Zulip):

okay lets talk about the profile first then

nikomatsakis (May 18 2018 at 13:10, on Zulip):

okay lets talk about the profile first tehn

ok so here is what I see

nikomatsakis (May 18 2018 at 13:10, on Zulip):
lunch-box. perf focus -F 99 '{rustc_mir}' --tree-callees --min-percent 10 --relative | c++filt
Matcher    : {rustc_mir}
Matches    : 2268
Not Matches: 2802
Percentage : 100
Time       : 22.91s at 99 Hz

Tree
| matched `{rustc_mir}` (100% total, 1% self)
: | rustc_mir::borrow_check::nll::compute_regions (33% total, 0% self)
: : | rustc_mir::borrow_check::nll::type_check::type_check_internal (29% total, 0% self)
: : : | rustc_mir::borrow_check::nll::type_check::type_check::{{closure}} (10% total, 0% self)
: : : : | rustc_mir::borrow_check::nll::type_check::liveness::generate (10% total, 1% self)
: | rustc::infer::InferCtxtBuilder::enter (29% total, 0% self)
: : | rustc::ty::context::tls::with_related_context (29% total, 0% self)
: : : | rustc_mir::borrow_check::do_mir_borrowck (29% total, 0% self)
: : : : | <rustc_mir::dataflow::at_location::FlowAtLocation<BD> as rustc_mir::dataflow::at_location::FlowsAtLocation>::reconstruct_statement_effect (19% total, 0% self)
: | rustc_mir::dataflow::do_dataflow (25% total, 0% self)
: : | _ZN9rustc_mir8dataflow5impls7borrows7Borrows35kill_loans_out_of_scope_at_location17h0e83141d89a6718dE.llvm.6243250854293542925 (12% total, 1% self)
: : : | rustc_mir::borrow_check::nll::region_infer::RegionInferenceContext::region_contains_point (11% total, 2% self)
: : | <rustc_mir::dataflow::impls::borrows::Borrows<'a, 'gcx, 'tcx> as rustc_mir::dataflow::BitDenotation>::statement_effect (10% total, 0% self)
: : : | _ZN9rustc_mir8dataflow5impls7borrows7Borrows35kill_loans_out_of_scope_at_location17h0e83141d89a6718dE.llvm.6243250854293542925 (10% total, 0% self)
nikomatsakis (May 18 2018 at 13:10, on Zulip):

let me summarize what you are looking at

nikomatsakis (May 18 2018 at 13:10, on Zulip):

first off, that's clap :)

nikomatsakis (May 18 2018 at 13:10, on Zulip):

those %ages are the %age of total time just for borrowck

nikomatsakis (May 18 2018 at 13:10, on Zulip):

first observation:

nikomatsakis (May 18 2018 at 13:11, on Zulip):

region propagation is not on there

nikomatsakis (May 18 2018 at 13:11, on Zulip):

you would see it under compute_regions

nikomatsakis (May 18 2018 at 13:11, on Zulip):

but making things location invariant basically removed it from the profile

nikomatsakis (May 18 2018 at 13:11, on Zulip):

therefore, just doing SEME alone wouldn't help much

pnkfelix (May 18 2018 at 13:11, on Zulip):

wait

pnkfelix (May 18 2018 at 13:11, on Zulip):

now i'm confused

pnkfelix (May 18 2018 at 13:11, on Zulip):

or okay

pnkfelix (May 18 2018 at 13:11, on Zulip):

I would see it under compute_regions

pnkfelix (May 18 2018 at 13:11, on Zulip):

never mind then

pnkfelix (May 18 2018 at 13:12, on Zulip):

okay so type_check_internal is where all the time in compute_regions is going, right?

nikomatsakis (May 18 2018 at 13:12, on Zulip):

here is a more concise variant:

lunch-box. perf focus -F 99 '{do_mir_borrowck}' --tree-callees --min-percent 10 --tree-max-depth 2 --relative | c++filt
Matcher    : {do_mir_borrowck}
Matches    : 2086
Not Matches: 2984
Percentage : 100
Time       : 21.07s at 99 Hz

Tree
| matched `{do_mir_borrowck}` (100% total, 0% self)
: | rustc_mir::borrow_check::nll::compute_regions (36% total, 0% self)
: : | rustc_mir::borrow_check::nll::type_check::type_check_internal (31% total, 0% self) [...]
: | rustc_mir::dataflow::do_dataflow (27% total, 0% self)
: : | _ZN9rustc_mir8dataflow5impls7borrows7Borrows35kill_loans_out_of_scope_at_location17h0e83141d89a6718dE.llvm.6243250854293542925 (13% total, 1% self) [...]
: : | <rustc_mir::dataflow::impls::borrows::Borrows<'a, 'gcx, 'tcx> as rustc_mir::dataflow::BitDenotation>::statement_effect (11% total, 0% self) [...]
: | <rustc_mir::dataflow::at_location::FlowAtLocation<BD> as rustc_mir::dataflow::at_location::FlowsAtLocation>::reconstruct_statement_effect (21% total, 0% self)
: : | <rustc_mir::dataflow::impls::borrows::Borrows<'a, 'gcx, 'tcx> as rustc_mir::dataflow::BitDenotation>::statement_effect (10% total, 0% self) [...]
: : | _ZN9rustc_mir8dataflow5impls7borrows7Borrows35kill_loans_out_of_scope_at_location17h0e83141d89a6718dE.llvm.6243250854293542925 (10% total, 1% self) [...]
nikomatsakis (May 18 2018 at 13:12, on Zulip):

@pnkfelix right

nikomatsakis (May 18 2018 at 13:12, on Zulip):

and, within there

nikomatsakis (May 18 2018 at 13:13, on Zulip):

well some of it is some closure :)I have to see what that is

nikomatsakis (May 18 2018 at 13:13, on Zulip):

and the rest is the liveness computation

nikomatsakis (May 18 2018 at 13:13, on Zulip):

I haven't drilled down deeper there

nikomatsakis (May 18 2018 at 13:13, on Zulip):

that said, most of the work on liveness is

nikomatsakis (May 18 2018 at 13:13, on Zulip):

well, I don't know, but it's probably figuring out the dropck types

nikomatsakis (May 18 2018 at 13:13, on Zulip):

that's what it was before

nikomatsakis (May 18 2018 at 13:13, on Zulip):

I have to see if there is a smarter way to do that (I suspect .. maybe)

pnkfelix (May 18 2018 at 13:13, on Zulip):

but you're saying that's hurting us more under NLL than it does under AST-borrowck ?

nikomatsakis (May 18 2018 at 13:14, on Zulip):

yeah, I'm not 100% sure why

pnkfelix (May 18 2018 at 13:14, on Zulip):

hmm.

pnkfelix (May 18 2018 at 13:14, on Zulip):

any idea how much more?

nikomatsakis (May 18 2018 at 13:14, on Zulip):

there were reasons that were fixed

nikomatsakis (May 18 2018 at 13:14, on Zulip):

but I don't know what the new reasons are

nikomatsakis (May 18 2018 at 13:14, on Zulip):

mm I don't know I can try to find out

nikomatsakis (May 18 2018 at 13:14, on Zulip):

that said, it's interesting to look at the rest of the profile too

pnkfelix (May 18 2018 at 13:14, on Zulip):

right

nikomatsakis (May 18 2018 at 13:14, on Zulip):

e.g. 27% is do_dataflow

pnkfelix (May 18 2018 at 13:15, on Zulip):

I jut don't want us wasting time trying to optimize something

nikomatsakis (May 18 2018 at 13:15, on Zulip):

of that a whopping 13% is killing loans :)

pnkfelix (May 18 2018 at 13:15, on Zulip):

that is already known to also be slow under AST-borrowck

pnkfelix (May 18 2018 at 13:15, on Zulip):

(i mean, it'd be great to make both faster)

nikomatsakis (May 18 2018 at 13:15, on Zulip):

yeah, I will try to compare

nikomatsakis (May 18 2018 at 13:15, on Zulip):

but last time I looked, and I forget the details why, it was harder in NLL that AST

pnkfelix (May 18 2018 at 13:15, on Zulip):

okay

pnkfelix (May 18 2018 at 13:15, on Zulip):

so, rest of profile

nikomatsakis (May 18 2018 at 13:15, on Zulip):

I will try to get a better answer why that is though :P

pnkfelix (May 18 2018 at 13:15, on Zulip):

(deleted)

nikomatsakis (May 18 2018 at 13:16, on Zulip):

reconstruct_statement_effect is the other big one

pnkfelix (May 18 2018 at 13:17, on Zulip):

yeah. let me go skim over those two

pnkfelix (May 18 2018 at 13:17, on Zulip):

I wrote them (or some version of them) so maybe I have some insight into their slowness

nikomatsakis (May 18 2018 at 13:17, on Zulip):

it may just be that we are calling it a lot?

nikomatsakis (May 18 2018 at 13:17, on Zulip):

perf can also give pretty detailed look

nikomatsakis (May 18 2018 at 13:18, on Zulip):

e.g., per line

nikomatsakis (May 18 2018 at 13:18, on Zulip):

so that might show something

pnkfelix (May 18 2018 at 13:18, on Zulip):

we also haven't made any attempt to adjust the order we visit the Basic blocks, right?

pnkfelix (May 18 2018 at 13:19, on Zulip):

i.e. isn't RPO supposed to actually perform better on the average?

pnkfelix (May 18 2018 at 13:19, on Zulip):

maybe that is not worth attacking first

pnkfelix (May 18 2018 at 13:19, on Zulip):

(oh and of course that depends on the analysis direction...)

nikomatsakis (May 18 2018 at 13:19, on Zulip):

we have not, but .. probably not?

nikomatsakis (May 18 2018 at 13:19, on Zulip):

as an aside, it's instructive to look at @Frank McSherry's datafrog

nikomatsakis (May 18 2018 at 13:20, on Zulip):

we might get wins from using that for these other computations too

nikomatsakis (May 18 2018 at 13:20, on Zulip):

it works rather differently but seems to be pretty fast :)

nikomatsakis (May 18 2018 at 13:20, on Zulip):

anyway I don't quite know what to get out of this yet

nikomatsakis (May 18 2018 at 13:20, on Zulip):

we need to do more of a deep dive probably

nikomatsakis (May 18 2018 at 13:20, on Zulip):

I could imagine splitting up the two problems

nikomatsakis (May 18 2018 at 13:20, on Zulip):

or even 3

nikomatsakis (May 18 2018 at 13:20, on Zulip):

that is:

nikomatsakis (May 18 2018 at 13:21, on Zulip):
nikomatsakis (May 18 2018 at 13:21, on Zulip):

those are the 3 hot spots

nikomatsakis (May 18 2018 at 13:21, on Zulip):

I'm not sure in each case whether we want to optimize the function itself or the way we are using it

nikomatsakis (May 18 2018 at 13:21, on Zulip):

usually the latter pays more than the former :)

nikomatsakis (May 18 2018 at 13:21, on Zulip):

...but it depends :)

nikomatsakis (May 18 2018 at 13:24, on Zulip):

(cc @David Wood, are you interested in trying to do some performance optimization?)

pnkfelix (May 18 2018 at 13:25, on Zulip):

I'm actually a little surprised that statement_effect shows up in profile

pnkfelix (May 18 2018 at 13:25, on Zulip):

because the intention, if I remember right

nikomatsakis (May 18 2018 at 13:25, on Zulip):

@pnkfelix actually, looking a bit more closely...

pnkfelix (May 18 2018 at 13:25, on Zulip):

is that you should be calling that O(#stmt) times: Once to build the gen-kill sets

davidtwco (May 18 2018 at 13:25, on Zulip):

@nikomatsakis I'm happy to try anything.

nikomatsakis (May 18 2018 at 13:26, on Zulip):

kill_loans_out_of_scope_at_location is hotter than I thought

pnkfelix (May 18 2018 at 13:26, on Zulip):

and a second time after Dataflow is solved, to actually do precise analysis within eac basic block

nikomatsakis (May 18 2018 at 13:26, on Zulip):

overall it accounts for 22% of total execution time (not just borrowck)

nikomatsakis (May 18 2018 at 13:26, on Zulip):

borrowck is 41%

nikomatsakis (May 18 2018 at 13:26, on Zulip):

so that's fully half

pnkfelix (May 18 2018 at 13:26, on Zulip):

hmm

nikomatsakis (May 18 2018 at 13:26, on Zulip):

kind of ridiculous

nikomatsakis (May 18 2018 at 13:27, on Zulip):

ah, it goes over every borrow

pnkfelix (May 18 2018 at 13:27, on Zulip):

we call that function a lot

nikomatsakis (May 18 2018 at 13:27, on Zulip):

yeah I mean I remember I wrote it quick-n-dirty

pnkfelix (May 18 2018 at 13:27, on Zulip):

well

pnkfelix (May 18 2018 at 13:27, on Zulip):

O(n) times again

pnkfelix (May 18 2018 at 13:27, on Zulip):

i.e. it should not be part of the iterative dataflow solving

nikomatsakis (May 18 2018 at 13:28, on Zulip):

well

nikomatsakis (May 18 2018 at 13:28, on Zulip):

but it is part of the walk we do later

nikomatsakis (May 18 2018 at 13:28, on Zulip):

i.e., as we walk down the list of statements

nikomatsakis (May 18 2018 at 13:28, on Zulip):

if I'm not mistaken

nikomatsakis (May 18 2018 at 13:28, on Zulip):

that is, it may be the reason that reconstruct_statement_effect is so expensive

pnkfelix (May 18 2018 at 13:28, on Zulip):

sure

pnkfelix (May 18 2018 at 13:28, on Zulip):

I guess part of my problem

pnkfelix (May 18 2018 at 13:28, on Zulip):

is that I don't know how many iterations dataflow is taking to converge

pnkfelix (May 18 2018 at 13:28, on Zulip):

in the bad cases

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

indeed it is called by the various _effect routines

pnkfelix (May 18 2018 at 13:29, on Zulip):

so my initial thinking of "where is the dataflow solving taking up time"

pnkfelix (May 18 2018 at 13:29, on Zulip):

may be an incorrect intuition

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

I can probably tell you that

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

that is, I can look where the effect functions are being called from

pnkfelix (May 18 2018 at 13:30, on Zulip):

well

pnkfelix (May 18 2018 at 13:30, on Zulip):

is walk_cfg in the profile?

nikomatsakis (May 18 2018 at 13:30, on Zulip):

it appears that they are called about 2/3 directly from do_mir_borrowck (or something inlined into it) and 1/3 from do_dataflow

nikomatsakis (May 18 2018 at 13:30, on Zulip):

walk-cfg is not in the profile

pnkfelix (May 18 2018 at 13:31, on Zulip):

okay

pnkfelix (May 18 2018 at 13:31, on Zulip):

because fn dataflow is in two parts

pnkfelix (May 18 2018 at 13:31, on Zulip):

build_sets() makes the entry gen and kill sets

pnkfelix (May 18 2018 at 13:31, on Zulip):

but it is not iterative

pnkfelix (May 18 2018 at 13:31, on Zulip):

and then propagate() does the iterative solution to the dataflow eqns.

pnkfelix (May 18 2018 at 13:32, on Zulip):

so my guess is that you'll see build_sets() in the profile.

pnkfelix (May 18 2018 at 13:32, on Zulip):

just because that construction is so darn expensive?

nikomatsakis (May 18 2018 at 13:32, on Zulip):

mm 0%

nikomatsakis (May 18 2018 at 13:32, on Zulip):

but sometimes inlining makes that misleading

pnkfelix (May 18 2018 at 13:32, on Zulip):

hmm

nikomatsakis (May 18 2018 at 13:33, on Zulip):

or else my tool is broken somehow ;)

nikomatsakis (May 18 2018 at 13:33, on Zulip):

but it seems to be yielding coherent results

pnkfelix (May 18 2018 at 13:33, on Zulip):

is your full stack trace showing the call chain to _effect visible somewhere?

nikomatsakis (May 18 2018 at 13:33, on Zulip):

@David Wood well this seems pretty important; I'm not 100% sure what to do yet, it's going to bear some digging around.

nikomatsakis (May 18 2018 at 13:33, on Zulip):

is your full stack trace showing the call chain to _effect visible somewhere?

I can paste it

nikomatsakis (May 18 2018 at 13:34, on Zulip):
lunch-box. perf focus -F 99 '{statement_effect}' --tree-callers | c++filt
Matcher    : {statement_effect}
Matches    : 696
Not Matches: 4374
Percentage : 13
Time       : 7.03s at 99 Hz

Tree
| matched `{statement_effect}` (13% total, 0% self)
: | rustc_mir::borrow_check::do_mir_borrowck (8% total, 0% self)
: : | rustc::ty::context::tls::with_related_context (8% total, 0% self)
: : : | rustc::infer::InferCtxtBuilder::enter (8% total, 0% self)
: : : : | rustc_mir::borrow_check::mir_borrowck (8% total, 0% self)
: : : : : | [unknown] (8% total, 8% self)
: : : : : | rustc::ty::maps::<impl rustc::ty::maps::config::QueryConfig<'tcx> for rustc::ty::maps::queries::mir_borrowck<'tcx>>::compute (0% total, 0% self)
: : : : : : | [unknown] (0% total, 0% self)
: | rustc_mir::dataflow::do_dataflow (4% total, 0% self)
: : | rustc_mir::borrow_check::do_mir_borrowck (4% total, 0% self)
: : : | rustc::ty::context::tls::with_related_context (4% total, 0% self)
: : : : | [unknown] (4% total, 4% self)
: : : : | rustc::infer::InferCtxtBuilder::enter (0% total, 0% self)
: : : : : | [unknown] (0% total, 0% self)
: | rustc_mir::dataflow::do_dataflow (0% total, 0% self)
: : | rustc_mir::borrow_check::do_mir_borrowck (0% total, 0% self)
: : : | rustc::ty::context::tls::with_related_context (0% total, 0% self)
: : : : | rustc::infer::InferCtxtBuilder::enter (0% total, 0% self)
: : : : : | [unknown] (0% total, 0% self)
pnkfelix (May 18 2018 at 13:34, on Zulip):

oh I see, it looks like a direct call from do_dataflow to xxx_effect, right?

nikomatsakis (May 18 2018 at 13:34, on Zulip):

yes

pnkfelix (May 18 2018 at 13:34, on Zulip):

yeah that sounds like inlining to me

nikomatsakis (May 18 2018 at 13:34, on Zulip):

sometimes I go and add #[inline(never)]

nikomatsakis (May 18 2018 at 13:34, on Zulip):

when doing this sort of thing

nikomatsakis (May 18 2018 at 13:34, on Zulip):

to various big fns

nikomatsakis (May 18 2018 at 13:34, on Zulip):

where inlining looks unlikely to be profitable

nikomatsakis (May 18 2018 at 13:34, on Zulip):

LLVM really likes to inline

nikomatsakis (May 18 2018 at 13:35, on Zulip):

anyway, I suspect we should spin off a distinct thread focused on this and carry on independently

nikomatsakis (May 18 2018 at 13:35, on Zulip):

and turn to the diagnostics topic?

pnkfelix (May 18 2018 at 13:35, on Zulip):

okay sure

nikomatsakis (May 18 2018 at 13:36, on Zulip):

ok...

nikomatsakis (May 18 2018 at 13:36, on Zulip):

so as for diagnostics

nikomatsakis (May 18 2018 at 13:36, on Zulip):

I guess the main job is that we need to categorize?

pnkfelix (May 18 2018 at 13:37, on Zulip):

yeah whoops that's a task I failed to follow up on

nikomatsakis (May 18 2018 at 13:37, on Zulip):

how many such files are there anyway

nikomatsakis (May 18 2018 at 13:37, on Zulip):

answer: 169

pnkfelix (May 18 2018 at 13:38, on Zulip):

I see 171

nikomatsakis (May 18 2018 at 13:38, on Zulip):

ok

pnkfelix (May 18 2018 at 13:38, on Zulip):

but close enough

nikomatsakis (May 18 2018 at 13:38, on Zulip):

well, yeah

nikomatsakis (May 18 2018 at 13:38, on Zulip):

this is some random branch of mine

pnkfelix (May 18 2018 at 13:38, on Zulip):

after this meeting I have to go run errands

pnkfelix (May 18 2018 at 13:38, on Zulip):

otherwise I'd offer to immediately make the todo list or this

nikomatsakis (May 18 2018 at 13:39, on Zulip):

well it's easy enough to massage those into a todo list

nikomatsakis (May 18 2018 at 13:39, on Zulip):

I can do that

nikomatsakis (May 18 2018 at 13:39, on Zulip):

the thing I'm mostly interested in

nikomatsakis (May 18 2018 at 13:39, on Zulip):

is how we expect to categorize

nikomatsakis (May 18 2018 at 13:39, on Zulip):

i.e., are there a fixed set of buckets?

nikomatsakis (May 18 2018 at 13:39, on Zulip):

I think you had a sample before

pnkfelix (May 18 2018 at 13:41, on Zulip):

"soundness bug", "ux bug", "all is well"

nikomatsakis (May 18 2018 at 13:42, on Zulip):

hmm

pnkfelix (May 18 2018 at 13:42, on Zulip):

(of course deviation from AST-borrowck does not immediately imply a bug. )

nikomatsakis (May 18 2018 at 13:42, on Zulip):

sounds decent :)

nikomatsakis (May 18 2018 at 13:42, on Zulip):

we may want one more category?

pnkfelix (May 18 2018 at 13:42, on Zulip):

can you think of more categories?

nikomatsakis (May 18 2018 at 13:42, on Zulip):

or maybe that's a good start

nikomatsakis (May 18 2018 at 13:42, on Zulip):

I can imagine wanting to drill into ux bug a bit more

pnkfelix (May 18 2018 at 13:42, on Zulip):

or subdivisions of interest?

pnkfelix (May 18 2018 at 13:42, on Zulip):

okay

nikomatsakis (May 18 2018 at 13:42, on Zulip):

e.g., "MY EYES" vs "meh"

pnkfelix (May 18 2018 at 13:42, on Zulip):

yes

pnkfelix (May 18 2018 at 13:42, on Zulip):

maybe like "wrong span" vs "totally illegible"

nikomatsakis (May 18 2018 at 13:42, on Zulip):

missing suggestions and tips will be a common thing

nikomatsakis (May 18 2018 at 13:43, on Zulip):

but I'm not sure we want to categorize in this way

nikomatsakis (May 18 2018 at 13:43, on Zulip):

that is, it might be more like how important is that suggestion/span/etc

nikomatsakis (May 18 2018 at 13:43, on Zulip):

(that's kind of a judgement call, obviously)

pnkfelix (May 18 2018 at 13:43, on Zulip):

I kind of like just weighing importance, as you put it in "MY EYES" vs "meh"

nikomatsakis (May 18 2018 at 13:43, on Zulip):

yeah

pnkfelix (May 18 2018 at 13:43, on Zulip):

yeah its subjective, but okay

nikomatsakis (May 18 2018 at 13:43, on Zulip):

I think that might be best

nikomatsakis (May 18 2018 at 13:43, on Zulip):

seems like that is ultimately what matters

pnkfelix (May 18 2018 at 13:43, on Zulip):

I guess subjectivity is troublesome

nikomatsakis (May 18 2018 at 13:43, on Zulip):

well it's just a first scratch

pnkfelix (May 18 2018 at 13:43, on Zulip):

because that means it may be hard to delegrate?

nikomatsakis (May 18 2018 at 13:43, on Zulip):

I mean we get the most important

nikomatsakis (May 18 2018 at 13:43, on Zulip):

then we go to the rest

nikomatsakis (May 18 2018 at 13:43, on Zulip):

and re-triage

nikomatsakis (May 18 2018 at 13:44, on Zulip):

rinse, repeat until we run out of time :party_popper:

pnkfelix (May 18 2018 at 13:44, on Zulip):

well

pnkfelix (May 18 2018 at 13:44, on Zulip):

how do we catch cases erroneously categorized as "meh"

nikomatsakis (May 18 2018 at 13:44, on Zulip):

in the second pass?

pnkfelix (May 18 2018 at 13:44, on Zulip):

without reviewing all UX bugs all the time?

pnkfelix (May 18 2018 at 13:44, on Zulip):

(deleted)

nikomatsakis (May 18 2018 at 13:44, on Zulip):

I guess i'm imagining we don't :)

pnkfelix (May 18 2018 at 13:44, on Zulip):

hmm I guess if there are a finite number of passes

nikomatsakis (May 18 2018 at 13:44, on Zulip):

until we've dealt with the rest

pnkfelix (May 18 2018 at 13:45, on Zulip):

then its not really an issue, okay

nikomatsakis (May 18 2018 at 13:45, on Zulip):

or maybe somebody reports it separately and we decide to promote

nikomatsakis (May 18 2018 at 13:45, on Zulip):

ultimately i'm not that worried, I feel like the main thing is we have to pick some stuff to focus on somehow

pnkfelix (May 18 2018 at 13:45, on Zulip):

(deleted)

pnkfelix (May 18 2018 at 13:46, on Zulip):

oh there is one more .... maybe work item ...

pnkfelix (May 18 2018 at 13:46, on Zulip):

we probably should port more (all?) compile-fail tests to ui/

pnkfelix (May 18 2018 at 13:46, on Zulip):

in order that they get subject to this .nll.stderr thing

pnkfelix (May 18 2018 at 13:46, on Zulip):

or is that a waste of effort ?

nikomatsakis (May 18 2018 at 13:47, on Zulip):

hmm

nikomatsakis (May 18 2018 at 13:47, on Zulip):

not sure

pnkfelix (May 18 2018 at 13:47, on Zulip):

it would probably (vastly?) increase the workload in terms of number of .nll.stderr files to review

nikomatsakis (May 18 2018 at 13:47, on Zulip):

but maybe we put it off :)

nikomatsakis (May 18 2018 at 13:47, on Zulip):

idea:

nikomatsakis (May 18 2018 at 13:47, on Zulip):

schedule 30 min or 1 hour

nikomatsakis (May 18 2018 at 13:47, on Zulip):

maybe monday

nikomatsakis (May 18 2018 at 13:47, on Zulip):

to do this together

nikomatsakis (May 18 2018 at 13:47, on Zulip):

i.e., to get started

nikomatsakis (May 18 2018 at 13:47, on Zulip):

see if we can come up with criteria

nikomatsakis (May 18 2018 at 13:47, on Zulip):

I suspect after we do a few

pnkfelix (May 18 2018 at 13:47, on Zulip):

you first since there's only ~170

nikomatsakis (May 18 2018 at 13:47, on Zulip):

it'll be clearer

pnkfelix (May 18 2018 at 13:48, on Zulip):

oh

pnkfelix (May 18 2018 at 13:48, on Zulip):

I see now

nikomatsakis (May 18 2018 at 13:48, on Zulip):

there aren't that many, it's true

pnkfelix (May 18 2018 at 13:48, on Zulip):

you're not intending that we finish in that time frame

pnkfelix (May 18 2018 at 13:48, on Zulip):

necessarily

nikomatsakis (May 18 2018 at 13:48, on Zulip):

right

pnkfelix (May 18 2018 at 13:48, on Zulip):

just that we better understand the task at hand

nikomatsakis (May 18 2018 at 13:48, on Zulip):

yeah I figured after we do like 5-10

pnkfelix (May 18 2018 at 13:48, on Zulip):

okay. I'm good with that plan, I think.

nikomatsakis (May 18 2018 at 13:48, on Zulip):

we can do it live over zulip even and others can kibbitz if they like :)

pnkfelix (May 18 2018 at 13:49, on Zulip):

beautful

pnkfelix (May 18 2018 at 13:49, on Zulip):

make a separate topic for each .nll.stderr file. ;)

pnkfelix (May 18 2018 at 13:49, on Zulip):

okay

pnkfelix (May 18 2018 at 13:49, on Zulip):

so that's performance and diagnostics

pnkfelix (May 18 2018 at 13:50, on Zulip):

(let me go schedule a meeting for monday now, let you decide whether it works for you)

nikomatsakis (May 18 2018 at 13:50, on Zulip):

I think (my) monday morning is basically free

pnkfelix (May 18 2018 at 13:50, on Zulip):

oh

pnkfelix (May 18 2018 at 13:50, on Zulip):

monday is a holiday here. Logan's daycare is closed

pnkfelix (May 18 2018 at 13:50, on Zulip):

The evening might work

pnkfelix (May 18 2018 at 13:51, on Zulip):

would you be around at 1pm your time?

pnkfelix (May 18 2018 at 13:51, on Zulip):

(then again, maybe this is not a good subject matter for a 10pm meeting)

pnkfelix (May 18 2018 at 13:51, on Zulip):

or wait

pnkfelix (May 18 2018 at 13:51, on Zulip):

time zone math!

pnkfelix (May 18 2018 at 13:52, on Zulip):

I'll just put something down and let you react to it

pnkfelix (May 18 2018 at 13:53, on Zulip):

okay

Jake Goulding (May 20 2018 at 16:15, on Zulip):

One thing in the context of error messages: Ideally NLL will reduce the total number of compiler errors that people get. You could say that a (temporary) regression in the quality of the errors they do get will be amortized by less overall errors experienced.

Last update: Nov 21 2019 at 14:40UTC