Stream: t-compiler/wg-prioritization

Topic: pre-meeting triage 2020-04-16 #54818


Santiago Pastorino (Apr 15 2020 at 17:56, on Zulip):

The @WG-prioritization will be doing pre-triage in this channel

Santiago Pastorino (Apr 15 2020 at 18:00, on Zulip):

going to create the agenda and we can get this started

Santiago Pastorino (Apr 15 2020 at 18:00, on Zulip):

should be shorter given that most of the prioritization was done async

Santiago Pastorino (Apr 15 2020 at 18:02, on Zulip):

Agenda is here

Santiago Pastorino (Apr 15 2020 at 18:04, on Zulip):

ok, so let's start

Santiago Pastorino (Apr 15 2020 at 18:04, on Zulip):

Remove leftovers

Santiago Pastorino (Apr 15 2020 at 18:04, on Zulip):
  1. Unnominate leftover I-nominated
    - Remove I-nominated tag from issues discussed on the last meeting.
Santiago Pastorino (Apr 15 2020 at 18:05, on Zulip):

we have four in that list, checking last meeting agenda ...

Santiago Pastorino (Apr 15 2020 at 18:05, on Zulip):

#70928 is there again

Santiago Pastorino (Apr 15 2020 at 18:06, on Zulip):

#70917 too

Santiago Pastorino (Apr 15 2020 at 18:06, on Zulip):

the other two are new

Santiago Pastorino (Apr 15 2020 at 18:06, on Zulip):

checking those to see if should be discussed again or something

Yuki Okushi (Apr 15 2020 at 18:07, on Zulip):

#70928 is now doing fcp for closing

pnkfelix (Apr 15 2020 at 18:08, on Zulip):

yeah maybe just put it on agenda as a reminder for people to check their boxes.

pnkfelix (Apr 15 2020 at 18:08, on Zulip):

(or post counter arguments)

Santiago Pastorino (Apr 15 2020 at 18:10, on Zulip):

yeah, that one is what I was going to do

Santiago Pastorino (Apr 15 2020 at 18:10, on Zulip):

I'm checking #70917, unsure if it should still be nominated

Santiago Pastorino (Apr 15 2020 at 18:10, on Zulip):

reading the issue

Santiago Pastorino (Apr 15 2020 at 18:10, on Zulip):

@pnkfelix thoughts?

Santiago Pastorino (Apr 15 2020 at 18:11, on Zulip):

nomination was for T-lang but then T-lang tag was removed and nomination left

Santiago Pastorino (Apr 15 2020 at 18:12, on Zulip):

ok, meanwhile people check this, gonna continue

Santiago Pastorino (Apr 15 2020 at 18:12, on Zulip):
  1. Remove leftover I-prioritize
    - Remove I-prioritize tag from already prioritized issues.
Santiago Pastorino (Apr 15 2020 at 18:12, on Zulip):

this is empty so moving on

Santiago Pastorino (Apr 15 2020 at 18:12, on Zulip):

Unprioritized I-prioritize

Santiago Pastorino (Apr 15 2020 at 18:13, on Zulip):

I guess in this section we are not going to need to do anything, anyway let's check

Santiago Pastorino (Apr 15 2020 at 18:13, on Zulip):
  1. No team assigned and No team assigned I-nominated
    - Add T-compiler tag when it corresponds.
Santiago Pastorino (Apr 15 2020 at 18:13, on Zulip):

both lists are empty

Santiago Pastorino (Apr 15 2020 at 18:13, on Zulip):
  1. T-compiler and T-compiler I-nominated
    - Prioritize issues and remove nomination of the ones not worth discussing.
    - Tag regressions accordingly.
    - Ping appropriate people and/or ICE-breakers.
Santiago Pastorino (Apr 15 2020 at 18:14, on Zulip):

both empty

Santiago Pastorino (Apr 15 2020 at 18:14, on Zulip):
  1. All teams and All teams I-nominated
    - No action required. It's nice to look at this for comparison.
Santiago Pastorino (Apr 15 2020 at 18:14, on Zulip):

there are some nominations for other teams

Santiago Pastorino (Apr 15 2020 at 18:15, on Zulip):

