Stream: t-compiler/wg-polymorphization

Topic: progress updates


davidtwco (Dec 03 2019 at 16:58, on Zulip):

(this thread continues from some private messages where I was keeping people up-to-date, it might also be useful for #t-compiler meeting wg check-ins)


Another update: continued from where I was before - got the test I added (still behind a flag) passing and showing that less mono-items are being produced.

I've since been trying to remove the flag and run the analysis for everything and get that working. I've been making progress and I've got everything up to libproc_macro compiling again, it is failing with this error that I've hit a bit of a wall fixing.

This branch still contains everything so if you have any ideas on what might be up, I'd appreciate it. I'm going to shift gears from focusing on the implementation to focusing on the interim report I need to write on this, which is due in a couple weeks (though, if I do unblock myself, I'd like to get a PR up with this working soon).

(the last commit is the one that enables the analysis w/out the flag - by changing the default value to true, I'll eventually remove it entirely)

There are also plenty of places where I just made a decision on how to do something so I could proceed, there's no doubt lots of it that needs work.

nikomatsakis (Dec 31 2019 at 14:50, on Zulip):

Hey, how goes it @davidtwco ? Sorry I've not been following too closely -- curious to hear how it goes!

davidtwco (Dec 31 2019 at 14:55, on Zulip):

@nikomatsakis I’ve not made a lot of progress since the last update, been working on the patch here and there during the holidays. Before the holidays, I was mostly focused on reports I had to write for the university.

I’ve made a change that I think fixed the ICE that I was getting, but I’m not completely confident. The fix made sense based on what eddyb had suggested was the issue, but I’ve now got an ICE in an earlier crate (libcore instead of libproc_macro) which I wasn’t expecting. Not made a whole lot of progress solving that. Lost some time last week when backtraces went missing (#67615).

nikomatsakis (Dec 31 2019 at 14:57, on Zulip):

OK -- I'm wondering how much this can be landed in slices

davidtwco (Dec 31 2019 at 14:58, on Zulip):

Everything except the last commit I have passes all tests - I’d want to tidy it a little bit, but everything is behind flags and only my tests use it. It isn’t enough to compile the compiler (which is what the last commit tries to enable).

davidtwco (Dec 31 2019 at 15:00, on Zulip):

One place that I’ve struggled is coming up with sufficient test cases such that I’m not debugging huge crates that ICE instead of smaller test cases. I think if I were able to do that better, I’d be able to better understand and fix issues quicker and get something that can bootstrap quicker.

nikomatsakis (Feb 04 2020 at 18:47, on Zulip):

@davidtwco do you think there would be room for others to get involved and help out here? I'm also wondering if we have any estimates beyond "intuition" of the payoff here yet?

nikomatsakis (Feb 04 2020 at 18:47, on Zulip):

@pnkfelix and I are trying to think carefully about where to spend our energies

nikomatsakis (Feb 04 2020 at 18:47, on Zulip):

This work still feels very promising to me in terms of making big wins for compilation time, at least some of the time

davidtwco (Feb 04 2020 at 20:48, on Zulip):

I’ve not got much of a sense of how impactful it would be. I’m still trying to hammer away at the ICE that I hit a few weeks ago (I rebased at one point which introduced new ICEs from some new code that was added which took a bit to work through to get back to where I was). I’ve just struggled to find the time between work and other university courses. I intend to start sinking more time into it soon.

davidtwco (Feb 04 2020 at 20:49, on Zulip):

Given that it’s just trying to get the core to build with the changes at the moment, I’m not sure there’s much room to parallelise.

davidtwco (Feb 04 2020 at 20:50, on Zulip):

I have a feeling there aren’t many ICEs left to deal with until it’s working, I just need to figure those out.

davidtwco (Feb 04 2020 at 20:52, on Zulip):

To elaborate on what error I’m seeing before, it’s the error where @eddyb suggested that trait substs for vtable entries were being used where impl substs were expected. I’ve been digging into it for a while now but struggling to make headway. I’ve got notes written down that I can fetch with what my theories and understanding was last I was working on this.

davidtwco (Feb 04 2020 at 20:53, on Zulip):

Though, I don’t know how much rework there will need to be if the compiler team don’t like some of the decisions I’ve made in integrating the unused type parameter analysis

davidtwco (Feb 04 2020 at 20:54, on Zulip):

@nikomatsakis ^

pnkfelix (Feb 05 2020 at 14:59, on Zulip):

Though, I don’t know how much rework there will need to be if the compiler team don’t like some of the decisions I’ve made in integrating the unused type parameter analysis

Maybe we could schedule a design meeting to discuss this?

davidtwco (Feb 05 2020 at 15:05, on Zulip):

Potentially, I don't think it's large enough to warrant that, discussion on the eventual PR would probably be sufficient.

pnkfelix (Feb 05 2020 at 15:09, on Zulip):

Well if you're worried about the design decisions, I figure it makes some amount of sense to advertise them ahead of time. Do the commit messages point out the cases you think might be contentious, or is there some other way that they are flagged up front?

pnkfelix (Feb 05 2020 at 15:09, on Zulip):

even if all we do is point it out during the triage meeting, that might be enough to at least give the people with an opinion a chance to voice that opinion.

davidtwco (Feb 05 2020 at 15:14, on Zulip):

That's a good point. People could definitely look at the branch and leave some comments here or on GitHub if we highlighted it at a triage meeting.

davidtwco (Feb 05 2020 at 15:16, on Zulip):

Primarily it is the way that I've chosen to integrate the analysis into Instance::resolve (and then that trickles down into fewer MonoItems and less codegen) rather than modify the Instance in MonoItem::Fn during monomorphisation with the analysis results and then update where we generate call instructions to match.

davidtwco (Feb 05 2020 at 15:19, on Zulip):

Modifying all Instances has more side-effects, as that type is used in more places (which has almost certainly resulted in some of the ICEs I've been dealing with), but results in a much less invasive change.

davidtwco (Feb 05 2020 at 15:21, on Zulip):

One of the descriptions of the Instance type that I read somewhere in the compiler made me think it was a good fit conceptually too.

pnkfelix (Feb 05 2020 at 16:09, on Zulip):

Actually, another thought: Even if this doesn't need a design meeting, per se, I think it does qualify as a major change

pnkfelix (Feb 05 2020 at 16:09, on Zulip):

which means you could/should write this up following the major change protocol

pnkfelix (Feb 05 2020 at 16:10, on Zulip):

in the same way that mw did on compiler-team#245

pnkfelix (Feb 05 2020 at 16:10, on Zulip):

and then any debate could be captured on the issue thread there

pnkfelix (Feb 05 2020 at 16:11, on Zulip):

I mainly want to ensure we have a good place to capture your exploration of the design space, and the justifications you used to guide your decisions

mark-i-m (Feb 10 2020 at 18:47, on Zulip):

I wasn't actually aware of this WG and it's not on the compiler-team website. Could someone give a brief summary of what polymorphization is and what the WG aims to solve?

lqd (Feb 10 2020 at 20:25, on Zulip):

https://rust-lang.github.io/compiler-team/working-groups/polymorphization/

pnkfelix (Feb 11 2020 at 17:19, on Zulip):

@davidtwco I assume the details of the crate names in the test output is a wart that we should get rid of somehow, right?

pnkfelix (Feb 11 2020 at 17:21, on Zulip):

I'm speaking of differences in test output like this: diff gist

pnkfelix (Feb 11 2020 at 17:22, on Zulip):

where the only difference is the ... stuff in core[...]::foo[0]::bar[0] or alloc[...]::foo[0]::bar[0]

pnkfelix (Feb 11 2020 at 17:22, on Zulip):

(Probably easiest/best to just normalize the test.stderr to get rid of it via a regexp substitution)

davidtwco (Feb 11 2020 at 17:22, on Zulip):

Are you looking at this branch - https://github.com/davidtwco/rust/tree/issue-46477-polymorphization ?

davidtwco (Feb 11 2020 at 17:23, on Zulip):

or this one - https://github.com/davidtwco/rust/tree/polymorphize-analysis ?

pnkfelix (Feb 11 2020 at 17:23, on Zulip):

the latter

pnkfelix (Feb 11 2020 at 17:23, on Zulip):

though mine might be out of date

davidtwco (Feb 11 2020 at 17:23, on Zulip):

Should be the former, I think.

davidtwco (Feb 11 2020 at 17:23, on Zulip):

The latter branch is when Niko and I first worked on this, not my most recent work.

davidtwco (Feb 11 2020 at 17:23, on Zulip):

These changes are the primary work - https://github.com/rust-lang/rust/compare/master...davidtwco:issue-46477-polymorphization

pnkfelix (Feb 11 2020 at 17:24, on Zulip):

oh oh you're right, I missed the year when I looked at the date of the last commit on polymorphize-analysis

pnkfelix (Feb 11 2020 at 17:24, on Zulip):

and I said "ah, January is more recent that December; done!"

davidtwco (Feb 11 2020 at 17:24, on Zulip):

This one only considers unused type parameters.

davidtwco (Feb 11 2020 at 17:25, on Zulip):

The testing is a little bit cleaner, I think.

davidtwco (Feb 11 2020 at 17:25, on Zulip):

The primary issue with it is that it cannot bootstrap (which the last commit enables by swapping a feature flag).

davidtwco (Feb 11 2020 at 17:25, on Zulip):

AFAIK (a rebase might have changed this), it passed all tests, including my own tests of the polymorphization's integration, without the last commit.

pnkfelix (Feb 11 2020 at 17:27, on Zulip):

(seems like the most recent commit does quite a bit more, at least superficially, than just swapping a feature flag...)

pnkfelix (Feb 11 2020 at 17:27, on Zulip):

but its WIP (and explicitly marked as such) so I'm not complaining

davidtwco (Feb 11 2020 at 17:28, on Zulip):

It does, I've been putting any WIP changes that I've been experimenting with (to fix the ICEs) in that commit so it's pushed somewhere when I'm between machines.

davidtwco (Feb 11 2020 at 17:28, on Zulip):

and then breaking bunches of those out into separate commits at later stages

davidtwco (Feb 11 2020 at 17:29, on Zulip):

I've not rebased since the start of the year so I don't know what impact that will have, I lost a week or so debugging a new ICE that was introduced at that rebase.

Eh2406 (Feb 12 2020 at 22:14, on Zulip):

How is it going? Is there something a Rubber duck can do to help?

nikomatsakis (Feb 14 2020 at 16:40, on Zulip):

I think maybe pulling down the branch and trying to build it, @Eh2406, and just see if you can figure out whatever is stopping it, would be the best thing

nikomatsakis (Feb 14 2020 at 16:41, on Zulip):

I have the feeling @davidtwco is pretty busy just now but if, when he comes back to it, there were a few fewer bugs to fix, I'm sure he wouldn't complain :)

nikomatsakis (Feb 14 2020 at 16:41, on Zulip):

actually maybe just rebasing the branch would be helpful?

davidtwco (Feb 14 2020 at 16:43, on Zulip):

@Eh2406 and I spoke briefly on Discord in #wg-compiler-performance there about it. It's blocked on me clearing some coursework off my plate before I can get back to it. I'm not sure how it works with this being part of my master's thesis if anyone else works on it, but if anyone wants to pull it and see if they can't work out what's going on, I'd be fine with that - it's still work that the project wants done, irrespective of it being part of my thesis.

nikomatsakis (Feb 14 2020 at 16:44, on Zulip):

I've always wondered a bit about that when it comes to theses; but I suspect it's not a problem if other people are helping out a little. Though you may want to talk to your advisor about it, yeah.

davidtwco (Feb 14 2020 at 16:46, on Zulip):

Normally it's probably not as required, but because of my degree program having an industrial placement last year, I need to take eight courses alongside the master's project (and I work three days a week) so my time is quite stretched.

Eh2406 (Feb 15 2020 at 23:04, on Zulip):

I did some work today, I got rust repo set up and building and tests passing. So I am set up to start trying to understand, next time I have time.

Eh2406 (Feb 16 2020 at 20:54, on Zulip):

An automatic understanding free rebase got hung up on the interactions with https://github.com/rust-lang/rust/pull/69100
The PR rebased pretty cleanly on to the conflicts parent. https://github.com/Eh2406/rust/tree/issue-46477-polymorphization

Eh2406 (Feb 16 2020 at 20:57, on Zulip):

I think the next step is a slower, one commit at a time cherry pick rebase. (Unless you have a alternative suggestion.)
@davidtwco does each commit check on its own?

davidtwco (Feb 16 2020 at 21:05, on Zulip):

It should do, I think I tried to ensure that was the case.

davidtwco (Feb 18 2020 at 22:52, on Zulip):

I've managed to get some time together today and I've rebased the PR.

davidtwco (Feb 18 2020 at 22:52, on Zulip):

Everything I have locally has been pushed.

davidtwco (Feb 18 2020 at 22:53, on Zulip):

It fails earlier than it did before, but after speaking to eddyb, I've got some ideas for a path forward, which I'll hopefully have time for during the remainder of the week.

Eh2406 (Feb 22 2020 at 20:32, on Zulip):

I was just going to ask for help getting my version of the reabase to work... glad to here I dont need to!

Eh2406 (Feb 22 2020 at 23:52, on Zulip):

Let me know how I (or my cpu) can (try) to help!

davidtwco (Mar 02 2020 at 23:38, on Zulip):

An update: thanks to help from eddyb, I've gotten the branch to the point where it can now bootstrap itself and is failing one UI test. There's still some tidying up to do and making sure it's working so I'll do that over the next few days and get a PR up.

nikomatsakis (Mar 03 2020 at 20:11, on Zulip):

@davidtwco that's awesome <3

Eh2406 (Mar 05 2020 at 15:31, on Zulip):

So is it working well enough to get some sense of perf?

davidtwco (Mar 05 2020 at 22:33, on Zulip):

Looked into the test failure a bunch, no luck on fixing. Opened a PR here: https://github.com/rust-lang/rust/pull/69749 - please leave all the comments!

davidtwco (Mar 05 2020 at 22:34, on Zulip):

@Eh2406 probably, I've not checked though

Eh2406 (Mar 07 2020 at 21:03, on Zulip):

Tried to build locally (so I can do a perf run) but I am getting an ICE. I think it looks like the ICE that CI hit.

davidtwco (Mar 07 2020 at 21:24, on Zulip):

Missed that ICE locally because I only did a stage one build, looking into it (and review comments) now.

davidtwco (Mar 08 2020 at 01:22, on Zulip):

Made progress on review comments, not so much on ICEs, will continue tomorrow.

davidtwco (Mar 08 2020 at 01:22, on Zulip):

(...or today, but later, I suppose)

davidtwco (Mar 14 2020 at 17:18, on Zulip):

Another update: I've pushed to the PR, hopefully resolving all compilation and test failures, see #69749.

Eh2406 (Mar 15 2020 at 03:56, on Zulip):

I tried to do a perf run. It did build successfully! I still haven't managed to set up perf correctly yet. but I did see some ICEs in the logs. I think syn is not working yet, but did not have time to investigate.

davidtwco (Mar 15 2020 at 11:08, on Zulip):

Turns out there were test failures in one of the smaller suites, will look into that during the week. Hopefully it’s not too buggy and we’ll be able to get a sense of whether there’s a perf difference.

davidtwco (Mar 15 2020 at 11:12, on Zulip):

I’m not going to rebase yet either, the PR that this depends on, #69935, is still in the queue and without conflicts so I’ll wait till that lands.

davidtwco (Mar 15 2020 at 12:28, on Zulip):

@eddyb @nikomatsakis Small update: I'm going to switch gears for a few days and work on my report for this - I'll be able to get most of it done, just leaving out the evaluation - I'll feel much more comfortable sinking time into the implementation afterwards when I know that work is out of the way.

My main concerns at the moment is that the implementation might not be at a place where I can compile some popular crates and get a idea of the perf difference, there are two reasons:

davidtwco (Mar 15 2020 at 12:31, on Zulip):

(if anyone's willing to help fix/investigate the test failure that's there, or look into the perf then I'm totally open to that - I've done enough of the changes that I'm sure someone else fixing a bug or two won't be a problem for the purposes of this being my master's project)

eddyb (Mar 15 2020 at 13:52, on Zulip):

Eh2406 said:

I tried to do a perf run. It did build successfully! I still haven't managed to set up perf correctly yet. but I did see some ICEs in the logs. I think syn is not working yet, but did not have time to investigate.

shouldn't we just be able to use @bors try and @rust-timer? or do you mean that?

eddyb (Mar 15 2020 at 13:57, on Zulip):

davidtwco said:

- There's eddyb's patch that could allow for a faster fix here, #69968, but we'll need to make a decision on whether the perf impact of that PR is acceptable.

that's not exactly the decision to make :P

I have guessed that the problem is adding a generic when there weren't any (i.e. there are enough captureless closures that now have an argument that's always ())

and my proposed potential solution is to tuple all of the synthetics together instead of just the upvars, but I haven't tried it yet

eddyb (Mar 15 2020 at 14:50, on Zulip):

eddyb said:

and my proposed potential solution is to tuple all of the synthetics together instead of just the upvars, but I haven't tried it yet

started working on that, just adjusted the existing commit to make it easier to tuple more things without breaking rustc::ty::print::pretty and rustc_typeck::check::closure

Eh2406 (Mar 15 2020 at 15:02, on Zulip):

shouldn't we just be able to use @bors try and @rust-timer? or do you mean that?

I was trying to be helpful by running locally. rust-timer will work when we have time for it.

davidtwco (Mar 15 2020 at 15:48, on Zulip):

that's not exactly the decision to make :P

Ah, I misunderstood.

eddyb (Mar 15 2020 at 16:52, on Zulip):

rust-timer will work when we have time for it.
are we running out of slots for that? I was about to start another run for my PR that tuples up upvars

simulacrum (Mar 15 2020 at 17:02, on Zulip):

no queue is empty

eddyb (Mar 15 2020 at 17:07, on Zulip):

@davidtwco https://github.com/rust-lang/rust/pull/69968#issuecomment-599237539

eddyb (Mar 15 2020 at 17:07, on Zulip):

see the HACK commit

davidtwco (Mar 15 2020 at 17:08, on Zulip):

Awesome

eddyb (Mar 15 2020 at 23:35, on Zulip):

@davidtwco screwed up whoops

eddyb (Mar 15 2020 at 23:40, on Zulip):

https://github.com/rust-lang/rust/pull/69968#issuecomment-599281756

eddyb (Mar 15 2020 at 23:45, on Zulip):

I'll look into the actual speedups tomorrow

eddyb (Mar 15 2020 at 23:45, on Zulip):

they basically involve deferring calling .expect_ty() or w/e until the very last moment :P

eddyb (Mar 15 2020 at 23:46, on Zulip):

since that's the only thing that looks significantly different to me (if it's actually the tuple itself that'd be annoying)

eddyb (Mar 18 2020 at 00:25, on Zulip):

@davidtwco decided to do this as a side thing but I don't think it will impact anything https://github.com/rust-lang/rust/pull/70089

eddyb (Mar 18 2020 at 00:25, on Zulip):

now to get back to the original PR and try to uhh make it fast

eddyb (Mar 18 2020 at 01:05, on Zulip):

@davidtwco https://github.com/rust-lang/rust/pull/69968#issuecomment-600370100

eddyb (Mar 18 2020 at 10:33, on Zulip):

phew those are better

eddyb (Mar 18 2020 at 10:34, on Zulip):

there's 18 less generic_of calls for the biggest win (-2.2%)

eddyb (Mar 18 2020 at 10:34, on Zulip):

so it might be related

davidtwco (Mar 18 2020 at 12:30, on Zulip):

That's awesome.

eddyb (Mar 19 2020 at 17:33, on Zulip):

@davidtwco https://github.com/rust-lang/rust/pull/69968#issuecomment-601230356

davidtwco (Mar 19 2020 at 17:45, on Zulip):

I saw, I'll need to wait for #69935 too before rebasing though.

davidtwco (Mar 19 2020 at 17:45, on Zulip):

I don't need to, but to save effort will.

nikomatsakis (Mar 24 2020 at 19:45, on Zulip):

@davidtwco what is the status here btw?

davidtwco (Mar 24 2020 at 19:45, on Zulip):

I just pushed earlier today, resolves any comments that were left and should have passing tests.

davidtwco (Mar 24 2020 at 19:46, on Zulip):

I've said that before though, but :fingers_crossed:

davidtwco (Mar 24 2020 at 19:48, on Zulip):

I've written like 65% of my paper. I'll shift back to that tomorrow with the aim of only having the evaluation left - where I'll try collect some numbers on the perf of this.

davidtwco (Mar 24 2020 at 19:49, on Zulip):

(the only sections I've got left to write are an abstract, introduction (what is rust, what is llvm, why do we want this, etc), talking about how it works, challenges faced, the evaluation, and a conclusion - easy parts hopefully)

davidtwco (Mar 24 2020 at 21:02, on Zulip):

CI is passing!

Eh2406 (Mar 24 2020 at 21:26, on Zulip):

davidtwco said:

(the only sections I've got left to write are an abstract, introduction (what is rust, what is llvm, why do we want this, etc), talking about how it works, challenges faced, the evaluation, and a conclusion - easy parts hopefully)

I am trying to do a local perf run.

Eh2406 (Mar 25 2020 at 01:21, on Zulip):

I did it a number of times, to check in case I set it up wrong. But I am seeing large regressions across the board. Even the helloworld benchmark is dramatically slower. I don't know the best way to share the results.

Eh2406 (Mar 25 2020 at 01:27, on Zulip):

commit-m-PR-x86_64-unknown-linux-gnu.json.sz commit-PR-x86_64-unknown-linux-gnu.json.sz

Eh2406 (Mar 25 2020 at 01:27, on Zulip):

m-PR is the master you rebased onto. PR is the PR.

davidtwco (Mar 25 2020 at 10:50, on Zulip):

It’s possible that this is just really bad, but there’s also a chance that one we look into why it’s bad, that I’ve just made a mistake somewhere.

davidtwco (Mar 25 2020 at 11:42, on Zulip):

I've queued a timer run so we'll see if that shows the same regression - I can think of one or two places where things could probably be made a little faster, but not many.

davidtwco (Mar 25 2020 at 11:47, on Zulip):

Looks like that try build is failing though :sad:

nikomatsakis (Mar 25 2020 at 20:25, on Zulip):

I mean certainly the analysis could be costing compilation time, and we might not be recouping it with reduced LLVM

nikomatsakis (Mar 25 2020 at 20:25, on Zulip):

It definitely bears investigation

davidtwco (Mar 25 2020 at 20:26, on Zulip):

It's certainly possible, I intend to look into it tomorrow (since the try build changed, there's more bugs to fix) - finishing as much of the report as I can today.

davidtwco (Mar 29 2020 at 20:56, on Zulip):

Another update: perf results are on the PR, they’re a mixed bag. I’ve finished writing my paper (only got a video presentation to do now, but that won’t take long) so I’m mostly done with the university requirements part of this work (I’ll have exams in a month but after that I can sink more time into this and other work).

cc @nikomatsakis @eddyb

nikomatsakis (Mar 31 2020 at 19:08, on Zulip):

@davidtwco so I was looking at the self-profile results from your PR for the 11% loss

nikomatsakis (Mar 31 2020 at 19:09, on Zulip):

it looks like it's in metadata_decode_entry and encode_query_results_for

nikomatsakis (Mar 31 2020 at 19:09, on Zulip):

any idea why that should be? that surprised me somewhat..?

nikomatsakis (Mar 31 2020 at 19:09, on Zulip):

er, wait

davidtwco (Mar 31 2020 at 19:10, on Zulip):

The 11% loss was due to LTO taking longer, other benchmarks didn’t have that big a loss, and those were typically due to metadata decode or something like that.

nikomatsakis (Mar 31 2020 at 19:10, on Zulip):

ok, so, these are the results that compare the "script-servo-opt" with "patched incremental: println" (11% perf regression)

nikomatsakis (Mar 31 2020 at 19:10, on Zulip):

that fits with what I've seeing

davidtwco (Mar 31 2020 at 19:10, on Zulip):

It only happened on one benchmark and I have no idea why.

davidtwco (Mar 31 2020 at 19:10, on Zulip):

Neither did eddy

davidtwco (Mar 31 2020 at 19:10, on Zulip):

IIRC

nikomatsakis (Mar 31 2020 at 19:10, on Zulip):

I was wondering about something else

nikomatsakis (Mar 31 2020 at 19:10, on Zulip):

we landed various PRs to libstd that did this optimization "by hand" for iterators

nikomatsakis (Mar 31 2020 at 19:10, on Zulip):

in principle, we ought to be able to revert those PRs now, right?

davidtwco (Mar 31 2020 at 19:11, on Zulip):

If this were to land; yeah.

nikomatsakis (Mar 31 2020 at 19:11, on Zulip):

I'm not sure we should roll that into this PR but it'd be interesting to see if doing so was "perf-neutral"

davidtwco (Mar 31 2020 at 19:12, on Zulip):

I’m not convinced that the perf gain from this is necessarily worth it landing, unless I’ve made a mistake somewhere and we’re losing out on what it could be, the gain is relatively small and it doesn’t play with incremental well as it’s currently written. Memory usage was also all over the place.

davidtwco (Mar 31 2020 at 19:13, on Zulip):

(basing the above just on the perf results)

nikomatsakis (Mar 31 2020 at 19:18, on Zulip):

Hmm. So, my hope was that this would be the first step towards more extensive work on polymorphization

nikomatsakis (Mar 31 2020 at 19:18, on Zulip):

But I admit I was hoing for better initial perf results

nikomatsakis (Mar 31 2020 at 19:18, on Zulip):

I think this might make a really nice topic for a design meeting

nikomatsakis (Mar 31 2020 at 19:18, on Zulip):

( I was also thinking that this ought to be a "major change proposal" probably, though that's not really in use .. )

nikomatsakis (Mar 31 2020 at 19:19, on Zulip):

but I guess I shoudlf just read your thesis? :)

nikomatsakis (Mar 31 2020 at 19:19, on Zulip):

I'm curious as to why it's not incremental friendly, though I guess I can imagine some of the reasons

davidtwco (Mar 31 2020 at 19:19, on Zulip):

Yeah, I’m slightly disappointed by how little perf impact there’s been, it’s very mixed.

nikomatsakis (Mar 31 2020 at 19:20, on Zulip):

nikomatsakis said:

Hmm. So, my hope was that this would be the first step towards more extensive work on polymorphization

I guess the real question is what the next steps look like -- this maybe raises again the question of the work we were doing earlier in trying to estimate how much opportunity is really out there

davidtwco (Mar 31 2020 at 19:20, on Zulip):

I just think the analysis walks the line of being not beneficial enough for caching the results to be valuable, but just expensive enough that it is, depending on the workload.

davidtwco (Mar 31 2020 at 19:21, on Zulip):

That said, I’m basing that on reading a handful of percentages, I don’t really know.

davidtwco (Mar 31 2020 at 19:22, on Zulip):

cc @eddyb who might have some thoughts

eddyb (Mar 31 2020 at 19:26, on Zulip):

@davidtwco did you measure mono items vs instantiations of them?

eddyb (Mar 31 2020 at 19:26, on Zulip):

something might be really weird with the CGU partitioning

eddyb (Mar 31 2020 at 19:26, on Zulip):

@davidtwco actually, more important is the patched incremental

davidtwco (Mar 31 2020 at 19:27, on Zulip):

was that what you mentioned with the multiple cgus on one line of the mono item output?

eddyb (Mar 31 2020 at 19:27, on Zulip):

(compile once with CARGO_INCREMENTAL=1 to force incremental even in release mode, make a change, then run the same command again)

eddyb (Mar 31 2020 at 19:27, on Zulip):

yeah

eddyb (Mar 31 2020 at 19:27, on Zulip):

but I just realized it's not the important thing I meant to point at here

eddyb (Mar 31 2020 at 19:27, on Zulip):

it's possible there are more mono items in the patched incremental build than there were before

eddyb (Mar 31 2020 at 19:28, on Zulip):

or something about the partitioning causes far more things to codegen

eddyb (Mar 31 2020 at 19:28, on Zulip):

@davidtwco RUSTC_LOG=info (the codegen_instance thing) might be more useful than mono items, I'm just realizing

eddyb (Mar 31 2020 at 19:28, on Zulip):

mono items might include stuff that doesn't need re-codegen-ing

eddyb (Mar 31 2020 at 19:29, on Zulip):

(I haven't done this sort of analysis, ever)

davidtwco (Mar 31 2020 at 19:35, on Zulip):

https://www.dropbox.com/sh/v0ffr9li5l0i75z/AABctvsXbAxMCUDZ1W98pDdPa?dl=0 - here's the mono item output from the script crate when I tested locally.

Polymorphisation
davidtwco (Mar 31 2020 at 19:35, on Zulip):

Just running cargo +$OLD clean && cargo +$OLD rustc -- -Z print-mono-items=lazy with the versions from perf, downloaded from the dev-static S3

davidtwco (Mar 31 2020 at 19:35, on Zulip):

and piping that into a file

davidtwco (Mar 31 2020 at 19:36, on Zulip):

I didn't try reproduce the clean, baseline, clean incr, patched stuff from perf when I counted mono items.

mark-i-m (Apr 01 2020 at 03:34, on Zulip):

Possibly a bad question, but how many items were we expecting could be polymorphic? i.e. what was the expected savings on the number of generated items? And are there any cases that we expected to be polymorphic that aren't?

eddyb (Apr 01 2020 at 14:28, on Zulip):

@mark-i-m it's almost entirely based on closures not using their parent fn's generics right now

eddyb (Apr 01 2020 at 14:29, on Zulip):

it's not transitive between fns nor taking layout into account

mark-i-m (Apr 01 2020 at 17:22, on Zulip):

@eddyb I see. Then would it make sense to do some sort of closure stress-test. i.e. create the best-case possible microbenchmark, something we expect to get gigantic improvements from polymorphization. Then we can see what the headroom is and where overhead is

Last update: Apr 05 2020 at 01:35UTC