Stream: t-compiler

Topic: weekly meeting 2019-03-28 #54818


pnkfelix (Mar 28 2019 at 12:57, on Zulip):

Hi @T-compiler/meeting ; our weekly meeting, held in this topic area, will be started in about 63 minutes.

pnkfelix (Mar 28 2019 at 12:58, on Zulip):

In the meantime I will be doing weekly triage in a parallel topic

pnkfelix (Mar 28 2019 at 12:59, on Zulip):

The schedule for WG-checkin says that this week we might be hearing from wg-parallel-rustc and wg-pgo

pnkfelix (Mar 28 2019 at 12:59, on Zulip):

@Zoxc or @mw : is one of you prepared to present for wg-parallel-rustc in about 90 minutes or so?

pnkfelix (Mar 28 2019 at 13:00, on Zulip):

and @mw , you are listed as the context for wg-pgo, so maybe you'll be doing both presentations? Or maybe you plan to delegate to @Zoxc ?

pnkfelix (Mar 28 2019 at 13:00, on Zulip):

(Just let me know so that I block off time accordingly. I know it is short notice; i should have sent out a note on Tuesday, but I forgot.)

mw (Mar 28 2019 at 13:10, on Zulip):

I can do a little update on PGO

mw (Mar 28 2019 at 13:11, on Zulip):

I'm feeling a bit disorganized around wg-parallel-rustc

pnkfelix (Mar 28 2019 at 13:12, on Zulip):

okay. Do you think that @Zoxc would have more to say? Or should we not bother with a wg-parallel-rustc check-in this week then?

mw (Mar 28 2019 at 13:13, on Zulip):

well, if @Zoxc would like to give an update, I'd certainly be interested. But if not, it's better to do it another time

centril (Mar 28 2019 at 13:13, on Zulip):

PSA to T-compiler -- if you r+ a PR someone else is assigned to, please r? yourself as well

pnkfelix (Mar 28 2019 at 13:14, on Zulip):

PSA to T-compiler -- if you r+ a PR someone else is assigned to, please r? yourself as well

fair enough. (for context for others: this is regarding what I did on PR #59468)

centril (Mar 28 2019 at 13:14, on Zulip):

@pnkfelix yes but it happens a lot ;)

pnkfelix (Mar 28 2019 at 13:15, on Zulip):

maybe the bot should do it

pnkfelix (Mar 28 2019 at 13:15, on Zulip):

its an interesting policy question, whether that should be automatic or not

centril (Mar 28 2019 at 13:15, on Zulip):

yeah that would be nice; ^ @Pietro Albini

pnkfelix (Mar 28 2019 at 13:15, on Zulip):

if one steals review action, it certainly seems like one should take on the burden of assignment as well.

centril (Mar 28 2019 at 13:16, on Zulip):

I do r? @Centril @bors r+ rollup often these days ;)

Pietro Albini (Mar 28 2019 at 13:17, on Zulip):

if/when you decide that's something you want open an issue on https://github.com/rust-lang/infra-team

pnkfelix (Mar 28 2019 at 13:19, on Zulip):

okay, I'll set up an emoji poll for (the benefit for those on mobile where normal poll feature does not work.)

centril (Mar 28 2019 at 13:20, on Zulip):

@pnkfelix I'll create a GH issue tagged with T-compiler, T-infra, T-release, T-libs, T-lang and rfcbot merge it

pnkfelix (Mar 28 2019 at 13:20, on Zulip):

Should @bors r+ cause bors to also r? <yourself>, which implies the issue is assigned to you? :thumbs_up: or :thumbs_down: or :shrug: based on your immediate opinion ( :shrug: is effectively "abstain").

centril (Mar 28 2019 at 13:20, on Zulip):

(since this is a matter for more than t-compiler)

pnkfelix (Mar 28 2019 at 13:21, on Zulip):

true. So we'll see how people react, but I'll try not to devote too much time to it since it is a cross-team-spanning concern.

centril (Mar 28 2019 at 13:21, on Zulip):

cool

pnkfelix (Mar 28 2019 at 13:26, on Zulip):

:construction_worker: seeking (or "sought") volunteer for "Future-incompatible warnings should always print a warning, even if lints are allowed" #34596

pnkfelix (Mar 28 2019 at 13:27, on Zulip):