so nothing to do as expected

Santiago Pastorino (Apr 15 2020 at 18:15, on Zulip):

Regressions

Santiago Pastorino (Apr 15 2020 at 18:15, on Zulip):
  1. Beta regressions without P-label
    - Prioritize.
    - Ping appropriate people and/or ICE-breakers.
    - Assign if possible; if it remains unassigned, add it to agenda so we can assign during the meeting.
pnkfelix (Apr 15 2020 at 18:15, on Zulip):

Santiago Pastorino said:

I'm checking #70917, unsure if it should still be nominated

Well, I suspect it doesn't have to remain nominated.

Santiago Pastorino (Apr 15 2020 at 18:15, on Zulip):

yeah me too :)

Santiago Pastorino (Apr 15 2020 at 18:15, on Zulip):

let's remove it then

pnkfelix (Apr 15 2020 at 18:15, on Zulip):

But #70917 is a regression and the comments there outline what we think needs to be done

Santiago Pastorino (Apr 15 2020 at 18:16, on Zulip):

yeah

pnkfelix (Apr 15 2020 at 18:16, on Zulip):

so I suspect we'll end up discussing it as it is currently an unassigned regression

pnkfelix (Apr 15 2020 at 18:16, on Zulip):

unless it gets assigned before the meeting tomorrow

Santiago Pastorino (Apr 15 2020 at 18:16, on Zulip):

yes, but going to remove nomination and add it as an unassigned regression

Santiago Pastorino (Apr 15 2020 at 18:16, on Zulip):

sounds good?

Santiago Pastorino (Apr 15 2020 at 18:17, on Zulip):

Santiago Pastorino said:

  1. Beta regressions without P-label
    - Prioritize.
    - Ping appropriate people and/or ICE-breakers.
    - Assign if possible; if it remains unassigned, add it to agenda so we can assign during the meeting.

there are no beta regressions without P label

Santiago Pastorino (Apr 15 2020 at 18:18, on Zulip):

Santiago Pastorino said:

sounds good?

done

Santiago Pastorino (Apr 15 2020 at 18:18, on Zulip):

so back to track

Santiago Pastorino (Apr 15 2020 at 18:19, on Zulip):

we were with regressions

Santiago Pastorino (Apr 15 2020 at 18:19, on Zulip):
  1. Nightly regressions without P-label
    - Prioritize.
    - Ping appropriate people and/or ICE-breakers.
    - Assign if possible; if it remains unassigned, add it to agenda so we can assign during the meeting.
Santiago Pastorino (Apr 15 2020 at 18:19, on Zulip):

no nightly regressions without P-label

Santiago Pastorino (Apr 15 2020 at 18:19, on Zulip):

Beta nominations

Santiago Pastorino (Apr 15 2020 at 18:20, on Zulip):
  1. No team assigned
    - Add T-compiler tag when it corresponds.
Santiago Pastorino (Apr 15 2020 at 18:20, on Zulip):

empty

Santiago Pastorino (Apr 15 2020 at 18:20, on Zulip):
  1. All teams
    - Add T-compiler tag when it corresponds.
Santiago Pastorino (Apr 15 2020 at 18:20, on Zulip):

we have 3 already beta accepted and one pending

Santiago Pastorino (Apr 15 2020 at 18:21, on Zulip):

it's not tagged T-compiler

Santiago Pastorino (Apr 15 2020 at 18:21, on Zulip):

adding the tag

Santiago Pastorino (Apr 15 2020 at 18:22, on Zulip):
  1. T-compiler
    - Add these issues to the meeting agenda.
Santiago Pastorino (Apr 15 2020 at 18:22, on Zulip):

we should have only that one :)

Wesley Wiser (Apr 15 2020 at 18:22, on Zulip):

Should probably remind meeting-goers that the next release is in 1 week so we really need to get the beta-backports finished asap.

Santiago Pastorino (Apr 15 2020 at 18:22, on Zulip):

yes

Santiago Pastorino (Apr 15 2020 at 18:22, on Zulip):

that's one big announcement I had in mind :)

Wesley Wiser (Apr 15 2020 at 18:22, on Zulip):

