here is where I'll have my previously mentioned "open discussion time"
My most immediate goal is to finish going through yesterday's agenda items
but given the informal nature, feel free to inject digressions or post links to other Zulip topics that you think would be of general interest. :)
In the performance logs for the last week, there were some small compile-time regressions injected
A few small compile-time regressions this week. The first was #70793, which added some specializations to the standard library in order to increase runtime performance. The second was #73996, which adds an option to the diagnostics code to print only the names of types and traits when they are unique instead of the whole path. The third was #75200, which refactored part of
BTreeMap to avoid aliasing mutable references.
Triage done by @ecstatic-morse
Revision range: d927e5a655809b6b20501889e093c085d6ffe6f7..35fc8359868e65a0970094f648ba87fce634e8c7
3 Regressions, 0 Improvements, 0 of them in rollups.
docbenchmarks, likely due to better optimized standard library.
let me see what that nag was about.
let me see what that nag was about.
@ssomers is not very happy with the review process
so I posted some followup thoughts in #t-compiler/performance > Triage nags and #75200
I don't think this nag needs too much follow-up
nor do I
the btreemap code seems to be just very unstable
in the "slight adjustment" produces relatively strong effect. My current theory is it's very sensitive to inlining decisions or similar.
/me is now skimming over the other metrics captures by perf here besides instruction counts
the max-rss and the #faults both went up quite a bit on a few benchmarks
max-rss is really unstable
but I think all of the significantly affected benchmarks are not representative of "real code" in the wild
@simulacrum in general, I assume you mean?
we see variability of +/- 20% easily on comment changes
I wonder if its worth consider changing the allocator just to make that number more stable
tough to say
maybe, not sure. I've long wanted to figure out why there's such wide variability in the first place, I'd personally not expect it
I've been on this ride before, in terms of the utility of specializing the benchmark host in such ways; doing so makes it diverge from the "real system", but if the measurements you're getting are too noisy to be useful ...
we've not historically worried too much about max-rss
we probably should be more worried about it
if there's a consistent jump then maybe but usually it's not too interesting
yes, for sure
in that we know that the memory use of the compiler makes development untenable for some people
anyway, something to keep in mind for the future
lets move along to the nominated issues for this week, see if there are any we can take care of even in an "unofficial" meeting like this
I had added this item
I think we definitely don't have time for weekly triage
in that we're already over-extended for each agenda each week?
Yeah, I agree.
we could probably allocate one of teh Friday meeting slots to it though
I think it would be valuable to maybe just to decide if this is something we even want to tackle.
some of these predate the MCP process, right?
Given the lack of activity on ~50% of these, arguably we could just close them as deferred.
like, one step we could take is just review them and see how many could be turned into MCP'S
I think nearly all of them do
But some are cross-team and not necessarily appropriate for the MCP process (IMO)
those cases can stay as RFCs
lets take a quick look now at the list, see if we can bucket them accordingly
But before we spam a bunch of RFC authors with requests to use the MCP process, it might be good to decide if we even have interest in tackling these
I don't want to ask people to do work we're ultimately going to reject
I'm going to try to review them here
are you thinking of inline commenting in that doc, or discussing one-by-one here?
my plan is to first reorganize them
ones with multiple team labels will go into a separate section
the ones that are just tagged with us I feel a higher priority to address
since they really are our sole responsibility to deal with
so, now that I've bucketed them in the above manner
I'm not sure it makes sense to go through them all one by one here
but it may be possible to quickly identify things we can take action on
I know I have thoughts about most of ours
Should we maybe jot that stuff down in the doc and then see if there's any common feelings?
sure, I'm happy to handle it that way
so there's a lot of content to catch up on if one really wants to read each RFC before commenting on its content
one detail I'm reflecting on, is that if you read the content of RFC PR #2400, there are some pretty strong claims made about whether what rustc currently does is valid in general
I don't have any more comments to add about that set of RFCs
and I think that its important to see that the comment feedback on RFC PR #2400 gives pretty good counter-arguments to those aforementioned claims
I would fine with closing 2400 because it doesn't sound high priority for the compiler team at this time and it needs more design work.
yes I think that is probably the right call
maybe now is the right time to just quickly go through the list here
So: Do you all think we should suggest to the author that they propose an MCP, or should we attempt to port the RFC to an MCP ourselves?
I think the right answer is to ask the author to do it
Yes, I think so. In particular I would only want to accept this if someone's willing to drive the impl work as well
(no need to create debt)
the main reason I balk at that is that I feel bad waiting all this time and then having the response be "oh sorry, use this other system we came up with in the meantime"
The author doesn't appear to be very active on GitHub.
but yes, @simulacrum 's point is crucial: We need someone willing to drive the work
so the only RFC that I would really consider porting oursevles
But it does sound like there are other interested parties who participated in the comments that might be willing to drive this forward.
is one that we ourselves see as such a clear benefit that we ourselves would be willing to drive the work during our own time
I am okay fcp merging, but like, I don't want this to end up as an open issue that we end up needing to reconsider from scratch in a year because it's so far out of cache for everyone
Yeah. Lets post a comment saying that we recommend this (RFC#2154) be converted to an MCP, either by the PR author or another interested volunteer.
I can write that comment
as previously discussed here, I think we should just close this, as described by @Wesley Wiser
by "accept", that means propose an FCP merge?
the MCP process may itself be lighter weight
Or even potentially just merge it
well, I would be fine just merging it
its just an amendment
As niko said:
We often do an FCP for amendments, but I also think it's ok to "just do it" if it is small and not controversial amongst the folks who were active in the RFC itself. I am fine with this change (but I've not been following the RFC closely).
okay yeah its just an amendment
lets just merge it then
Seems like a good candidate for MCP especially as the author is quite active in the project.
here again I think we can take the same approach that was suggested for rfc#2154: suggest it be converted to MCP, either by PR author or by another interested contributor
so based on the comments here
(It was unclear to me why this was an RFC, to be honest)
the main reason why this might not be "just" a PR
is that the implementation is "hard"
at least doing it well
The author may have also been trying to get feedback from diagnostics people if this would be accepted or not before doing the work.
I think that's a good candidate for MCP today -- to get buy-in on idea before working on impl
right, and then maybe it requires a project group, maybe not
but it also seems like the kind of thing where we usually have "just" a diagnostics issue on rust-lang/rust for
(or, half a dozen targeting different instances of this)
in that we know that the memory use of the compiler makes development untenable for some people
for context, I regularly run out of ram compiling docs.rs
and just have to restart my whole laptop because of thrashing
yeah back in the day @eddyb had the same problem, @Joshua Nelson
it led them to invest substantial effort in getting rustc's peak memory usage down
anyway, regarding rfc#2777
lets write a comment suggesting that the RFC be converted to an MCP
oh, we didn't actually say wehther we were going to close such RFCs
I ... think the right thing would be a label
something like "convert-to-mcp" or "should-be-mcp"
I would be inclined to close, personally
and we leave the RFC open until an MCP is created, or until some amount of time has passed
But maybe, say, in a month
yeah, lets give it a month (or maybe six weeks, heh)
That seems good to me
I just want to make sure that 1. good ideas don't get lost in the shuffle and 2. contributors don't get too discouraged , which I think closing can cause, even with really nice comments tacked on
so okay, next
is the parselib WG also dealing with macros?
oh I guess tat detail doesn't matter
I mean, it does matter
but the point is more that this RFC is clearly subsumed by the parselib WG, right?
That's my feeling yeah
Not that WGs can't submit RFCs but I feel like the project has already signed off on this work at least to some extent.
lets close this and say that active work is going on with parselib WG, and link to the appropriate pages describing the WG and/or MCPs
well, that, or
parselib will come up with something and propose a new RFC
but yeah, I think it would be weird to keep this open when I think the most important step has been taken
in terms of telling these contributors "yes, we want something here; go figure out what to do."
so okay, close this with links to places tracking ongoing work
I'm skimming over the RFC text now
I don't currently understand exactly how the modifiers here translate into what changes to the linker invocation
each modifier has a specific flag (or set of flags) it translates to, and that may be linker-dependant
(and the spec for each modifier states its behavior on linkers that don't support it ... which I think in all given cases is "no op if unsupported", right?)
the reason I'm digging into this level of detail is that I'm trying to figure out if this is indeed something that needs to loop in T-lang as well
I think overall I'm in favor of moving forward with it unstably
we can resolve the T-lang part concurrently
i.e. the main thing I can imagine T-lang having an opinion on is whether its simply a bad idea to stuff this into into the
and that's a discussion that doesn't need to block unstable development of the feature
lets go ahead and suggest it be posted as an MCP
same as other cases above
okay, we're well past the hour
thanks to @simulacrum and @Wesley Wiser for helping out here
Sorry for my topic dominating the hour but it feels like we made a huge amount of progress!
I honestly didn't expect we'd get that far.
(and to everyone in @T-compiler/meeting for attending)
yeah this was a good use of time I think
@pnkfelix are you planning to follow-up and post these comments? I volunteered to do 2154 and can do 2256 as well if you'd like
my current plan is to go back through yesterday's meeting
and make sure comments got posted where appropriate for that
when I'm done with that, I'll do the same for this meeting
anyone who wants to take one of the RFC's listed above, feel free. :)
(I suppose it would be good to use some common template for the common pattern of "port this to MCP")
okay. I'll take 2256 and 2154. Probably won't have a chance to type those up until later today, but there's no real rush I guess :)
by the way, @simulacrum , do you indeed think you'll have a chance to make a PR for #74753 that disables the check locally?
yes it's on my todo list
(and now I'm wondering what that meant? as in, the check will never run on people's local machines, just on CI alone?)
that is my plan yes
it is a bit unfortunate but ultimately CI is the only place I know of where we can assume things about the filesystem, and I don't want to try to write logic that detects filesystem properties
hmm. Just because that's less fragile than this logic to disable it on certain hosts based on inspecting the
/proc/version tells you nothing about the filesystem afaict, which is what you really want
yeah "fragile" was probably not the right word
or at least, its not the only thing that concerns us about the logic here
I guess thats' fine
though I'm now musing about filesystem inspection
wouldn't it suffice to touch a fresh file and then inspect its exec bit?
but then I wonder ... why haven't we already done that?
maybe because we don't want to write into the source directory
its possible that someone might actually have it read-only
(though arguably if so we can just say "well probably you're not editing it either")
(and is doing the build elsewhere ... not that I've ever done a build in a directory that isn't the root directory nor a subdirectory of the root)
sure, but the tidy run doesn't know whether you've tried to
that would be funny, actually
I guess I added it to fix https://github.com/rust-lang/rust/issues/36706
e.g. first do git status or somesuch, and then only if git status reports unclean, then touch a file and inpsect its exec bit
it = /proc/version check
I might try the touch a file strategy
I have to admit, I would prefer some way to avoid a check living solely in CI
and we can just skip the bins check if it's not possible to touch a file
I like that
that seems very likely to cover the cases of interest
@pnkfelix RFC 2951 has an MCP https://github.com/rust-lang/compiler-team/issues/356
this is a public facing feature, isn't it
And I think libsyntax2.0 was some @matklad 's proposal that I feel should be now subsumed by some joint rustc / rust-analyzer librarifycation and the new parser work
right, I think we reached the same conclusion in this discussion
Ok, sorry, I was just skimming through it
hey @LeSeulArtichaut or @simulacrum , I don't know if I screwed something up over here: https://github.com/rust-lang/compiler-team/issues/356#issuecomment-691169524
is there a way to undo a second so that I can make it a proper "fcp merge" instead?
(do I need to close and reopen the issue or something?)
they're to different bots
you should put
rfcbot fcp merge
I think rustbot links to triagebot's doc
oh that's the other bot you mean
(maybe rustbot should say so when you ask it to fcp merge something)
(I think you wrote
rfc instead of
I think it should too. I would've expected an unknown command comment
/me needs to learn to cut-and-paste
are there docs for this somewhere?
I don't think there is any doc about
@rfcbot, probably should go in Forge
does anyone mind if I add
D-newcomer-roadblock but for people working on rustc itself
Seems fine to me