(to be clear, @centril did say they would look at #34596. But I'm just following a protocol here to increase visibility of the work item)

Esteban Küber (Mar 28 2019 at 13:32, on Zulip):

That's accomplished by making them warnings and not lints

Esteban Küber (Mar 28 2019 at 13:33, on Zulip):

And I personally would expect that silencing a lint that is warning me about deprecation will eventually lead to my code not compiling in a subsequent version... Of course that gets hairy with libs hiding these lint warnings from their downstreams

centril (Mar 28 2019 at 13:34, on Zulip):

@Esteban Küber I think you should be able to deny(...) them so keeping them as lints makes sense

centril (Mar 28 2019 at 13:34, on Zulip):

rather, we should be able to designate lints as future-compat ones and assign a cap (e.g. "no more liberal than Warn")

Esteban Küber (Mar 28 2019 at 13:42, on Zulip):

Fair point

Esteban Küber (Mar 28 2019 at 13:43, on Zulip):

Disregard my digression, the implementing the suggested behavior isn't that much work and it is clearly a better situation than either of the current options

pnkfelix (Mar 28 2019 at 13:58, on Zulip):

:alert: Re: "beta 1.33 seems to break tarpaulin on multithreading" #58104 -- I'm planning to close this as non-actionable, as I suggested doing back when I nominated it four weeks ago.

pnkfelix (Mar 28 2019 at 14:01, on Zulip):

hi @T-compiler/meeting

pnkfelix (Mar 28 2019 at 14:02, on Zulip):

I've been doing triage in the parallel topic

pnkfelix (Mar 28 2019 at 14:02, on Zulip):

I actually think it went better than much in week's past. Still didn't get through every P-high issue, but I got through a lot

pnkfelix (Mar 28 2019 at 14:03, on Zulip):

there were ten issues left, eight of them are assigned. so lets just collectively double-check the two that were unassigned

pnkfelix (Mar 28 2019 at 14:03, on Zulip):

first: "ripgrep fails to build with MUSL on Linux since the nightly release on 2019-03-15" #59411

pnkfelix (Mar 28 2019 at 14:03, on Zulip):

this is new, and I only assigned it P-high duing a triage pre-pass

pnkfelix (Mar 28 2019 at 14:04, on Zulip):

it also has a PR (#59468). I guess I might try to assign it to the author of the PR ... and if that fails, I'll assign it to myself

pnkfelix (Mar 28 2019 at 14:05, on Zulip):

second unassigned P-high issue: "Wrapping simd call results in compiler panic (nightly)" #59469

pnkfelix (Mar 28 2019 at 14:06, on Zulip):

this is also new

pnkfelix (Mar 28 2019 at 14:06, on Zulip):

(like, less than a day old)

pnkfelix (Mar 28 2019 at 14:06, on Zulip):

so I won't panic about assigning it to someone yet. But if someone wants to take it now, feel free.

pnkfelix (Mar 28 2019 at 14:06, on Zulip):

okay, that's all the P-high's I care to discuss.

pnkfelix (Mar 28 2019 at 14:07, on Zulip):

next, beta-noms marked T-compiler

pnkfelix (Mar 28 2019 at 14:07, on Zulip):

namely: "Fix fallout from #57667" #58021

pnkfelix (Mar 28 2019 at 14:07, on Zulip):

not my favorite PR description ever. (the joke is that no description was given by PR author. I'll put one in.)

pnkfelix (Mar 28 2019 at 14:08, on Zulip):

(especially since github issue titles don't do the hyperlinks)

nikomatsakis (Mar 28 2019 at 14:10, on Zulip):

there was some issue that I tried to reproduce last week but failed

nikomatsakis (Mar 28 2019 at 14:11, on Zulip):

it was one of those "requesting volunteer" issues I thiink...

varkor (Mar 28 2019 at 14:11, on Zulip):

(especially since github issue titles don't do the hyperlinks)

(give refined GitHub a try)

nikomatsakis (Mar 28 2019 at 14:11, on Zulip):

ah well no matter, I guess you took a look during pre-triage

pnkfelix (Mar 28 2019 at 14:12, on Zulip):

sorry for my silence, I was reviewing the comment thread on PR #58021

pnkfelix (Mar 28 2019 at 14:12, on Zulip):

but now I've managed to confuse myself

pnkfelix (Mar 28 2019 at 14:12, on Zulip):

since the comment thread prepared me for a really complicated PR

pnkfelix (Mar 28 2019 at 14:13, on Zulip):

but the actual diff that was settled on looks ... very simple?

oli (Mar 28 2019 at 14:14, on Zulip):

yea I think everyone realized that Box is more magical than everyone was aware of, and everything went well from there

pnkfelix (Mar 28 2019 at 14:14, on Zulip):

I guess @eddyb even asked @nikomatsakis and me to explain when box became magical enough to do this: https://github.com/rust-lang/rust/pull/58021#discussion_r265109175

pnkfelix (Mar 28 2019 at 14:14, on Zulip):

is there any chance that the point where box became "sufficiently magical" actually postdates the current beta?

pnkfelix (Mar 28 2019 at 14:15, on Zulip):

I think the probability of that is about 0.00001%

pnkfelix (Mar 28 2019 at 14:15, on Zulip):

is there any chance that the point where box became "sufficiently magical" actually postdates the current beta?

(because this would be the only reason I would see to not approve a beta backport here, I think)

nikomatsakis (Mar 28 2019 at 14:15, on Zulip):

I would guess that this fell our from NLL

nikomatsakis (Mar 28 2019 at 14:15, on Zulip):

We made a decision to stop artificially hobbling boxes in the analysis

pnkfelix (Mar 28 2019 at 14:16, on Zulip):

okay and the code in question is somewhere in libstd that has NLL enabled?

nikomatsakis (Mar 28 2019 at 14:16, on Zulip):

that is my guess

pnkfelix (Mar 28 2019 at 14:16, on Zulip):

I'd also like to know what bugs were observed from the orginal mistake

pnkfelix (Mar 28 2019 at 14:17, on Zulip):

i.e. it certainly sounds like the code being fixed is doing a double-drop, right?

pnkfelix (Mar 28 2019 at 14:17, on Zulip):

(Why was there no regression test added as part of this?)

pnkfelix (Mar 28 2019 at 14:17, on Zulip):

:frown:

nikomatsakis (Mar 28 2019 at 14:17, on Zulip):

That's what I understood from @eddyb's comment

pnkfelix (Mar 28 2019 at 14:18, on Zulip):

okay, well... what's the current beta schedule? When's the next beta being cut?

nikomatsakis (Mar 28 2019 at 14:18, on Zulip):

(Why was there no regression test added as part of this?)

oversight I guess

pnkfelix (Mar 28 2019 at 14:19, on Zulip):

because I would personally like to get the chance to write a regression test for this bug and just double check that against stable + beta + nightly

pnkfelix (Mar 28 2019 at 14:19, on Zulip):

and while I don't want to prevent this from getting into stable in a timely fashion as part of the natural train flow

pnkfelix (Mar 28 2019 at 14:20, on Zulip):

if the beta is being cut sometime after next week's T-compiler meeitng

pnkfelix (Mar 28 2019 at 14:20, on Zulip):

then I'd prefer to delay approval of beta-backporting this

pnkfelix (Mar 28 2019 at 14:20, on Zulip):

I guess that may be silly of me

pnkfelix (Mar 28 2019 at 14:20, on Zulip):

perhaps better to just go ahead and approve backport

pnkfelix (Mar 28 2019 at 14:20, on Zulip):

and do my investigation in parallel?

pnkfelix (Mar 28 2019 at 14:20, on Zulip):

Let me go ahead and put up an emoji poll

pnkfelix (Mar 28 2019 at 14:21, on Zulip):

Q: Should we beta-backport "Fix fallout from #57667" #58021

pnkfelix (Mar 28 2019 at 14:22, on Zulip):

In other issues: We still have the long-standing "[do not merge] Measure performance impact of local interners" #57214 waiting on our team

pnkfelix (Mar 28 2019 at 14:23, on Zulip):

(SNL has a skit about a talk show where they always run out of time before they talk to their third guest... who is always the same character...)

pnkfelix (Mar 28 2019 at 14:23, on Zulip):

there has at least been discussion on the issue itself, that is good

nikomatsakis (Mar 28 2019 at 14:24, on Zulip):

In other issues: We still have the long-standing "[do not merge] Measure performance impact of local interners" #57214 waiting on our team

maybe we should schedule a time to talk about this?

pnkfelix (Mar 28 2019 at 14:24, on Zulip):

or about memory usage in general

nikomatsakis (Mar 28 2019 at 14:24, on Zulip):

and assign someone to lead and prep the discussion?

nikomatsakis (Mar 28 2019 at 14:24, on Zulip):

yeah I mean it would be good to figure out what we are blocked on precisely

nikomatsakis (Mar 28 2019 at 14:25, on Zulip):

my sense is that it's a bit of "uncertainty about how to proceed" more than anything

nikomatsakis (Mar 28 2019 at 14:25, on Zulip):

I guess that the concerns @nagisa raised were mostly kind of "memory usage is too high, let's not regress, even a bit"?

pnkfelix (Mar 28 2019 at 14:25, on Zulip):

:construction_worker: seeking volunteer to schedule and lead a meeting for T-compiler to discuss #57214, and perhaps digress into related issues of how to address rustc memory usage.

mw (Mar 28 2019 at 14:25, on Zulip):

I have the feeling that everyone wants to do this but there's a vague fear of using too much memory

mw (Mar 28 2019 at 14:26, on Zulip):

and being so vague there's no actual way to overcome it

pnkfelix (Mar 28 2019 at 14:26, on Zulip):

I do really like @Zoxc 's response there: lets identify simpler ways to keep ourselves under a memory budget.

pnkfelix (Mar 28 2019 at 14:26, on Zulip):

anyway, I'm happy the issue PR got some more visibility

nikomatsakis (Mar 28 2019 at 14:26, on Zulip):

Right. So I see a few outcomes:

pnkfelix (Mar 28 2019 at 14:27, on Zulip):

so we have eight nominated T-compiler issues

nikomatsakis (Mar 28 2019 at 14:27, on Zulip):

I agree with @Zoxc as well -- I don't thik it's "fair" to "blame" this change for our high memory usage per se, but I also agree with @nagisa that it is perhaps a problem worth attacking.

nikomatsakis (Mar 28 2019 at 14:27, on Zulip):

/me will leave some comments

pnkfelix (Mar 28 2019 at 14:28, on Zulip):

I think based on @mw 's earlier comments that we are only going to hear from one working-group today, not two

mw (Mar 28 2019 at 14:28, on Zulip):

https://perf.rust-lang.org/compare.html?start=aeed63bf383e46bbaf932f4b555697ddb8328d89&end=2eb7307a44bc76a18eeb0c4d2947492b2be4cbf2&stat=max-rss

mw (Mar 28 2019 at 14:28, on Zulip):

this is data on the regression, right?

mw (Mar 28 2019 at 14:28, on Zulip):

yes, I'll give an update on PGO

pnkfelix (Mar 28 2019 at 14:29, on Zulip):

okay lets do that update now

mw (Mar 28 2019 at 14:29, on Zulip):

alright

pnkfelix (Mar 28 2019 at 14:29, on Zulip):

and then we'll spend the time afterthe update on nominated issues

pnkfelix (Mar 28 2019 at 14:29, on Zulip):

since that can balloon

mw (Mar 28 2019 at 14:29, on Zulip):

so PGO stands for "profile-guided optimization" and, in general, works first collection data about a programs typical execution

mw (Mar 28 2019 at 14:29, on Zulip):

(e.g. which branches it is likely to take)

mw (Mar 28 2019 at 14:29, on Zulip):

and then using that data to inform optimizations such as inlining, machine-code layout (for better i-cache behavior), register allocation, etc.

mw (Mar 28 2019 at 14:30, on Zulip):

it's often considered an important optimization

mw (Mar 28 2019 at 14:30, on Zulip):

there are different ways of collecting data about a program.

mw (Mar 28 2019 at 14:30, on Zulip):

one is to run the program inside a profiler, such as perf.

mw (Mar 28 2019 at 14:30, on Zulip):

another is to create an instrumented binary that has data collection built right into it.

mw (Mar 28 2019 at 14:30, on Zulip):

the latter usually provides more accurate data.

mw (Mar 28 2019 at 14:30, on Zulip):

LLVM supports both approaches and for the instrumented approach it support 3 (sic!) variations:

mw (Mar 28 2019 at 14:30, on Zulip):
mw (Mar 28 2019 at 14:30, on Zulip):
mw (Mar 28 2019 at 14:31, on Zulip):
mw (Mar 28 2019 at 14:31, on Zulip):

rustc has experimental support for IR-level instrumentation, which is both the easiest to support (because LLVM does most of the work) and is also the more "modern" approach.

mw (Mar 28 2019 at 14:31, on Zulip):

I don't know if we ever want to support FE-level instrumentation or GCOV.

mw (Mar 28 2019 at 14:31, on Zulip):

so far, the work of this working group (of which I am the only member) has been to find out about the stuff described above

mw (Mar 28 2019 at 14:31, on Zulip):

how it all works (roughly) at the level of LLVM and compiler-rt

mw (Mar 28 2019 at 14:32, on Zulip):

and trying to get a non-trivial, mixed Rust/C++ code base (Firefox) to compile with PGO enabled.

nikomatsakis (Mar 28 2019 at 14:32, on Zulip):

(what does clang do?)

nikomatsakis (Mar 28 2019 at 14:32, on Zulip):

or are you getting to that...

mw (Mar 28 2019 at 14:32, on Zulip):

clang supports all of them

mw (Mar 28 2019 at 14:32, on Zulip):

the D-frontend for LLVM only supports front-end level, I think

mw (Mar 28 2019 at 14:33, on Zulip):

I was able to verify that instrumented code from Rust and from C++ can be linked together into a single binary

mw (Mar 28 2019 at 14:33, on Zulip):

and that running the binary generates profiling data that contains counts for both Rust and C++

mw (Mar 28 2019 at 14:33, on Zulip):

IFF both binaries are instrumented with the IR-level approach

mw (Mar 28 2019 at 14:33, on Zulip):

Mixing IR-level and FE-level instrumentation does not work, although they are both built on the same infrastructure in compiler-rt.

mw (Mar 28 2019 at 14:33, on Zulip):

that took me a few days to find out :P

mw (Mar 28 2019 at 14:33, on Zulip):

I have yet to work on verifying the PGO data actually gets used correctly and that it improves performance.

mw (Mar 28 2019 at 14:33, on Zulip):

There are also platform specific problems that are not yet resolved.

mw (Mar 28 2019 at 14:34, on Zulip):

E.g. instrumentation seems to crash Clang when it encounters a function using SEH on Windows, which is something that we might run into too.

mw (Mar 28 2019 at 14:34, on Zulip):

yeah, so this is where I am at right now

mw (Mar 28 2019 at 14:34, on Zulip):

it's nice that PGO subsumes "ordering files", it seems

mw (Mar 28 2019 at 14:35, on Zulip):

another optimization that affects code layout in binaries

mw (Mar 28 2019 at 14:35, on Zulip):

leading to better caching behavior

mw (Mar 28 2019 at 14:35, on Zulip):

LLVM's pgo should take care of that

nikomatsakis (Mar 28 2019 at 14:35, on Zulip):

E.g. instrumentation seems to crash Clang when it encounters a function using SEH on Windows, which is something that we might run into too.

that does seem unfortunate, presumably an LLVM bug?

mw (Mar 28 2019 at 14:36, on Zulip):

bug or "not yet implemented" I guess

nikomatsakis (Mar 28 2019 at 14:36, on Zulip):

It sounds like the plan, though, is still to do IR-level? This seems to fit reasonably well with the whole ThinLTO approach, no?

mw (Mar 28 2019 at 14:36, on Zulip):

fortunately we have someone looking into it who really knows windows well

nikomatsakis (Mar 28 2019 at 14:36, on Zulip):

in other words, if you want max perf for a mixed Rust/C++-level thing, then we try to get all the sources to the level of LLVM IR, and then apply a combined pipeline for profiling, inlining, and optimizing them..?

nikomatsakis (Mar 28 2019 at 14:37, on Zulip):

Or is that overly simplified :)

mw (Mar 28 2019 at 14:37, on Zulip):

yes, it should work together with THinLTO

mw (Mar 28 2019 at 14:37, on Zulip):

that's basically how it works, yes

mw (Mar 28 2019 at 14:38, on Zulip):

you could also do Rust and C++ separately though

mw (Mar 28 2019 at 14:38, on Zulip):

ThinLTO is orthogonal

mw (Mar 28 2019 at 14:38, on Zulip):

also cross-language LTO is orthogonal

mw (Mar 28 2019 at 14:39, on Zulip):

but like with cross lang LTO you better make sure that your Clang and Rust's LLVM match :)

nikomatsakis (Mar 28 2019 at 14:39, on Zulip):

ThinLTO is orthogonal

in the sense that you could incorporate the pgo data when compiling the rust code on its own

mw (Mar 28 2019 at 14:39, on Zulip):

yes

nikomatsakis (Mar 28 2019 at 14:40, on Zulip):

it seems like pgo would however be helpful to ThinLTO (to guide inlining decisions)

mw (Mar 28 2019 at 14:40, on Zulip):

yep

mw (Mar 28 2019 at 14:40, on Zulip):

and ThinLTO makes use of PGO data indeed

mw (Mar 28 2019 at 14:41, on Zulip):

so, that's pretty much my update

mw (Mar 28 2019 at 14:41, on Zulip):

questions?

nikomatsakis (Mar 28 2019 at 14:42, on Zulip):

I'm trying to think

nikomatsakis (Mar 28 2019 at 14:42, on Zulip):

it all makes sense of course

mw (Mar 28 2019 at 14:42, on Zulip):

we could use it, of course, to make the compiler itself faster too

pnkfelix (Mar 28 2019 at 14:43, on Zulip):

Maybe orthogonal Q: what is our current policy about when ThinLTO is enabled?

nikomatsakis (Mar 28 2019 at 14:43, on Zulip):

I guess my question is -- what is your next step? and to what extent is this work (along with ThinLTO) going to be something that we can help projects beyond FF take advantage of?

nikomatsakis (Mar 28 2019 at 14:43, on Zulip):

And maybe if you have any sense of how much work remains -- it's fine if it's too early to say.

mw (Mar 28 2019 at 14:44, on Zulip):

what is your next step?

verifying that rustc's version of LLVM uses the data correctly

mw (Mar 28 2019 at 14:44, on Zulip):

to what extent is this work (along with ThinLTO) going to be something that we can help projects beyond FF take advantage of?

Once it's stable, any project can use it to get better performance for their release builds

mw (Mar 28 2019 at 14:45, on Zulip):

PGO is very popular amoing game developers, usually

mw (Mar 28 2019 at 14:45, on Zulip):

but anywhere where you have the resources to set up some automation for this can take advantage

mw (Mar 28 2019 at 14:46, on Zulip):

it would be interesting to see this used with, for example, ripgrep

mw (Mar 28 2019 at 14:46, on Zulip):

which is performance sensitive

nikomatsakis (Mar 28 2019 at 14:46, on Zulip):

yeah

nikomatsakis (Mar 28 2019 at 14:46, on Zulip):

and you mentioned rustc, of course

mw (Mar 28 2019 at 14:47, on Zulip):

Maybe orthogonal Q: what is our current policy about when ThinLTO is enabled?

ThinLTO is the default for cargo build --release

mw (Mar 28 2019 at 14:47, on Zulip):

iirc

pnkfelix (Mar 28 2019 at 14:47, on Zulip):

hmm

pnkfelix (Mar 28 2019 at 14:47, on Zulip):

oh, cargo

pnkfelix (Mar 28 2019 at 14:48, on Zulip):

which may differ from the defaults of rustc, right?

pnkfelix (Mar 28 2019 at 14:48, on Zulip):

I need to keep that in mind when I benchmark in the future. :sad:

mw (Mar 28 2019 at 14:48, on Zulip):

I think rustc uses it by default for opt-level >= 2 and codegen-units > 1

pnkfelix (Mar 28 2019 at 14:48, on Zulip):

right, the problem I saw

pnkfelix (Mar 28 2019 at 14:48, on Zulip):

was that it isn't enabled for codegen-units=1

mw (Mar 28 2019 at 14:49, on Zulip):

yes

pnkfelix (Mar 28 2019 at 14:49, on Zulip):

but you need codegen-units=1 to observe optimal performance in some cases

pnkfelix (Mar 28 2019 at 14:49, on Zulip):

this was what I was looking at: https://github.com/rust-lang/rust/issues/11084#issuecomment-475577073

pnkfelix (Mar 28 2019 at 14:49, on Zulip):

and also the comment immediately above it

pnkfelix (Mar 28 2019 at 14:49, on Zulip):

anyway I know this is not the problem of PGO

pnkfelix (Mar 28 2019 at 14:50, on Zulip):

it just reminded me that I was idly wondering about whether our defaults for -C opt-level=3 are "good"

mw (Mar 28 2019 at 14:50, on Zulip):

yeah, the defaults won't give you the best performance

pnkfelix (Mar 28 2019 at 14:51, on Zulip):

I guess I should probably look for pre-existing issues about the UX here

pnkfelix (Mar 28 2019 at 14:51, on Zulip):

anyway

mw (Mar 28 2019 at 14:52, on Zulip):

-Ccodegen-units could be made to default to 1 if -Copt-level=3?

pnkfelix (Mar 28 2019 at 14:52, on Zulip):

That was something I was indeed musing about

pnkfelix (Mar 28 2019 at 14:52, on Zulip):

(and have it still default to lto=thin, I think, based on what I observed on that ticket I linked)

mw (Mar 28 2019 at 14:53, on Zulip):

well, there are two ways of doing THinLTO with rustc

pnkfelix (Mar 28 2019 at 14:53, on Zulip):

7 minutes left, and 7 nominated issues

mw (Mar 28 2019 at 14:53, on Zulip):

"local" and "crate-graph"

pnkfelix (Mar 28 2019 at 14:53, on Zulip):

"local" and "crate-graph"

oh I'm not familiar with this

mw (Mar 28 2019 at 14:53, on Zulip):

"local" makes no sense with 1 cgu...

pnkfelix (Mar 28 2019 at 14:54, on Zulip):

let me quickly run through the nominations

mw (Mar 28 2019 at 14:54, on Zulip):

anyway, let's discuss nominated issues

pnkfelix (Mar 28 2019 at 14:54, on Zulip):

make sure there are no fires

pnkfelix (Mar 28 2019 at 14:54, on Zulip):

:construction_worker: "Refactor rustc_codegen_ssa (part 2)" #56636 is crying out for a reviewer

pnkfelix (Mar 28 2019 at 14:54, on Zulip):

any volunteers?

pnkfelix (Mar 28 2019 at 14:54, on Zulip):

(@eddyb is assigned but I think we should hand it off to someone else)

pnkfelix (Mar 28 2019 at 14:55, on Zulip):

((if we're going to do that refactoring at all))

centril (Mar 28 2019 at 14:55, on Zulip):

It's blocked on https://github.com/rust-lang/rust/pull/58846

pnkfelix (Mar 28 2019 at 14:55, on Zulip):

I'll skip #57214 since we already discussed it

pnkfelix (Mar 28 2019 at 14:55, on Zulip):

It's blocked on https://github.com/rust-lang/rust/pull/58846

okay then ...

pnkfelix (Mar 28 2019 at 14:56, on Zulip):

:construction_worker: also seeking reviewer for "Misc refactorings to rustc_codegen_ssa" #58846

centril (Mar 28 2019 at 14:56, on Zulip):

(but that one could use a reviewer since eddyb hasn't done much here)

pnkfelix (Mar 28 2019 at 14:56, on Zulip):

"Rustc should use a variable other than RUST_LOG for env_logger." #57985 was accidentally still nominated; fixed now.

pnkfelix (Mar 28 2019 at 14:57, on Zulip):

next, "beta 1.33 seems to break tarpaulin on multithreading" #58104 : as announced in an :alert: at start of meeting, I'm planning to close this as non-actionable if no-one speaks up

pnkfelix (Mar 28 2019 at 14:57, on Zulip):

next: "Implement "pipelined" rustc compilation (compiler side)" #58465. This needs prioritization

pnkfelix (Mar 28 2019 at 14:58, on Zulip):

or we need to explicitly put it on a steering meeting agenda

pnkfelix (Mar 28 2019 at 14:58, on Zulip):

@nikomatsakis what do you think at this point? I'm not sure we're ever going to have time for #58465 at the thursday meetings, unless we allocate a WG-checkin slot to it on its own, and assign someone to drive discussion.

pnkfelix (Mar 28 2019 at 14:59, on Zulip):

next: "Incorporate RLS bug tracking into compiler team triage" #58858. This was left nominated from when it was originally created but I actually think we can un-nominate it, unless I continue to fail to make any progress on it.

pnkfelix (Mar 28 2019 at 14:59, on Zulip):

I'm going to thus un-nominated #58858

pnkfelix (Mar 28 2019 at 15:00, on Zulip):

finally: " Specific code layout can cause compiler panic with lto=true #59137"

mw (Mar 28 2019 at 15:00, on Zulip):

didn't have time to look into that yet :/

pnkfelix (Mar 28 2019 at 15:00, on Zulip):

and, hah, that was another accidental nomination tag I forgot to remove

pnkfelix (Mar 28 2019 at 15:00, on Zulip):

that's okay @mw, the nomination was left over from when I wanted a volunteer for it

pnkfelix (Mar 28 2019 at 15:01, on Zulip):

we ... we actually "got through" everything

pnkfelix (Mar 28 2019 at 15:01, on Zulip):

perhaps only in a superficial manner

pnkfelix (Mar 28 2019 at 15:01, on Zulip):

but I like to stress the super in "superficial"

pnkfelix (Mar 28 2019 at 15:01, on Zulip):

Okay then, thanks to everyone in @T-compiler/meeting for attending!

mw (Mar 28 2019 at 15:02, on Zulip):

thanks for driving, @pnkfelix !

centril (Mar 28 2019 at 15:03, on Zulip):

@pnkfelix @nikomatsakis btw... re. the Box magic thing... is this something to discuss at T-Lang? If we can avoid putting more problems in front of de-magickifiying Box that would be nice.

pnkfelix (Mar 28 2019 at 15:03, on Zulip):

Should I actually unassign @eddyb from the review of PRs #58846 and #56636 ?

centril (Mar 28 2019 at 15:03, on Zulip):

@pnkfelix not until you assign someone else

oli (Mar 28 2019 at 15:04, on Zulip):

I've grabbed 58846

pnkfelix (Mar 28 2019 at 15:04, on Zulip):

thanks @oli

pnkfelix (Mar 28 2019 at 15:04, on Zulip):

@centril let me make sure I understand the logic here

pnkfelix (Mar 28 2019 at 15:04, on Zulip):

if I decide I don't have time to review something, I can unassign myself without finding someone else

pnkfelix (Mar 28 2019 at 15:05, on Zulip):

I do think that the correct protocol here would be to first ping @eddyb (or any other assigned reviewer) before just blindly unassigning them.

centril (Mar 28 2019 at 15:05, on Zulip):

I can unassign myself without finding someone else

Ideally not... at least the team should be pinged or so... All PRs should have reviewers

centril (Mar 28 2019 at 15:06, on Zulip):

Yes, pinging is right

pnkfelix (Mar 28 2019 at 15:06, on Zulip):

I agree that ideally, one would delegate a review to someone else rather than just ceding responsibility entirely.

centril (Mar 28 2019 at 15:06, on Zulip):

I think reassigning is preferable to pinging tho it much time has gone by since it is usually faster

pnkfelix (Mar 28 2019 at 15:07, on Zulip):

but finding someone to take on a review is itself a task

pnkfelix (Mar 28 2019 at 15:07, on Zulip):

and so I'm claiming that in the general case, we cannot avoid people unassigning themselves

centril (Mar 28 2019 at 15:07, on Zulip):

@pnkfelix we could make it a thing to unassign yourself and let T-release find a reviewer

pnkfelix (Mar 28 2019 at 15:08, on Zulip):

I guess all I'm saying is that there may be a process breakdown here

centril (Mar 28 2019 at 15:08, on Zulip):

what I would prefer not to happen is for a PR to be unassigned for a long time without anything happening to it

pnkfelix (Mar 28 2019 at 15:08, on Zulip):

since I tend to assume that if a PR has an assignee, it is getting dealt wth

pnkfelix (Mar 28 2019 at 15:08, on Zulip):

and so I'll skim the PR list and look for ones that have no assignee

pnkfelix (Mar 28 2019 at 15:09, on Zulip):

to try to determine what needs attention

pnkfelix (Mar 28 2019 at 15:09, on Zulip):

but this process will break down if PR's are artifically holding on to their assignees, right?

eddyb (Mar 28 2019 at 15:09, on Zulip):

btw I am hard to reach because of the choice of chat systems is horrible for me

pnkfelix (Mar 28 2019 at 15:09, on Zulip):

(I do agree that it would also be a break down in process for PR's to go unassigned for long periods of time)

pnkfelix (Mar 28 2019 at 15:09, on Zulip):

@eddyb I understand that problem

eddyb (Mar 28 2019 at 15:10, on Zulip):

IRC channels/PM > Discord PMs > Zulip PMs > Discord channels (some of which I avoid because I still haven't processed the backlog) > Zulip topics

pnkfelix (Mar 28 2019 at 15:10, on Zulip):

where do Zulip topics rank compared to Github notifications though? :smiley:

eddyb (Mar 28 2019 at 15:10, on Zulip):

as for GH notification I still have a backlog but try to often go through the last N pings, using octobox.io

eddyb (Mar 28 2019 at 15:11, on Zulip):

well I can't see GH notification context without clicking on them

pnkfelix (Mar 28 2019 at 15:11, on Zulip):

I cannot hope to keep up with Discord myself

eddyb (Mar 28 2019 at 15:11, on Zulip):

srsly talk to me in PM if you need me personally

pnkfelix (Mar 28 2019 at 15:11, on Zulip):

(I still find the Discord mention system to be inscrutable.)

eddyb (Mar 28 2019 at 15:11, on Zulip):

you can use Discord's search instead

centril (Mar 28 2019 at 15:11, on Zulip):

@pnkfelix I think that 1) if unassigning without assigning someone else is a thing, then we need a T-compiler/T-release process for reassignment. 2) If folks cannot review a PR they should either a) unassign themselves, b) reassign someone else.

eddyb (Mar 28 2019 at 15:11, on Zulip):

and search for mentions: or something

eddyb (Mar 28 2019 at 15:12, on Zulip):

also highfive is assigning me a ridiculous amount of things, most of which are irrelevant for my particular areas of the compiler I care about or can honestly review

eddyb (Mar 28 2019 at 15:12, on Zulip):

I was filing an issue about that earlier

centril (Mar 28 2019 at 15:12, on Zulip):

highfive shouldn't be assigning Niko and @eddyb all the time

pnkfelix (Mar 28 2019 at 15:12, on Zulip):

1) if unassigning without assigning someone else is a thing, then we need a T-compiler/T-release process for reassignment.

Hmm. true, I do visit the P-high issues each week looking in part for the unassigned cases

pnkfelix (Mar 28 2019 at 15:13, on Zulip):

arguably I should also be visiting the PR's

pnkfelix (Mar 28 2019 at 15:13, on Zulip):

but those aren't always explicitly marked with T-compiler, right?

centril (Mar 28 2019 at 15:13, on Zulip):

@pnkfelix PRs? no.

pnkfelix (Mar 28 2019 at 15:14, on Zulip):

yeah. So it would not be trivial for me to include the PRs in the weekly triage right now

eddyb (Mar 28 2019 at 15:14, on Zulip):

it's incredible how many bad repercussions shipping Rust 2018 + taking work in December had on my ability to stay on top of things

pnkfelix (Mar 28 2019 at 15:14, on Zulip):

but I suppose if someone, before unassigning themself, did have to at least tag the team who should find the next reviewer

pnkfelix (Mar 28 2019 at 15:14, on Zulip):

then that would help there, right @centril ?

centril (Mar 28 2019 at 15:14, on Zulip):

@pnkfelix https://github.com/rust-lang/rust/pulls?q=is%3Aopen+is%3Apr+no%3Aassignee the list is pretty small

centril (Mar 28 2019 at 15:15, on Zulip):

yeah assigning the team would be good

eddyb (Mar 28 2019 at 15:15, on Zulip):

anyway, small announcement, I got a 1440p 24" monitor this week, and will try to more aggressively go through things. I wish chrome had tree-style tabs or firefox was easy to swap all the state to...

centril (Mar 28 2019 at 15:15, on Zulip):

or fallback to t-release if no team is there

eddyb (Mar 28 2019 at 15:15, on Zulip):

the Zulip app is worse than ever, perf-wise, though, jeeez

centril (Mar 28 2019 at 15:15, on Zulip):

@eddyb did you kill your laptop yet?

pnkfelix (Mar 28 2019 at 15:16, on Zulip):

the Zulip app is worse than ever, perf-wise, though, jeeez

on the mobile device? Or on the computer?

eddyb (Mar 28 2019 at 15:16, on Zulip):

on my laptop. on my phone it's much worse

pnkfelix (Mar 28 2019 at 15:16, on Zulip):

/me must have stockholm syndrome when it comes to Zulip

eddyb (Mar 28 2019 at 15:16, on Zulip):

the mobile app may be a phone health risk, more so than mobile games

eddyb (Mar 28 2019 at 15:16, on Zulip):

I like Zulip's ideas but the UI and UX are depressing

pnkfelix (Mar 28 2019 at 15:17, on Zulip):

pnkfelix https://github.com/rust-lang/rust/pulls?q=is%3Aopen+is%3Apr+no%3Aassignee the list is pretty small

let me try adding this to the list of weekly triage items, see if its worth including in that parallel topic

eddyb (Mar 28 2019 at 15:17, on Zulip):

oh hmm, Zulip looks way less bad when tall

pnkfelix (Mar 28 2019 at 15:17, on Zulip):

if the list grows significnatly in the future, then that will be a sign that we need more pro-active T-tagging on it

eddyb (Mar 28 2019 at 15:18, on Zulip):

but no chat app deserves a full column of my tiled layout

pnkfelix (Mar 28 2019 at 15:18, on Zulip):

At least zulip can deal with being squished sideways

pnkfelix (Mar 28 2019 at 15:18, on Zulip):

Discord wont let me

centril (Mar 28 2019 at 15:18, on Zulip):

@pnkfelix so... re. Box & magic... discuss on T-Lang meeting?

centril (Mar 28 2019 at 15:18, on Zulip):

What I would like most is to understand the issue

centril (Mar 28 2019 at 15:19, on Zulip):

what the magic consists of exactly and document it well on a GH issue

eddyb (Mar 28 2019 at 15:19, on Zulip):

oooh I can make Zulip taller at the expense of IRC & DIscord

eddyb (Mar 28 2019 at 15:19, on Zulip):

@centril DerefMove but without an impl opt-in

pnkfelix (Mar 28 2019 at 15:19, on Zulip):

pnkfelix so... re. Box & magic... discuss on T-Lang meeting?

If you want to add it as an item, I guess you can. I think most people who care about the issue tend to think that the solution is to factor the magic into its own DerefMove trait

eddyb (Mar 28 2019 at 15:19, on Zulip):

the semantics are what I want for DerefMove

centril (Mar 28 2019 at 15:19, on Zulip):

@pnkfelix is DerefMove the only thing?

centril (Mar 28 2019 at 15:20, on Zulip):

or is there additional magic now?

centril (Mar 28 2019 at 15:20, on Zulip):

(wrt. https://github.com/rust-lang/rust/pull/58021)

pnkfelix (Mar 28 2019 at 15:20, on Zulip):

regarding #58021, I have no idea

pnkfelix (Mar 28 2019 at 15:20, on Zulip):

I still haven't processed what the heck happened there. Thus my desire to make a regression test.

Vadim Petrochenkov (Mar 28 2019 at 15:21, on Zulip):

The destructor is magical.

Vadim Petrochenkov (Mar 28 2019 at 15:21, on Zulip):

But perhaps it needs to be for any DerefMove, not sure.

eddyb (Mar 28 2019 at 15:21, on Zulip):

@Vadim Petrochenkov I think the right solution is what @Ariel Ben-Yehuda came up with, which is that DerefMove opts into drop glue being drop_in_place(&mut *x); Drop::drop(&mut x);

eddyb (Mar 28 2019 at 15:22, on Zulip):

I suppose that needs to make Drop an unsafe impl

Vadim Petrochenkov (Mar 28 2019 at 15:22, on Zulip):

Support for box patterns is magical.
Also seems a part of DerefMove.

pnkfelix (Mar 28 2019 at 15:22, on Zulip):

/me needs to go AFK, but feel free to carry on chatting. @centril : double check whether niko will be at the meeting tonight. If not, then I wouldn't try to do the Box Q at tonight's meeting, unless @eddyb is attending.

Vadim Petrochenkov (Mar 28 2019 at 15:23, on Zulip):

Boxs ABI was also very special, not sure about the current status.

centril (Mar 28 2019 at 15:23, on Zulip):

@eddyb care to join the meeting?

centril (Mar 28 2019 at 15:23, on Zulip):

@nikomatsakis so... I forget, are you attending the T-Lang meeting today?

eddyb (Mar 28 2019 at 15:23, on Zulip):

ah, patterns, well, I think we should maybe consider checking the MIR via borrowck instead of having a separate pattern checker...

centril (Mar 28 2019 at 15:24, on Zulip):

(box patterns are unstable)

eddyb (Mar 28 2019 at 15:24, on Zulip):

i.e. check that the if-else chains or switch w/e generated by pattern lowering are exhaustive

eddyb (Mar 28 2019 at 15:24, on Zulip):

not sure I can join voice/video meetings, given my new office situation

eddyb (Mar 28 2019 at 15:24, on Zulip):

also, is the DST hell over?

pnkfelix (Mar 28 2019 at 15:25, on Zulip):

@eddyb no, not until next week

eddyb (Mar 28 2019 at 15:25, on Zulip):

because I think the meeting is after the time I have to leave to catch the subway during this time

pnkfelix (Mar 28 2019 at 15:25, on Zulip):

(which I was mistaken about up until last monday)

eddyb (Mar 28 2019 at 15:25, on Zulip):

so was I pinged only for #58021?

pnkfelix (Mar 28 2019 at 15:25, on Zulip):

@eddyb there were some PR's in your review queue that we also discussed

eddyb (Mar 28 2019 at 15:26, on Zulip):

right, sorry about that

pnkfelix (Mar 28 2019 at 15:26, on Zulip):

in terms of whether to delegate the reviews to other people

eddyb (Mar 28 2019 at 15:26, on Zulip):

people need to contaact me personally, GH pings will not guarantee I will see it

pnkfelix (Mar 28 2019 at 15:26, on Zulip):

(I don't think you need to apologize.)

eddyb (Mar 28 2019 at 15:26, on Zulip):

good idea to ask before reassigning, some things I can review on the spot

pnkfelix (Mar 28 2019 at 15:26, on Zulip):

(we know you've juggling a lot)

eddyb (Mar 28 2019 at 15:26, on Zulip):

again, much easier since this week

eddyb (Mar 28 2019 at 15:27, on Zulip):

it's less being busy and more a cascading failure to keep up :P

eddyb (Mar 28 2019 at 15:27, on Zulip):

like leaving one minute late and being an hour late by the time you reach wherever you want to get

centril (Mar 28 2019 at 15:31, on Zulip):

@pnkfelix https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=dae27f4169c6e5cf24c73dd763984055

centril (Mar 28 2019 at 15:31, on Zulip):

5 happens before 3

centril (Mar 28 2019 at 15:31, on Zulip):

thus making it observable

eddyb (Mar 28 2019 at 15:34, on Zulip):

hehehe I just realized I can give Zulip more vertical screenspace

pnkfelix (Mar 28 2019 at 15:35, on Zulip):

5 happens before 3

hmm isn't that what's supposed to happen there? Is that actually an example of weirdness...?

pnkfelix (Mar 28 2019 at 15:35, on Zulip):

since you used a _ pattern in that arm that causes dbg!(5) ?

centril (Mar 28 2019 at 15:36, on Zulip):

@pnkfelix changing it to b => { dbg!(5); } makes no difference

centril (Mar 28 2019 at 15:37, on Zulip):

(or b => { dbg!(b); })

pnkfelix (Mar 28 2019 at 15:37, on Zulip):

I'm still not convinced this is what matters

pnkfelix (Mar 28 2019 at 15:37, on Zulip):

(i.e., b should be dropped 5 is printed...)

centril (Mar 28 2019 at 15:38, on Zulip):

@pnkfelix not sure I get what you mean... this is just demonstrating the drop order that ariel noted

pnkfelix (Mar 28 2019 at 15:38, on Zulip):

but you are correct that I was wrong to think that the _ pattern mattered there

pnkfelix (Mar 28 2019 at 15:38, on Zulip):

my claim is that the drop order you are describing is what we would want to adhere to, even with the changes ariel wants

eddyb (Mar 28 2019 at 15:38, on Zulip):

pasted image

eddyb (Mar 28 2019 at 15:39, on Zulip):

this seems more workable

centril (Mar 28 2019 at 15:39, on Zulip):

@pnkfelix oh... well I don't disagree with that

centril (Mar 28 2019 at 15:39, on Zulip):

@pnkfelix I mostly thought it would be good to proactively discuss and affirm our view with the whole language team

pnkfelix (Mar 28 2019 at 15:40, on Zulip):

okay ... but this doesn't directly show the duplicated storage-dead's that @Ariel Ben-Yehuda described.

pnkfelix (Mar 28 2019 at 15:41, on Zulip):

but ooh maybe we can get miri to complain about them!

centril (Mar 28 2019 at 15:41, on Zulip):

@pnkfelix yeah I only went for drop order since that's easiest to show ;)

centril (Mar 28 2019 at 15:42, on Zulip):

ill nominate then

pnkfelix (Mar 28 2019 at 15:43, on Zulip):

well if you nominate I'm not sure I'm going to be able to present tonight

pnkfelix (Mar 28 2019 at 15:44, on Zulip):

someone will need to drive that discussion but I doubt it will be me, at least not this week (DST hell)

centril (Mar 28 2019 at 15:44, on Zulip):

@pnkfelix worst case we delay till next week

pnkfelix (Mar 28 2019 at 15:45, on Zulip):

k

nagisa (Mar 28 2019 at 19:16, on Zulip):

I guess that the concerns nagisa raised were mostly kind of "memory usage is too high, let's not regress, even a bit"?

FWIW I managed to build rustc on a 4G 64-bit VM, so there’s that. I’m significantly less concerned about our memory usage now.

nagisa (Mar 28 2019 at 19:16, on Zulip):

That was FreeBSD btw.

Tatsuyuki Ishi (Mar 29 2019 at 00:25, on Zulip):

Regarding the test, I originally thought of using miri, because it can test memory semantics apart from double free which the allocator wouldn't care otherwise. But for a "regression" test it could be written without miri. I didn't bother with the miri test because I was short on time and didn't know how miri based tests work.

Tatsuyuki Ishi (Mar 29 2019 at 00:27, on Zulip):

(that was about #58021)

pnkfelix (Mar 29 2019 at 08:40, on Zulip):

@Tatsuyuki Ishi ah, I didn't look properly at where this change was being applied before. I had thought it was some fn map in libstd, not libsyntax

pnkfelix (Mar 29 2019 at 08:40, on Zulip):

i am far far far less concerned about this now.

Last update: Nov 22 2019 at 05:25UTC