I'm sure everyone is already aware though :slight_smile:

Santiago Pastorino (Apr 15 2020 at 18:23, on Zulip):

ok this is really weird

Santiago Pastorino (Apr 15 2020 at 18:23, on Zulip):

the issue is not showing up in that list

Santiago Pastorino (Apr 15 2020 at 18:24, on Zulip):

#71131

Santiago Pastorino (Apr 15 2020 at 18:24, on Zulip):

not showing up in https://github.com/rust-lang/rust/issues?utf8=%E2%9C%93&q=label%3Abeta-nominated+label%3AT-compiler

Santiago Pastorino (Apr 15 2020 at 18:24, on Zulip):

any clue?

Wesley Wiser (Apr 15 2020 at 18:25, on Zulip):

Weird...

Wesley Wiser (Apr 15 2020 at 18:25, on Zulip):

GH bug?

Santiago Pastorino (Apr 15 2020 at 18:25, on Zulip):

I've just tagged at t-compiler

Santiago Pastorino (Apr 15 2020 at 18:25, on Zulip):

maybe they have some sort of cache

Santiago Pastorino (Apr 15 2020 at 18:25, on Zulip):

anyway, let's consider just that one

Wesley Wiser (Apr 15 2020 at 18:25, on Zulip):

Yeah almost seems like a caching issue

Wesley Wiser (Apr 15 2020 at 18:25, on Zulip):

Since you just added the label

Santiago Pastorino (Apr 15 2020 at 18:25, on Zulip):

yep

Santiago Pastorino (Apr 15 2020 at 18:25, on Zulip):

it's lasting to long imo :P

Santiago Pastorino (Apr 15 2020 at 18:25, on Zulip):

anyway

DPC (Apr 15 2020 at 18:26, on Zulip):

hard refresh? xD

Santiago Pastorino (Apr 15 2020 at 18:26, on Zulip):

adding this to the agenda

Santiago Pastorino (Apr 15 2020 at 18:27, on Zulip):

Stable nominations

Santiago Pastorino (Apr 15 2020 at 18:27, on Zulip):
  1. No team assigned
    - Add T-compiler tag when it corresponds.
Santiago Pastorino (Apr 15 2020 at 18:27, on Zulip):

empty

Santiago Pastorino (Apr 15 2020 at 18:27, on Zulip):
  1. All teams
    - Add T-compiler tag when it corresponds.
Santiago Pastorino (Apr 15 2020 at 18:28, on Zulip):

there's just one but already stable accepted

Santiago Pastorino (Apr 15 2020 at 18:28, on Zulip):

@pnkfelix remember me when the stable/beta-nominated tags are removed

Santiago Pastorino (Apr 15 2020 at 18:28, on Zulip):

should be done once it's accepted? or that's done with t-release?

Santiago Pastorino (Apr 15 2020 at 18:29, on Zulip):
  1. T-compiler
    - Add these issues to the meeting agenda.
pnkfelix (Apr 15 2020 at 18:29, on Zulip):

@Santiago Pastorino you don't touch the stable/beta nomination tags

Santiago Pastorino (Apr 15 2020 at 18:29, on Zulip):

Santiago Pastorino said:

  1. T-compiler
    - Add these issues to the meeting agenda.

checking just in case, but nothing interesting as expected given that all had nothing

Santiago Pastorino (Apr 15 2020 at 18:29, on Zulip):

pnkfelix said:

Santiago Pastorino you don't touch the stable/beta nomination tags

yeah I don't :), I was just wondering how/who handles it

pnkfelix (Apr 15 2020 at 18:30, on Zulip):

@Santiago Pastorino the nominations themselves get removed after the backporting PR is merged

pnkfelix (Apr 15 2020 at 18:30, on Zulip):

into stable/beta

Santiago Pastorino (Apr 15 2020 at 18:30, on Zulip):

:+1:

pnkfelix (Apr 15 2020 at 18:30, on Zulip):

I don't recall who ends up taking care of that, but it is the protocol

Santiago Pastorino (Apr 15 2020 at 18:30, on Zulip):

makes sense

