Stream: t-compiler/meetings

Topic: [weekly meeting] 2020-07-30 #54818


Santiago Pastorino (Jul 29 2020 at 16:26, on Zulip):

Hi @T-compiler/meeting; the triage meeting will happen tomorrow at 2pm UTC

Santiago Pastorino (Jul 29 2020 at 16:26, on Zulip):

The @WG-prioritization have done pre-triage in #t-compiler/wg-prioritizationa/alerts

Santiago Pastorino (Jul 29 2020 at 16:29, on Zulip):

@WG-prioritization have prepared the meeting agenda

Santiago Pastorino (Jul 29 2020 at 16:29, on Zulip):

We will have checkins from @WG-polonius and WG-Polymorphization

Santiago Pastorino (Jul 29 2020 at 16:30, on Zulip):

@nikomatsakis and @lqd do you have something you want to share about @WG-polonius?

Santiago Pastorino (Jul 29 2020 at 16:30, on Zulip):

@davidtwco do you have something you want to share about WG-Polymorphization?

davidtwco (Jul 29 2020 at 16:31, on Zulip):

I’ll write something up for you, do I add it directly to the agenda?

lqd (Jul 29 2020 at 16:34, on Zulip):

@Santiago Pastorino not exactly, there's nothing to report apart that the WG will be doing a little focus "week" next week ! (Also I'm not sure we have enough activity to be in the check-ins rotation that often, as you've seen)

LeSeulArtichaut (Jul 29 2020 at 16:36, on Zulip):

@davidtwco I believe you should write it directly, yes

Santiago Pastorino (Jul 29 2020 at 18:57, on Zulip):

davidtwco said:

I’ll write something up for you, do I add it directly to the agenda?

yes, if you can't please give me your HackMD user and I'll give you access

Santiago Pastorino (Jul 29 2020 at 18:58, on Zulip):

lqd said:

Santiago Pastorino not exactly, there's nothing to report apart that the WG will be doing a little focus "week" next week ! (Also I'm not sure we have enough activity to be in the check-ins rotation that often, as you've seen)

ok, about checkins it's perfectly fine to skip

nikomatsakis (Jul 29 2020 at 19:25, on Zulip):

I added some content for wg-polonius, basically what @lqd said

Santiago Pastorino (Jul 30 2020 at 13:01, on Zulip):

Hi @T-compiler/meeting, triage meeting will be starting in ~ 59 minutes

Santiago Pastorino (Jul 30 2020 at 13:01, on Zulip):

Check out the meeting agenda

pnkfelix (Jul 30 2020 at 14:01, on Zulip):

Hi @T-compiler/meeting! Add a :wave: emoji to show you're here :)

pnkfelix (Jul 30 2020 at 14:01, on Zulip):

we will start off with 5 minutes for ...

Announcements

simulacrum (Jul 30 2020 at 14:02, on Zulip):

@Santiago Pastorino I think I forgot the T-compiler label, but could we add https://github.com/rust-lang/rust/pull/74682 to the agenda? (I can't seem to edit). It would be good to get to it today.

pnkfelix (Jul 30 2020 at 14:02, on Zulip):

I'll add it

Santiago Pastorino (Jul 30 2020 at 14:02, on Zulip):

@pnkfelix don't worry I can do it so you can continue

pnkfelix (Jul 30 2020 at 14:02, on Zulip):

I think its already sort of in there, as part of the performance reports

pnkfelix (Jul 30 2020 at 14:02, on Zulip):

(or I guess that was take one, and this is take two)

pnkfelix (Jul 30 2020 at 14:03, on Zulip):

anyway

pnkfelix (Jul 30 2020 at 14:03, on Zulip):
pnkfelix (Jul 30 2020 at 14:03, on Zulip):