Santiago Pastorino (Apr 15 2020 at 18:30, on Zulip):

PR's waiting for our team

Santiago Pastorino (Apr 15 2020 at 18:30, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:31, on Zulip):

there are 4, all of them with disposition to merge

Santiago Pastorino (Apr 15 2020 at 18:31, on Zulip):

I guess I'm going to add them all to the agenda noting that

DPC (Apr 15 2020 at 18:32, on Zulip):

(2nd one needs a reviewer)

Yuki Okushi (Apr 15 2020 at 18:34, on Zulip):

just a note: #70175's fcp is finished

Santiago Pastorino (Apr 15 2020 at 18:34, on Zulip):

DPC said:

(2nd one needs a reviewer)

good point

Santiago Pastorino (Apr 15 2020 at 18:35, on Zulip):

done

Santiago Pastorino (Apr 15 2020 at 18:35, on Zulip):

Critical and High priority issues

Santiago Pastorino (Apr 15 2020 at 18:36, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:37, on Zulip):

there's one and it's assigned to @oli

Santiago Pastorino (Apr 15 2020 at 18:37, on Zulip):

going to add it to the agenda given that it's P-critical

Santiago Pastorino (Apr 15 2020 at 18:38, on Zulip):

gonna add some stuff without great organization so we can continue and will fix later

Santiago Pastorino (Apr 15 2020 at 18:38, on Zulip):

anyway, let's move on

Santiago Pastorino (Apr 15 2020 at 18:39, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:39, on Zulip):

45 p-high and 25 unassigned

Santiago Pastorino (Apr 15 2020 at 18:40, on Zulip):

Stable to beta regressions

Santiago Pastorino (Apr 15 2020 at 18:40, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:41, on Zulip):

all are T-compiler

Santiago Pastorino (Apr 15 2020 at 18:42, on Zulip):

the P-high ones are assigned, P-medium are not

Santiago Pastorino (Apr 15 2020 at 18:42, on Zulip):

gonna add those to the agenda then

pnkfelix (Apr 15 2020 at 18:43, on Zulip):

the P-medium ones, that is?

Santiago Pastorino (Apr 15 2020 at 18:44, on Zulip):

yeah, just a little mention given that we are close to the release that those P-medium regressions are not assigned

Santiago Pastorino (Apr 15 2020 at 18:44, on Zulip):

or do you think it's not worth?

Santiago Pastorino (Apr 15 2020 at 18:44, on Zulip):

Stable to nightly regressions

Santiago Pastorino (Apr 15 2020 at 18:45, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:45, on Zulip):

there are 5, 2 p-high, 2 p-medium and one unprioritized

Santiago Pastorino (Apr 15 2020 at 18:45, on Zulip):

but it's T-rustdoc

pnkfelix (Apr 15 2020 at 18:45, on Zulip):

I just wanted to be clear

pnkfelix (Apr 15 2020 at 18:46, on Zulip):

(that you were talking about the unassigned P-mediums, and not the assigned P-highs)

Santiago Pastorino (Apr 15 2020 at 18:46, on Zulip):

in the stable to nightly case all of them are unassigned

Santiago Pastorino (Apr 15 2020 at 18:46, on Zulip):

noting that to the agenda too

Santiago Pastorino (Apr 15 2020 at 18:48, on Zulip):

Stable to stable regressions

Santiago Pastorino (Apr 15 2020 at 18:48, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:49, on Zulip):

72 stable to stable regressions and 26 unprioritized

Santiago Pastorino (Apr 15 2020 at 18:49, on Zulip):

@pnkfelix should we add this information to the agenda?

Santiago Pastorino (Apr 15 2020 at 18:49, on Zulip):

maybe as part of a summary with the P-high issues

Santiago Pastorino (Apr 15 2020 at 18:50, on Zulip):

I-nominated T-compiler

Santiago Pastorino (Apr 15 2020 at 18:50, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:50, on Zulip):

there are 3 nominations

pnkfelix (Apr 15 2020 at 18:50, on Zulip):

hmm. That's interesting about the unprioritized stable-to-stable regressions

pnkfelix (Apr 15 2020 at 18:50, on Zulip):