(I assume that tomorrow's is a planning meeting)

pnkfelix (Jul 30 2020 at 14:04, on Zulip):

which means that people may want to check out https://github.com/rust-lang/compiler-team/issues?q=is%3Aopen+is%3Aissue+label%3Ameeting-proposal and perhaps add new proposals

pnkfelix (Jul 30 2020 at 14:04, on Zulip):
pnkfelix (Jul 30 2020 at 14:04, on Zulip):
pnkfelix (Jul 30 2020 at 14:05, on Zulip):
pnkfelix (Jul 30 2020 at 14:05, on Zulip):
pnkfelix (Jul 30 2020 at 14:05, on Zulip):
pnkfelix (Jul 30 2020 at 14:05, on Zulip):
Santiago Pastorino (Jul 30 2020 at 14:05, on Zulip):

note that the agenda is really packed, so @pnkfelix we may want to decide as we go to skip certain things ... if we really want to cover https://github.com/rust-lang/rust/pull/74682 which is the very last item of the agenda

pnkfelix (Jul 30 2020 at 14:06, on Zulip):
Wesley Wiser (Jul 30 2020 at 14:06, on Zulip):

Help would be greatly appreciated!

nikomatsakis (Jul 30 2020 at 14:06, on Zulip):

@pnkfelix can you put that mtg on the compiler team calendar, if you didn't already?

pnkfelix (Jul 30 2020 at 14:06, on Zulip):

The wg-incr-comp kickoff meeting will specifically be at

pnkfelix (Jul 30 2020 at 14:07, on Zulip):

! (fixed; my usage of <time didn't work at first)

pnkfelix (Jul 30 2020 at 14:07, on Zulip):

@nikomatsakis it should be on there already

nikomatsakis (Jul 30 2020 at 14:07, on Zulip):

I see it

pnkfelix (Jul 30 2020 at 14:08, on Zulip):

WG checkins

pnkfelix (Jul 30 2020 at 14:08, on Zulip):

@WG-polonius checkin by @nikomatsakis and @lqd:

We are planning a "sprint" Aug 3 - 5, which means that we'll be putting some energy into pushing polonius along. We've been changing to this "sprint-oriented" model because it's hard to keep a complex project like polonius moving in fits and starts, it requires some dedicated attention.

Last sprint we focused on getting a "complete write-up" of the polonius rules and in particular incorporating liveness and other things. I'm not sure what we'll focus on this sprint, probably similar themes, but I know that lqd has some ideas. --nikomatsakis

pnkfelix (Jul 30 2020 at 14:08, on Zulip):

WG-Polymorphization checkin by @davidtwco:

Since the last check-in from the polymorphisation working group (myself with lots of help from eddyb and lcnr), we’ve made a few steps forward and a few steps back:

Polymorphisation (#69749) was merged and enabled by default. Another perf run was performed before merge, which found that the improvements from earlier perf runs had vanished - some investigation revealed that this was due to an update to the script-servo benchmark that patched the opportunity that polymorphisation was taking advantage of.

We quickly discovered a regression that was missed by the test suite in #74614. Some investigation revealed the issue but we fixed the issue by disabling polymorphisation again in #74633. #74636 was filed to track fixing the underlying bug - there are some subtle implications of polymorphisation here that we hadn’t thought about and warrant some discussion.

#74717 has been submitted to fix that regression, but it’s still pending review and warrants some discussion before merge (particularly with respect to the intersection between polymorphisation and reflection).

lcnr fixed another bug with polymorphisation that was discovered while investigating the regression with #74623.

njn reported that disabling polymorphisation didn’t regain the performance losses of enabling it - I intend to look into why this might be soon.

pnkfelix (Jul 30 2020 at 14:09, on Zulip):

Beta-nominations

T-compiler

pnkfelix (Jul 30 2020 at 14:09, on Zulip):
pnkfelix (Jul 30 2020 at 14:10, on Zulip):

do we know why this was beta-nominated?

Santiago Pastorino (Jul 30 2020 at 14:10, on Zulip):

unknown from the comments but I guess it's to have support for that target as soon as possible?

Wesley Wiser (Jul 30 2020 at 14:10, on Zulip):

Is it beta-nominated so it ships by some date?

pnkfelix (Jul 30 2020 at 14:10, on Zulip):

this target is currently only available in the Apple DTK, right? Do we need to prioirtize getting that out in 12 weeks time?

bjorn3 (Jul 30 2020 at 14:10, on Zulip):

I think so the bootstrap compiler contains it earlier, making it easier to bootstrap for the target.

pnkfelix (Jul 30 2020 at 14:11, on Zulip):

oh I see. Hmm.

simulacrum (Jul 30 2020 at 14:11, on Zulip):

The goal is just to get bootstrap to work without custom hacks

simulacrum (Jul 30 2020 at 14:11, on Zulip):

imo, it's a low-risk patch -- essentially "tier 3" support at this point

pnkfelix (Jul 30 2020 at 14:11, on Zulip):

Okay I guess it seems fine

pnkfelix (Jul 30 2020 at 14:12, on Zulip):

I sort of wish we had a way to add bootstrap support without also adding even tier3 support to our stable system 6 weeks sooner than we really need.

pnkfelix (Jul 30 2020 at 14:12, on Zulip):

but it is low risk.

pnkfelix (Jul 30 2020 at 14:12, on Zulip):

anyway, beta-accepted

Wesley Wiser (Jul 30 2020 at 14:12, on Zulip):

lowrisc

pnkfelix (Jul 30 2020 at 14:12, on Zulip):
pnkfelix (Jul 30 2020 at 14:14, on Zulip):

This seems fine; @Vadim Petrochenkov did note that they were not sure if the affected targets were even usable on stable

pnkfelix (Jul 30 2020 at 14:15, on Zulip):

notably, none of the issues its tagged as fixing are themselves tagged as regression-stable-to-beta?

pnkfelix (Jul 30 2020 at 14:15, on Zulip):

anyway, I don't mind approving

pnkfelix (Jul 30 2020 at 14:15, on Zulip):

it just seemed a little weird

pnkfelix (Jul 30 2020 at 14:15, on Zulip):

beta-accepted

pnkfelix (Jul 30 2020 at 14:15, on Zulip):
pnkfelix (Jul 30 2020 at 14:16, on Zulip):

yeah seems like obvious beta-accept to me

pnkfelix (Jul 30 2020 at 14:17, on Zulip):

the detail about making sure that codegen & miri agree on how to make vtables like this seems important; @oli can you confirm that?

pnkfelix (Jul 30 2020 at 14:17, on Zulip):

beta-accepted

pnkfelix (Jul 30 2020 at 14:17, on Zulip):
nagisa (Jul 30 2020 at 14:18, on Zulip):

This one was nominated just now. Improves correctness of a warn-by-default lint.

nikomatsakis (Jul 30 2020 at 14:18, on Zulip):

fairly complex patch

nagisa (Jul 30 2020 at 14:18, on Zulip):

Fairly complex diff, so I don’t have a strong opinion on whether it should actually be backported.

pnkfelix (Jul 30 2020 at 14:19, on Zulip):

the bug itself was introduced by PR #70946

nagisa (Jul 30 2020 at 14:19, on Zulip):

it also touches code for the other important lint around whether types are C-safe in FFI APIs.

pnkfelix (Jul 30 2020 at 14:19, on Zulip):

and PR #70946 merged on June 21

pnkfelix (Jul 30 2020 at 14:20, on Zulip):

/me is trying to figure out if the lint breakage has hit stable yet or not

nagisa (Jul 30 2020 at 14:20, on Zulip):

its currently in beta I believe and not in stable.

simulacrum (Jul 30 2020 at 14:20, on Zulip):

We could just backport a switch of the lint to allow-by-default?

nagisa (Jul 30 2020 at 14:20, on Zulip):

A better backport could probably be yeah that

pnkfelix (Jul 30 2020 at 14:20, on Zulip):

l.ets do that

pnkfelix (Jul 30 2020 at 14:21, on Zulip):

beta-declined, with note that we would prefer to "just" backport a switch of lint to allow-by-default to beta.

pnkfelix (Jul 30 2020 at 14:21, on Zulip):

libs-impl

T-rustdoc

pnkfelix (Jul 30 2020 at 14:22, on Zulip):

Stable-nominations

T-compiler

pnkfelix (Jul 30 2020 at 14:22, on Zulip):
simulacrum (Jul 30 2020 at 14:22, on Zulip):

(Stable release is scheduled for today, btw)

pnkfelix (Jul 30 2020 at 14:22, on Zulip):

why is this stable-nominated and not beta-nominated?

pnkfelix (Jul 30 2020 at 14:23, on Zulip):

oh it already got into beta

pnkfelix (Jul 30 2020 at 14:23, on Zulip):

right?

simulacrum (Jul 30 2020 at 14:23, on Zulip):

yes

simulacrum (Jul 30 2020 at 14:23, on Zulip):

it missed the beta train last cycle by several hours

pnkfelix (Jul 30 2020 at 14:23, on Zulip):

so something went wrong in our tracking that we didn't nominate this sooner I take it?

Santiago Pastorino (Jul 30 2020 at 14:23, on Zulip):

simulacrum said:

(Stable release is scheduled for today, btw)

release is today?, I didn't see that on the rust releases calendar, will ping you in private to sync about this

simulacrum (Jul 30 2020 at 14:24, on Zulip):

@Santiago Pastorino we have a calendar?!

pnkfelix (Jul 30 2020 at 14:24, on Zulip):

if we stable approve this, would it just be enqueued for a future point release?

simulacrum (Jul 30 2020 at 14:24, on Zulip):

@pnkfelix sort of but let's not discuss now?

simulacrum (Jul 30 2020 at 14:24, on Zulip):

no
, it'll go out today

pnkfelix (Jul 30 2020 at 14:24, on Zulip):

oh okay

pnkfelix (Jul 30 2020 at 14:24, on Zulip):

Okay then, yeah, seems like obvious stable-accept then, I think.

Wesley Wiser (Jul 30 2020 at 14:25, on Zulip):

Yeah, I think when I initially r+'d it, it should have made the train but by the time it actually merged, it missed the window.

pnkfelix (Jul 30 2020 at 14:25, on Zulip):

stable-accepted

Wesley Wiser (Jul 30 2020 at 14:25, on Zulip):

I'm wondering if we should think about doing a quick review of all the PRs that land within ~24hr after beta branches to try to catch this kind of issue in the future.

Santiago Pastorino (Jul 30 2020 at 14:25, on Zulip):

simulacrum said:

Santiago Pastorino we have a calendar?!

well if you don't know it probably unofficial :), anyway let's discuss in private

pnkfelix (Jul 30 2020 at 14:26, on Zulip):
pnkfelix (Jul 30 2020 at 14:27, on Zulip):

yeah this also seems like a good one to stable-accept

pnkfelix (Jul 30 2020 at 14:27, on Zulip):

libs-impl

T-rustdoc

pnkfelix (Jul 30 2020 at 14:27, on Zulip):

PRs S-waiting-on-team

T-compiler

simulacrum (Jul 30 2020 at 14:27, on Zulip):

@Santiago Pastorino Could we make a note to ping me or T-release with stable noms? This could've been included today if we had known when it was posted or so

Santiago Pastorino (Jul 30 2020 at 14:28, on Zulip):

sorry, a note where?

pnkfelix (Jul 30 2020 at 14:29, on Zulip):

lets maybe make a separate topic, or private msg group maybe, to discuss how we'll modify things to try to avoid this situation in future?

simulacrum (Jul 30 2020 at 14:29, on Zulip):

:thumbs_up:

pnkfelix (Jul 30 2020 at 14:29, on Zulip):

(don't know if it needs to be held in private; PM me if you want the discussion to be private; otherwise I'll create a topic at the end of today's meeting for the discussion)

pnkfelix (Jul 30 2020 at 14:30, on Zulip):

so, back to

pnkfelix (Jul 30 2020 at 14:30, on Zulip):

PRs S-waiting-on-team

T-compiler

pnkfelix (Jul 30 2020 at 14:31, on Zulip):

seems like there is ongoing discussion of what best to do here

pnkfelix (Jul 30 2020 at 14:32, on Zulip):

the PR itself may or may not provide significant win in real world benchmarks. but more important to me is that further discussion may yield better variations of this idea?

pnkfelix (Jul 30 2020 at 14:32, on Zulip):

anyway I'm happy to let this remain in a waiting-on-team state

pnkfelix (Jul 30 2020 at 14:32, on Zulip):

but: We could also just land it

pnkfelix (Jul 30 2020 at 14:32, on Zulip):

and let future work be just that: future work

pnkfelix (Jul 30 2020 at 14:32, on Zulip):

any thoughts?

simulacrum (Jul 30 2020 at 14:33, on Zulip):

I'm in favor of landing, and if we find that the complexity makes things hard, we can always back it out in the future (e.g., with other work that tries different strategies)

pnkfelix (Jul 30 2020 at 14:33, on Zulip):

(i'd also be curious to do some research of the literature for related work here... this seems like a problem that a lot of compilers have had to deal wtih?)

pnkfelix (Jul 30 2020 at 14:34, on Zulip):

does anyone present want to argue against landing this? I know that @Vadim Petrochenkov commented:

I wish we didn't have to do this, it's just too damn ugly.
Yes, Span uses the same technique, but it at least doesn't have to provide functionality like as_str or is_*_keyword.

pnkfelix (Jul 30 2020 at 14:35, on Zulip):

is there anyone else in that camp, to the degree that they'd like to argue against landing this?

nikomatsakis (Jul 30 2020 at 14:36, on Zulip):

I haven't looked closely at the PR to see how much complexity it adds,

nikomatsakis (Jul 30 2020 at 14:36, on Zulip):

but I wouldn't argue against landing it.

pnkfelix (Jul 30 2020 at 14:36, on Zulip):

most notably, follow-up comments from @Vadim Petrochenkov suggest that we might be better off trying other ideas, like providing direct access to the string interner

nikomatsakis (Jul 30 2020 at 14:36, on Zulip):

Well.

nikomatsakis (Jul 30 2020 at 14:36, on Zulip):

The wins are pretty small :)

nikomatsakis (Jul 30 2020 at 14:36, on Zulip):

though widespread

pnkfelix (Jul 30 2020 at 14:36, on Zulip):

okay lets just land it. Like @simulacrum says, we can back it out later.

bjorn3 (Jul 30 2020 at 14:37, on Zulip):

Interestingly the objc runtime always packs small strings into the pointer value using pointer tagging.

pnkfelix (Jul 30 2020 at 14:37, on Zulip):

@nikomatsakis yeah, but maybe those small wins are indicative of problems that other approaches would yield even bigger wins?

nikomatsakis (Jul 30 2020 at 14:37, on Zulip):

As @njn wrote:

The performance results look decent but the biggest improvements all accrue to the shortest-running benchmarks, which are mostly artificial. By the time we get to real programs, such as futures, the improvements are ~0.5% or less.

pnkfelix (Jul 30 2020 at 14:37, on Zulip):

(but also, I don't know if ~0.5% is actually small.)

nikomatsakis (Jul 30 2020 at 14:38, on Zulip):

seems small to me

pnkfelix (Jul 30 2020 at 14:38, on Zulip):

@simulacrum you had noted that you wanted the implementation to be "somewhat abstract"

pnkfelix (Jul 30 2020 at 14:38, on Zulip):

was that an implicit request for a different PR?

simulacrum (Jul 30 2020 at 14:38, on Zulip):

no, I think this PR is pretty much there already

pnkfelix (Jul 30 2020 at 14:38, on Zulip):

okay then

pnkfelix (Jul 30 2020 at 14:38, on Zulip):

lets say its approved for landing

pnkfelix (Jul 30 2020 at 14:39, on Zulip):

and move along in this meeting

pnkfelix (Jul 30 2020 at 14:39, on Zulip):

libs-impl

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

I think the answer is that I wouldn't try to block it from landing

pnkfelix (Jul 30 2020 at 14:39, on Zulip):

Issues of Note

Short Summary

pnkfelix (Jul 30 2020 at 14:39, on Zulip):

P-critical

T-compiler

pnkfelix (Jul 30 2020 at 14:39, on Zulip):
pnkfelix (Jul 30 2020 at 14:40, on Zulip):

My intent had been to make sure an LLVM bug was filed

pnkfelix (Jul 30 2020 at 14:41, on Zulip):

but I haven't done that yet

pnkfelix (Jul 30 2020 at 14:43, on Zulip):

I don't have anything else to report there. Its probably not useful to try to revert the Range::next change that @Luqman Aden identified as being the source of LLVM's woes here

pnkfelix (Jul 30 2020 at 14:44, on Zulip):
pnkfelix (Jul 30 2020 at 14:45, on Zulip):

hmm. Lots of questions here.

pnkfelix (Jul 30 2020 at 14:46, on Zulip):

I'll self-assign to at least assist with getting an MCVE

nikomatsakis (Jul 30 2020 at 14:47, on Zulip):

hmm

pnkfelix (Jul 30 2020 at 14:47, on Zulip):
nikomatsakis (Jul 30 2020 at 14:48, on Zulip):

/me sighs

pnkfelix (Jul 30 2020 at 14:48, on Zulip):

I haven't studied the example in rust#74933

nikomatsakis (Jul 30 2020 at 14:48, on Zulip):

this is why we can't have nice things :)

pnkfelix (Jul 30 2020 at 14:48, on Zulip):

is it possible that this code should never have compiled?

pnkfelix (Jul 30 2020 at 14:48, on Zulip):

/me looks

nikomatsakis (Jul 30 2020 at 14:49, on Zulip):

I haven't studied it enough yet

nikomatsakis (Jul 30 2020 at 14:49, on Zulip):

not a great error msg

pnkfelix (Jul 30 2020 at 14:49, on Zulip):

diagnostic is definitely subpar

pnkfelix (Jul 30 2020 at 14:49, on Zulip):

"these two types" ==> "what two types? You highlighted one..."

nikomatsakis (Jul 30 2020 at 14:50, on Zulip):

(random question, do we have any stats for how often bisection yields rollups?)

pnkfelix (Jul 30 2020 at 14:50, on Zulip):

and yeah, "data from field flows into field here...)

pnkfelix (Jul 30 2020 at 14:50, on Zulip):

nikomatsakis said:

(random question, do we have any stats for how often bisection yields rollups?)

we can probably get that data from the cut-and-pasted cargo-bisect-rustc reports

pnkfelix (Jul 30 2020 at 14:50, on Zulip):

as in, by scaping those comments

nikomatsakis (Jul 30 2020 at 14:50, on Zulip):

anyway if I had to guess

pnkfelix (Jul 30 2020 at 14:50, on Zulip):

that might be an interesting exercise

Santiago Pastorino (Jul 30 2020 at 14:51, on Zulip):

nikomatsakis said:

(random question, do we have any stats for how often bisection yields rollups?)

pretty often :)

nikomatsakis (Jul 30 2020 at 14:51, on Zulip):

somehow #72280 probably removed an instance of subtyping

nikomatsakis (Jul 30 2020 at 14:51, on Zulip):

such that &'a Foo<'b> is now being equated with &'c Foo<'c>

nikomatsakis (Jul 30 2020 at 14:51, on Zulip):

feels like it could happen

pnkfelix (Jul 30 2020 at 14:51, on Zulip):

is it &'c Foo<'c> ?

pnkfelix (Jul 30 2020 at 14:51, on Zulip):

or is it &'b Foo<'c> ?

pnkfelix (Jul 30 2020 at 14:52, on Zulip):

the declared type is &Field<'_>; is the inner '_ going to be the same as the lifetime attached to the &?

pnkfelix (Jul 30 2020 at 14:52, on Zulip):

oh wait maybe I misread your message

pnkfelix (Jul 30 2020 at 14:52, on Zulip):

thought you wrote &'a Foo<'a>

pnkfelix (Jul 30 2020 at 14:53, on Zulip):

because that's what is in the impl s of Index and IndexMut

pnkfelix (Jul 30 2020 at 14:53, on Zulip):

anyway

pnkfelix (Jul 30 2020 at 14:53, on Zulip):

we are officially in the weeds here

pnkfelix (Jul 30 2020 at 14:53, on Zulip):

@nikomatsakis can you take point on this issue?

nikomatsakis (Jul 30 2020 at 14:53, on Zulip):

well

pnkfelix (Jul 30 2020 at 14:53, on Zulip):

i.e. can I assign #74933 to you niko?

nikomatsakis (Jul 30 2020 at 14:54, on Zulip):

go ahead for now

pnkfelix (Jul 30 2020 at 14:54, on Zulip):

okay

nikomatsakis (Jul 30 2020 at 14:54, on Zulip):

I am hesitating beacuse I know I have pretty limited time over next few weeks

nikomatsakis (Jul 30 2020 at 14:54, on Zulip):

but I should be able to do a bit of digging

pnkfelix (Jul 30 2020 at 14:54, on Zulip):

libs-impl

T-rustdoc

pnkfelix (Jul 30 2020 at 14:55, on Zulip):

OMG we still haven't gotten to the performance report stuff

pnkfelix (Jul 30 2020 at 14:55, on Zulip):

okay I'm skipping the unassigned P-high regressions

pnkfelix (Jul 30 2020 at 14:55, on Zulip):

we can't skip this for two weeks in a row

pnkfelix (Jul 30 2020 at 14:55, on Zulip):

Performance logs

pnkfelix (Jul 30 2020 at 14:55, on Zulip):

what we wrote on the agenda was that we should revisit last weeks' report

pnkfelix (Jul 30 2020 at 14:55, on Zulip):

and we should

pnkfelix (Jul 30 2020 at 14:56, on Zulip):

but we should also keep in mind that some of the regressions from last week were addressed this week

pnkfelix (Jul 30 2020 at 14:56, on Zulip):

so lets see

pnkfelix (Jul 30 2020 at 14:56, on Zulip):

so what I'll do

pnkfelix (Jul 30 2020 at 14:56, on Zulip):

is paste both reports

pnkfelix (Jul 30 2020 at 14:56, on Zulip):

one after another

pnkfelix (Jul 30 2020 at 14:56, on Zulip):

and we can try to discuss together

pnkfelix (Jul 30 2020 at 14:57, on Zulip):

and this is where we may also want to discuss that PR that @simulacrum mentioned but I cannot recall the PR # for at the moment and cannot take time to scroll up to find it

simulacrum (Jul 30 2020 at 14:57, on Zulip):

https://github.com/rust-lang/rust/pull/74682

pnkfelix (Jul 30 2020 at 14:57, on Zulip):

Last week

pnkfelix (Jul 30 2020 at 14:57, on Zulip):

Triage done by njn

This week was a disaster, perf-wise. 28 revisions checked. 7 regressions, several of them ranging from large to huge, many in rollups. Some additional regressions may have occurred in rollups that were masked by other regressions/improvements. 3 improvements, one of which was a reversion of a regression. Thanks for Mark-Simulacrum and eddyb for follow-up measurements and adding new tooling to help investigate regressions in rollups. A follow-up thread on Zulip is here.

In better news, rustdoc performance is now being benchmarked, thanks to the
efforts of Joshua Nelson.

Triage done by njn. Revision range: 9d09331e00b02f81c714b0c41ce3a38380dd36a2..71384101ea3b030b80f7def80a37f67e148518b0.

Regressions

Improvements

Notes

Rollup of 11 pull requests #74468

This rollup was originally judged as responsible for a 10% regression in
instrutions:u. However, since then, it has been determined that the likely cause of this regression is actually perf's move to using x.py dist-shipped std's rather than building one locally. Investigation into why this move caused a regression is as yet not done, but is being tracked in rustc-perf#724.

Initially #74069 and/or #72414 were suspected as the cause of the regression, but further testing showed that to not be the case.

We have since opened a PR to re-land #74069, as well: #74802.

pnkfelix (Jul 30 2020 at 14:57, on Zulip):

This week

pnkfelix (Jul 30 2020 at 14:57, on Zulip):

Triage done by njn. Revision range: 71384101ea3b030b80f7def80a37f67e148518b0..efc02b03d18b0cbaa55b1e421d792f70a39230b2

2 regressions, 1 improvement, none of them on rollups.

Regressions

Improvements

pnkfelix (Jul 30 2020 at 14:58, on Zulip):

So, my impression from reading the two reports

pnkfelix (Jul 30 2020 at 14:58, on Zulip):

is that the two most significant sources of regression from last week

pnkfelix (Jul 30 2020 at 14:58, on Zulip):

have now been addressed by reverting their associated PR's

simulacrum (Jul 30 2020 at 14:58, on Zulip):

Correct.

pnkfelix (Jul 30 2020 at 14:59, on Zulip):

so that may lead to conclusion that the two weeks combined are a wash. Well, maybe not a wash, there are still some regressions

pnkfelix (Jul 30 2020 at 15:00, on Zulip):

Idea for @njn if you aren't already doing this: In a situation like this, where we have such a bad week followed by a flurry of reverts ...

pnkfelix (Jul 30 2020 at 15:00, on Zulip):

... it would probably be good for the performance report to include both the data for comparing the last week and for comparing the last two weeks, right?

pnkfelix (Jul 30 2020 at 15:01, on Zulip):

(Maybe that's just adding extra effort when you are already making note of the improvements and tying them to the reversion PRs....)

pnkfelix (Jul 30 2020 at 15:01, on Zulip):

anyway, lets maybe focus for moment on PR #74682

pnkfelix (Jul 30 2020 at 15:01, on Zulip):

obviously meeting time is up. but I figure the people who can stick around can voice opinions on this

simulacrum (Jul 30 2020 at 15:02, on Zulip):

This re-lands the gimli change, and is a clear regression (up to 20%). However, it is also essentially a soundness fix -- libbacktrace has been a long standing "black hole" of code we know we can't trust but didn't have much choice about. Now we do :)

pnkfelix (Jul 30 2020 at 15:02, on Zulip):

namely this comment from @Alex Crichton :

Is it possible to bump this sooner on the agenda? It was intended to land this just after the beta branch to give this maximal testing on nightly to weed out unexpected regressions. Landing after August 6th delays that by 3 weeks and halves the amount of nightly testing this is going to get.

I'll reiterate I'm more than happy to revert this if the decision is that it shouldn't land. Given though that this is highly likely to land I continue to not understand the very strong reluctance to put this in tree.

simulacrum (Jul 30 2020 at 15:02, on Zulip):

That said, the wall-clock regressions are tiny -- a few milliseconds at most -- and all of the regressions come from crate loading.

nikomatsakis (Jul 30 2020 at 15:02, on Zulip):

do we have any sense of whether that 20% affects only "small" tests or larger ones too?

simulacrum (Jul 30 2020 at 15:03, on Zulip):

Just small ones

nikomatsakis (Jul 30 2020 at 15:03, on Zulip):

when we were discussing parallelization

nikomatsakis (Jul 30 2020 at 15:03, on Zulip):

we had a (semi-arbitary) threshold

simulacrum (Jul 30 2020 at 15:03, on Zulip):

The regressions are largely due to the (quite problematic) point of the trait evaluation code loading all structs in the universe

nikomatsakis (Jul 30 2020 at 15:03, on Zulip):

I kind of forget what it was, but I think it was something like >nms and >n%

nikomatsakis (Jul 30 2020 at 15:03, on Zulip):

or maybe it was "or" :)

simulacrum (Jul 30 2020 at 15:03, on Zulip):

It doesn't fit any bar on the >nms side

simulacrum (Jul 30 2020 at 15:04, on Zulip):

IMO, we should land this -- it's a clear regression, but plausibly a fixable one, and it seems like a clear win functionality wise

simulacrum (Jul 30 2020 at 15:04, on Zulip):

we've already heard from folks outside the rust teams that having a fully Rust libstd is really nice

nikomatsakis (Jul 30 2020 at 15:04, on Zulip):

yeah, I think we should land it too

nikomatsakis (Jul 30 2020 at 15:04, on Zulip):

shame about the hit

nikomatsakis (Jul 30 2020 at 15:05, on Zulip):

simulacrum said:

The regressions are largely due to the (quite problematic) point of the trait evaluation code loading all structs in the universe

I'd like to hear a bit more about this :)

nikomatsakis (Jul 30 2020 at 15:05, on Zulip):

(but not during this mtg presumably)

nikomatsakis (Jul 30 2020 at 15:05, on Zulip):

I guess you have some zulip links...

pnkfelix (Jul 30 2020 at 15:05, on Zulip):

So the main benefit here

pnkfelix (Jul 30 2020 at 15:05, on Zulip):

is not just fixing backtracing for ICE's on windows-gnu

simulacrum (Jul 30 2020 at 15:05, on Zulip):

we've historically had random segfaults and such from libbacktrace

pnkfelix (Jul 30 2020 at 15:05, on Zulip):

but rather the improvement of having Rust-based backtracing for others?

pnkfelix (Jul 30 2020 at 15:05, on Zulip):

hmm okay

simulacrum (Jul 30 2020 at 15:06, on Zulip):

which we've sort of fixed but usually been unable to upstream effectively

simulacrum (Jul 30 2020 at 15:06, on Zulip):

and as a large pile of C code, it's not like we expect those to be finished even after 5+ years of usage

simulacrum (Jul 30 2020 at 15:07, on Zulip):

This also opens the door to things like split-dwarf

simulacrum (Jul 30 2020 at 15:07, on Zulip):

which I expect to be a really nice win (e.g., we could ship debuginfo for the compiler and such then)

pnkfelix (Jul 30 2020 at 15:07, on Zulip):

Actually I'm with niko here: are there links to zulip conversations about this and the other PR, and the regressions associated with them, etc?

simulacrum (Jul 30 2020 at 15:08, on Zulip):

In terms of the investigation done into the causes of the regression?

nikomatsakis (Jul 30 2020 at 15:08, on Zulip):

yeah, I think the motivation for me is that I know that libbacktrace has come up numerous times as a thorn in the side of various plans

pnkfelix (Jul 30 2020 at 15:08, on Zulip):

I figure the best thing would be to provide links to those, if they exist, and gnerally just move forward on landing this PR

nikomatsakis (Jul 30 2020 at 15:08, on Zulip):

so having some control over that codebase I feel like is going to be a boon long term

nikomatsakis (Jul 30 2020 at 15:08, on Zulip):

though I admit this is kind of me pattern matching vague memories and not having precise ideas

simulacrum (Jul 30 2020 at 15:08, on Zulip):

https://github.com/rust-lang/rust/pull/74682#issuecomment-665376022 is a cachegrind instruction diff

simulacrum (Jul 30 2020 at 15:08, on Zulip):

there's not been a ton of conversation here yet

pnkfelix (Jul 30 2020 at 15:08, on Zulip):

also I'm trying to understand this comment from alex: "I'll reiterate I'm more than happy to revert this if the decision is that it shouldn't land. Given though that this is highly likely to land I continue to not understand the very strong reluctance to put this in tree."

pnkfelix (Jul 30 2020 at 15:09, on Zulip):

that is, I just don't know how to parse "X is highly likely to land && there is reluctance to put X in tree"

pnkfelix (Jul 30 2020 at 15:09, on Zulip):

unless two occurrences of the word "this" is referring to different things in alex's remark?

simulacrum (Jul 30 2020 at 15:10, on Zulip):

no, they're the same

simulacrum (Jul 30 2020 at 15:10, on Zulip):

the reluctance is due to performance though

simulacrum (Jul 30 2020 at 15:11, on Zulip):

and Alex believes (rightly, I think) that the performance is sort of a "fundamental" problem with how rustc currently views the universe of crates -- i.e., dependencies have a (relatively small) cost even if largely unused, it's not viable for us to block on fixing that

pnkfelix (Jul 30 2020 at 15:11, on Zulip):

so alex's remark is, more simply stated, "resistance is futile" ? (I'm making a big joke here, I know alex isn't putting up that kind of attitude)

pnkfelix (Jul 30 2020 at 15:12, on Zulip):

okay

simulacrum (Jul 30 2020 at 15:12, on Zulip):

basically "let's get this in tree for functionality benefits and ecosystem testing ASAP, then we can fix performance later"

simulacrum (Jul 30 2020 at 15:12, on Zulip):

well, maybe fix performance -- there's not a "known path" of course, or we would've done it :)

pnkfelix (Jul 30 2020 at 15:12, on Zulip):

right

pnkfelix (Jul 30 2020 at 15:13, on Zulip):

okay well it sounds like everyone present is willing to accept the tradeoffs here

pnkfelix (Jul 30 2020 at 15:13, on Zulip):

So yeah, go ahead and land it

pnkfelix (Jul 30 2020 at 15:13, on Zulip):

and with that, I'm going to call this meeting over.

pnkfelix (Jul 30 2020 at 15:13, on Zulip):

Thanks to everyone in @T-compiler/meeting for attending! Sorry for running way over time today.

simulacrum (Jul 30 2020 at 15:14, on Zulip):

I can write up the comment summarizing this decision

pnkfelix (Jul 30 2020 at 15:14, on Zulip):

thanks @simulacrum

Last update: Nov 25 2020 at 02:45UTC