I should look at that list some time

DPC (Apr 15 2020 at 18:51, on Zulip):

i'm looking to minimise the list if possible as well :slight_smile:

Santiago Pastorino (Apr 15 2020 at 18:51, on Zulip):

do you want to try to add it to the agenda and see what happens? :)

Santiago Pastorino (Apr 15 2020 at 18:52, on Zulip):

Santiago Pastorino said:

there are 3 nominations

#70928 is in disposition to close, going to add it to the list with that text, we probably don't want to discuss this again :)

Santiago Pastorino (Apr 15 2020 at 18:52, on Zulip):

#70453 is in disposition to merge, going to add it to the list with that text too

Santiago Pastorino (Apr 15 2020 at 18:53, on Zulip):

Announcements

Santiago Pastorino (Apr 15 2020 at 18:53, on Zulip):
Santiago Pastorino (Apr 15 2020 at 18:53, on Zulip):

we have this https://github.com/rust-lang/compiler-team/issues/272

Santiago Pastorino (Apr 15 2020 at 18:55, on Zulip):

@simulacrum what was your idea with this?

Santiago Pastorino (Apr 15 2020 at 18:55, on Zulip):

have added it to the agenda and removed the label

Santiago Pastorino (Apr 15 2020 at 18:56, on Zulip):

I'm not sure I follow what do you want to say, maybe edit the agenda directly

Santiago Pastorino (Apr 15 2020 at 19:00, on Zulip):

ok, was adding some stuff to the agenda

Santiago Pastorino (Apr 15 2020 at 19:01, on Zulip):

ok moving on

Santiago Pastorino (Apr 15 2020 at 19:01, on Zulip):

Toolstate

Santiago Pastorino (Apr 15 2020 at 19:01, on Zulip):

Check toolstate for outstanding tool breakage.

Wesley Wiser (Apr 15 2020 at 19:01, on Zulip):

I think the thing with compiler-team#272 was just to announce the process of using rustbot to second issues

Santiago Pastorino (Apr 15 2020 at 19:01, on Zulip):

yeah, I think so

Wesley Wiser (Apr 15 2020 at 19:03, on Zulip):

Seems there's pending PRs that fix the broken tools

Santiago Pastorino (Apr 15 2020 at 19:03, on Zulip):

I guess it may be a good idea to add a little explanation

Santiago Pastorino (Apr 15 2020 at 19:03, on Zulip):

well was going to say

Santiago Pastorino (Apr 15 2020 at 19:03, on Zulip):

bunch of broken tools

Santiago Pastorino (Apr 15 2020 at 19:03, on Zulip):

clippy, miri, rls and rustfmt

Santiago Pastorino (Apr 15 2020 at 19:04, on Zulip):

one of the things that I wonder is ... we note that during this meeting and what should we do?

Santiago Pastorino (Apr 15 2020 at 19:04, on Zulip):

should we notify somebody about this?

Santiago Pastorino (Apr 15 2020 at 19:04, on Zulip):

in case people are not already aware about this

DPC (Apr 15 2020 at 19:04, on Zulip):

there are prs in the queue

Santiago Pastorino (Apr 15 2020 at 19:04, on Zulip):

ok, but if there weren't?

DPC (Apr 15 2020 at 19:05, on Zulip):

then yeah you can notify

Santiago Pastorino (Apr 15 2020 at 19:05, on Zulip):

notify who?

Santiago Pastorino (Apr 15 2020 at 19:05, on Zulip):

I'm trying to come up with a well defined process

Santiago Pastorino (Apr 15 2020 at 19:05, on Zulip):

in the document we have ... let's check toolstate

Santiago Pastorino (Apr 15 2020 at 19:05, on Zulip):

but nothing else

DPC (Apr 15 2020 at 19:05, on Zulip):

i can notify the corresponding teams

Santiago Pastorino (Apr 15 2020 at 19:05, on Zulip):

:+1:

DPC (Apr 15 2020 at 19:06, on Zulip):

do we do it every week or just the week before release?

Santiago Pastorino (Apr 15 2020 at 19:06, on Zulip):

@DPC can you add who/how do you notify teams to the procedure document?

Santiago Pastorino (Apr 15 2020 at 19:06, on Zulip):

DPC said:

do we do it every week or just the week before release?

that's what I meant as defining a process :)

DPC (Apr 15 2020 at 19:07, on Zulip):

rsure

Santiago Pastorino (Apr 15 2020 at 19:07, on Zulip):

what we are doing with this is ill-defined

Santiago Pastorino (Apr 15 2020 at 19:07, on Zulip):

maybe our process could be, just check 1 or 2 weeks before the release and not the rest of the time and then if things are broken notify all the teams

Santiago Pastorino (Apr 15 2020 at 19:08, on Zulip):

but I guess in order to do this quickly would be nice to have documented properly how/who should we notify

Santiago Pastorino (Apr 15 2020 at 19:08, on Zulip):

if you're adding that info great

Santiago Pastorino (Apr 15 2020 at 19:08, on Zulip):

anyway, let's continue

DPC (Apr 15 2020 at 19:08, on Zulip):

notifying is just leaving a message in the corresponding team channel

Santiago Pastorino (Apr 15 2020 at 19:08, on Zulip):

:+1:

Santiago Pastorino (Apr 15 2020 at 19:08, on Zulip):

maybe we can do a little list miri -> Discord #something

Santiago Pastorino (Apr 15 2020 at 19:08, on Zulip):

etc

Santiago Pastorino (Apr 15 2020 at 19:09, on Zulip):

to be honest I have no idea where do most of the teams hangout

Santiago Pastorino (Apr 15 2020 at 19:09, on Zulip):

Performance regressions

Santiago Pastorino (Apr 15 2020 at 19:09, on Zulip):

Santiago Pastorino said:

maybe we can do a little list miri -> Discord #something

this will be great for when we have all this automated :)

Santiago Pastorino (Apr 15 2020 at 19:09, on Zulip):

Check perf regressions.

Santiago Pastorino (Apr 15 2020 at 19:11, on Zulip):

image.png

Santiago Pastorino (Apr 15 2020 at 19:11, on Zulip):

unsure what happened there

Santiago Pastorino (Apr 15 2020 at 19:12, on Zulip):

https://perf.rust-lang.org/compare.html?start=1edcfc83c6a08ddc5e63fc652b149baea0236e7c&end=d249d756374737eb014079901ac132f1e1ed905e&stat=instructions:u

Santiago Pastorino (Apr 15 2020 at 19:14, on Zulip):

https://github.com/rust-lang/rust/compare/1edcfc83c6a08ddc5e63fc652b149baea0236e7c...d249d756374737eb014079901ac132f1e1ed905e

Santiago Pastorino (Apr 15 2020 at 19:14, on Zulip):

what happened here?

Santiago Pastorino (Apr 15 2020 at 19:14, on Zulip):

I meant, is someone aware of this?

Yuki Okushi (Apr 15 2020 at 19:16, on Zulip):

hm, related to #70565?

Yuki Okushi (Apr 15 2020 at 19:18, on Zulip):

Santiago Pastorino said:

I meant, is someone aware of this?

I'm not sure about this though

Santiago Pastorino (Apr 15 2020 at 19:19, on Zulip):

Yuki Okushi said:

hm, related to #70565?

no idea, but do you think that inlining those functions made this test regress that bad?

Santiago Pastorino (Apr 15 2020 at 19:19, on Zulip):

if so, why?

DPC (Apr 15 2020 at 19:20, on Zulip):

how do we confirm?

Santiago Pastorino (Apr 15 2020 at 19:21, on Zulip):

we could run perf tests on each of those intermediate commits

Santiago Pastorino (Apr 15 2020 at 19:21, on Zulip):

just in case, I meant locally

Santiago Pastorino (Apr 15 2020 at 19:22, on Zulip):

I don't think it's possible to do it in our cloud infraestructure now that things are already merged

DPC (Apr 15 2020 at 19:23, on Zulip):

we could revert it, re-commit and run perf tests if needed

Yuki Okushi (Apr 15 2020 at 19:23, on Zulip):

Santiago Pastorino said:

Yuki Okushi said:

hm, related to #70565?

no idea, but do you think that inlining those functions made this test regress that bad?

it's just my rough guess, since it's related to vector thing

Santiago Pastorino (Apr 15 2020 at 19:24, on Zulip):

unsure to be honest

Santiago Pastorino (Apr 15 2020 at 19:24, on Zulip):

@simulacrum ?

Wesley Wiser (Apr 15 2020 at 19:25, on Zulip):

I think it's very likely that it's the #[inline] that got added to into_vec()

DPC (Apr 15 2020 at 19:25, on Zulip):

(mark isn't online)

Wesley Wiser (Apr 15 2020 at 19:26, on Zulip):

deep-vec-opt is extremely sensitive to inlining

Santiago Pastorino (Apr 15 2020 at 19:28, on Zulip):

there are like 4 or 5 regressions in performance

Santiago Pastorino (Apr 15 2020 at 19:28, on Zulip):

unsure if you spotted all of them

Santiago Pastorino (Apr 15 2020 at 19:28, on Zulip):

"minor" I'd say

Santiago Pastorino (Apr 15 2020 at 19:29, on Zulip):

also ... this is another area where I'm not sure what process we should follow

Santiago Pastorino (Apr 15 2020 at 19:29, on Zulip):

cc @pnkfelix

Santiago Pastorino (Apr 15 2020 at 19:29, on Zulip):

maybe note this kind of things in the agenda?

DPC (Apr 15 2020 at 19:37, on Zulip):

anything else to discuss?

Santiago Pastorino (Apr 15 2020 at 19:39, on Zulip):

that's all, thanks everyone :wave:

Santiago Pastorino (Apr 15 2020 at 19:40, on Zulip):

I'll be waiting for more comments about performance and gonna shape a little better the agenda

DPC (Apr 15 2020 at 19:40, on Zulip):

i've added the toolstate part to the procedure

Santiago Pastorino (Apr 15 2020 at 19:41, on Zulip):

great @DPC thanks!

Yuki Okushi (Apr 16 2020 at 12:48, on Zulip):

Wesley Wiser said:

I think it's very likely that it's the #[inline] that got added to into_vec()

@Wesley Wiser Do you think it's worth reverting it (or just removing the attr from into_vec) and run perf checks?

Wesley Wiser (Apr 16 2020 at 13:27, on Zulip):

@Yuki Okushi The rest of the changes look good to me so I'd try just reverting the into_vec() inline and do a perf run.

Yuki Okushi (Apr 16 2020 at 13:28, on Zulip):

@Wesley Wiser Sounds good! So I'll open a PR for it.

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

have zero context about this, but does the inline of into_vec helped in some way in other scenarios?

Santiago Pastorino (Apr 16 2020 at 13:30, on Zulip):

I meant, unsure why this was done but did we win something and this is the drawback or we are just regressing and this needs to be reverted?

Wesley Wiser (Apr 17 2020 at 16:08, on Zulip):

There were a few small regressions from undoing the inlining (see here). I think, at least for now, we should return to the previous status quo. If we decide that regressing deep-vec is worth it, then we can always add it back but it doesn't look to me like anyone consciously made that decision.

Santiago Pastorino (Apr 17 2020 at 19:59, on Zulip):

it seems better undoing inlining to me

Santiago Pastorino (Apr 17 2020 at 20:00, on Zulip):

in any case may be good idea to get involved the creator of the original PR, don't remember if it was Zoxc or who

pnkfelix (Apr 18 2020 at 00:29, on Zulip):

it was Zoxc, who is taking a break

pnkfelix (Apr 18 2020 at 00:30, on Zulip):

and wesley has already r+'ed the reverting PR

pnkfelix (Apr 18 2020 at 00:31, on Zulip):

we can independently investigate whether there is a more targeted fix to achieve the goals of PR #70565

Wesley Wiser (Apr 18 2020 at 00:53, on Zulip):

pnkfelix said:

and wesley has already r+'ed the reverting PR

I'm happy to remove my r+ if we want to discuss this more first; it just seemed the correct thing to do to me.

Last update: Jun 05 2020 at 22:55UTC