Stream: t-compiler

Topic: Removing DUMMY_HIR_ID


marmeladema (Apr 12 2020 at 17:49, on Zulip):

I made a first attempt at removing small usage of DUMMY_HIR_ID: https://github.com/rust-lang/rust/pull/71069

marmeladema (Apr 12 2020 at 17:49, on Zulip):

(tests are compiling locally right now, so i don't know if they pass yet)

eddyb (Apr 12 2020 at 18:32, on Zulip):

heh you got there before me

eddyb (Apr 12 2020 at 18:32, on Zulip):

:+1: on ObligationCause::dummy()

eddyb (Apr 12 2020 at 18:36, on Zulip):

idk if I should do a PSA but "CI / PR (x86_64-gnu-llvm-7) (pull_request) Successful in 41m " is the thing to look for in general

eddyb (Apr 12 2020 at 18:37, on Zulip):

it's faster than the same checks running on the older Azure

marmeladema (Apr 12 2020 at 18:37, on Zulip):

Yep, i looked at it and all the tests pass. Azure build takes 120m :fear:

marmeladema (Apr 12 2020 at 18:38, on Zulip):

There is also this other quick one: https://github.com/rust-lang/rust/pull/71071

eddyb (Apr 12 2020 at 18:39, on Zulip):

3x speedup for 4x the cores I guess :D

eddyb (Apr 12 2020 at 18:39, on Zulip):

@marmeladema ping me when this second one passes too :P

eddyb (Apr 12 2020 at 18:40, on Zulip):

actually in general, ping me here because I'll see it faster than in GH notifications

marmeladema (Apr 12 2020 at 18:41, on Zulip):

Will do, thanks! I might have one or two other small ones

eddyb (Apr 12 2020 at 18:41, on Zulip):

it might be worth combining them into one PR?

eddyb (Apr 12 2020 at 18:41, on Zulip):

since they share in common the removal of DUMMY_HIR_ID uses

marmeladema (Apr 12 2020 at 18:41, on Zulip):

yep, sorry. I split the one for ObligationCause because i was not sure

marmeladema (Apr 12 2020 at 18:42, on Zulip):

But yes, i will merge all the subsequent ones if they are straightforward

eddyb (Apr 12 2020 at 18:52, on Zulip):

you can even combine them into the PR I approved and I can just reapprove

marmeladema (Apr 12 2020 at 20:06, on Zulip):

I added more commits to https://github.com/rust-lang/rust/pull/71069

eddyb (Apr 12 2020 at 20:21, on Zulip):

I'm still awake so I'll review now

marmeladema (Apr 12 2020 at 20:28, on Zulip):

Thanks

eddyb (Apr 12 2020 at 20:29, on Zulip):

@marmeladema do you have more commits coming?

marmeladema (Apr 12 2020 at 20:31, on Zulip):

Not right now

eddyb (Apr 12 2020 at 20:31, on Zulip):

I'll r+ once it passes

marmeladema (Apr 12 2020 at 20:31, on Zulip):

I hope it passes :/

eddyb (Apr 12 2020 at 20:33, on Zulip):

I don't see anything problematic

marmeladema (Apr 12 2020 at 20:44, on Zulip):

it passes :tada:

marmeladema (Apr 13 2020 at 10:03, on Zulip):

Thanks! It got merged pretty quickly! I am king a now blocked on removing it from librustc_ast_lowering. A table that maps node id to hir id is pre allocated in lower_node_id_generic. I tried to change to type of that IndexVec to store Option<HirId> instead of bare HirId and then filter out unused value when calling init_node_id_to_hir_id_mapping but that breaks because there is another table node_id_to_def_id that maps node id to def id and because i removed unused entries in the first table, the indexes do not match anymore in this second table

marmeladema (Apr 13 2020 at 10:05, on Zulip):

I could just store Option's too in librustc_hir but that would mean either returning Option everywhere or calling unwrap() before returning which might impact perf

marmeladema (Apr 13 2020 at 10:07, on Zulip):

Or I could change the IndexVec to FxHashMap? Dunno if thats a good idea either

eddyb (Apr 13 2020 at 10:22, on Zulip):

@marmeladema hmm what if you move DUMMY_HIR_ID inside the function that it's used in? unless it's used in more than one file?

marmeladema (Apr 13 2020 at 10:24, on Zulip):

well, i could but it means that it will be really stored as value in the table and that it can be "leaked" to other module that access the table using node_id_to_hir_id or local_def_id_to_hir_id

eddyb (Apr 13 2020 at 10:25, on Zulip):

okay now that your PR is merged I can refresh my search and see if I can understand what is going on

marmeladema (Apr 13 2020 at 10:27, on Zulip):

So from what i understand, during lowering DUMMY_HIR_ID is used to pre-allocate hir ids because lower_node_id_generic is not called with consecutive ast_node_id but it is used as an index in that table, so it fills the table to "empty entries" and use the DUMMY_HIR_ID as value

eddyb (Apr 13 2020 at 10:28, on Zulip):

and then the hir_id_validator ensures there are no dummy entries left

marmeladema (Apr 13 2020 at 10:28, on Zulip):

Oh i'll look to that one, i have missed it

eddyb (Apr 13 2020 at 10:28, on Zulip):

@marmeladema so I think you can use Option

eddyb (Apr 13 2020 at 10:29, on Zulip):

Option<HirId> will be the same size as HirId

eddyb (Apr 13 2020 at 10:29, on Zulip):

(thanks to some niche tricks we do with index types)

marmeladema (Apr 13 2020 at 10:29, on Zulip):

i did try the Option<> but it failed miserably because there are some empty entries somehow

eddyb (Apr 13 2020 at 10:29, on Zulip):

oh!1

eddyb (Apr 13 2020 at 10:30, on Zulip):

but how does hir_id_validator not see them later?

marmeladema (Apr 13 2020 at 10:30, on Zulip):

thats what i want to look at because either my branch was broken or something is wrong somewhere. I did not know about hir_id_validator last night, i am taking a look

marmeladema (Apr 13 2020 at 11:39, on Zulip):

So, i am not sure of anything, but reading the hir_id_validator code, i believe that it iterates over all the modules and look at the hir tree of those modules. But if we look at the flat list of hir ids, i think there are still empty entries in it even if they might not be reachable from any module tree.

marmeladema (Apr 13 2020 at 11:43, on Zulip):

Also, I found this comment:

    // FIXME(eddyb) don't go through `ast::NodeId` to convert between `HirId`
    // and `LocalDefId` - ideally all `LocalDefId`s would be HIR owners.
    node_id_to_def_id: FxHashMap<ast::NodeId, LocalDefId>,
eddyb (Apr 13 2020 at 11:47, on Zulip):

oh yeah I still need to work on that sigh

eddyb (Apr 13 2020 at 11:48, on Zulip):

@marmeladema unreachable entries may explain it, hmpf. but still, it's kind of reckless of us to even have those

marmeladema (Apr 13 2020 at 11:49, on Zulip):

yep, but that would explain when i removed that it broke badly because I basically "re-numbered" node id and the node id to def id table was out of sync

eddyb (Apr 13 2020 at 11:54, on Zulip):

what do you mean "removed"?

eddyb (Apr 13 2020 at 11:54, on Zulip):

the resize call?

marmeladema (Apr 13 2020 at 11:54, on Zulip):

I found the code a bit convoluted because some lookup table are stored globally in the (dyn Resolver).definitions() and some locally in the current LoweringContext

eddyb (Apr 13 2020 at 11:55, on Zulip):

I am pretty sure this can use Option https://github.com/rust-lang/rust/blob/0c835b0cca83fe21090562603e4bda77c183ace3/src/librustc_ast_lowering/lib.rs#L581-L586

marmeladema (Apr 13 2020 at 11:55, on Zulip):

So what i did is to change the type of node_id_to_hir_id in LoweringContext to an indexvec of Option<HirId> instead of bare HirId

eddyb (Apr 13 2020 at 11:55, on Zulip):

oh wait it's passed to init_node_id_to_hir_id_mapping hmpf

marmeladema (Apr 13 2020 at 11:56, on Zulip):

then in the call of init_node_id_to_hir_id_mapping i filter_map to remove the "unused entries"

eddyb (Apr 13 2020 at 11:56, on Zulip):

aaaaaah

eddyb (Apr 13 2020 at 11:56, on Zulip):

yeah no it's a map

eddyb (Apr 13 2020 at 11:56, on Zulip):

the indices are "keys"

marmeladema (Apr 13 2020 at 11:56, on Zulip):

yeah i figured that

eddyb (Apr 13 2020 at 11:56, on Zulip):

okay so this is the problem https://github.com/rust-lang/rust/blob/0c835b0cca83fe21090562603e4bda77c183ace3/src/librustc_hir/definitions.rs#L478

marmeladema (Apr 13 2020 at 11:56, on Zulip):

but in the hir, i don't want Optio<HirId> anymore, do i?

eddyb (Apr 13 2020 at 11:57, on Zulip):

now I think I'm caught up

marmeladema (Apr 13 2020 at 11:57, on Zulip):

yeah I replaced this line to something like self.node_id_to_hir_id = mapping.into_iter().filter_map(|hir_id| hir_id).collect();

marmeladema (Apr 13 2020 at 11:58, on Zulip):

to remove the unused entry and avoid changing the type of self.node_id_to_hir_id in the hir

eddyb (Apr 13 2020 at 11:59, on Zulip):

have you tried .map(Option::unwrap) :D?

eddyb (Apr 13 2020 at 11:59, on Zulip):

since that's what I thought broke

eddyb (Apr 13 2020 at 11:59, on Zulip):

oh wait that's the same failure condition facepalm

eddyb (Apr 13 2020 at 11:59, on Zulip):

just detected earlier

marmeladema (Apr 13 2020 at 11:59, on Zulip):

I haven't but because it broke with filter_map it means i removed some entries and re number the ast node id

eddyb (Apr 13 2020 at 11:59, on Zulip):

right right

marmeladema (Apr 13 2020 at 11:59, on Zulip):

yep

eddyb (Apr 13 2020 at 11:59, on Zulip):

did it broke during building libstd?

eddyb (Apr 13 2020 at 12:00, on Zulip):

wait hang on this just means there are NodeIds with no corresponding HirId

marmeladema (Apr 13 2020 at 12:00, on Zulip):

it broke early during stage1 iirc

eddyb (Apr 13 2020 at 12:00, on Zulip):

this is normal

eddyb (Apr 13 2020 at 12:00, on Zulip):

AST->HIR lowering replaces some constructs

eddyb (Apr 13 2020 at 12:00, on Zulip):

like (a+b) is just a+b after AST->HIR lowering

eddyb (Apr 13 2020 at 12:01, on Zulip):

so yeah you have to change https://github.com/rust-lang/rust/blob/0c835b0cca83fe21090562603e4bda77c183ace3/src/librustc_hir/definitions.rs#L90

marmeladema (Apr 13 2020 at 12:01, on Zulip):

Yes, that makes sense, but maybe we should not use an indexvec but really a fxhashmap?

eddyb (Apr 13 2020 at 12:01, on Zulip):

every place where you can use a flat array with decent occupancy is a perf win

eddyb (Apr 13 2020 at 12:02, on Zulip):

and Option<HirId> is the same size as HirId

marmeladema (Apr 13 2020 at 12:02, on Zulip):

I figured thats why it was used for yes, perf

eddyb (Apr 13 2020 at 12:02, on Zulip):

anyway, eventually this will be used less and less and will matter less and less

marmeladema (Apr 13 2020 at 12:03, on Zulip):

But if i replace that with Option<HirId>, then i need to either return Options in the related functions
or unwrap() before returning

eddyb (Apr 13 2020 at 12:03, on Zulip):

the latter is correct

eddyb (Apr 13 2020 at 12:03, on Zulip):

and see what breaks

marmeladema (Apr 13 2020 at 12:03, on Zulip):

and i suspect calling unwrap() in each call might impact perf?

eddyb (Apr 13 2020 at 12:03, on Zulip):

anything that breaks will want opt_ versions of the regular methods

marmeladema (Apr 13 2020 at 12:03, on Zulip):

ok

eddyb (Apr 13 2020 at 12:03, on Zulip):

it might, but that's why we'll check

eddyb (Apr 13 2020 at 12:03, on Zulip):

first priority is to get something that compiles

marmeladema (Apr 13 2020 at 12:04, on Zulip):

Ok

eddyb (Apr 13 2020 at 12:04, on Zulip):

ideally nothing uses NodeIds although I know that's not exactly true :P

marmeladema (Apr 13 2020 at 12:06, on Zulip):

I'll try that then!

marmeladema (Apr 13 2020 at 13:32, on Zulip):

On a similar topic, i made to remove some usage of DUMMY_HIR_ID again: https://github.com/rust-lang/rust/pull/71092

marmeladema (Apr 13 2020 at 13:32, on Zulip):

test pass on CI, but locally, rustdoc tests are failing

marmeladema (Apr 13 2020 at 13:32, on Zulip):

How is that possible? oO

eddyb (Apr 13 2020 at 13:33, on Zulip):

@marmeladema can you share your local failures?

eddyb (Apr 13 2020 at 13:34, on Zulip):

also you can put r? @eddyb in the PR description

marmeladema (Apr 13 2020 at 13:35, on Zulip):

I have another commit that i removed because i thought it was responsible for rustdoc test failures, but i reran locally without it and its still failing

marmeladema (Apr 13 2020 at 13:36, on Zulip):

https://paste.ee/p/PaE4g

eddyb (Apr 13 2020 at 13:37, on Zulip):

that's.... weird

eddyb (Apr 13 2020 at 13:37, on Zulip):

what was the commit?

marmeladema (Apr 13 2020 at 13:37, on Zulip):

I just added it back to the PR

eddyb (Apr 13 2020 at 13:38, on Zulip):

this one? https://github.com/rust-lang/rust/pull/71092/commits/23da8057990355291235b8b8eaf164806111f20f

marmeladema (Apr 13 2020 at 13:39, on Zulip):

Yes, but in fact, no matter what commit I add, i feel like my rustdoc tests are failing

marmeladema (Apr 13 2020 at 13:39, on Zulip):

I'll try to run them from master and see what happens

marmeladema (Apr 13 2020 at 13:41, on Zulip):

azure tests are failing because of some error getting python-setuptools

marmeladema (Apr 13 2020 at 13:41, on Zulip):
E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/p/python-setuptools/python3-setuptools_20.7.0-1_all.deb  503  Service Unavailable
eddyb (Apr 13 2020 at 13:41, on Zulip):

lol

eddyb (Apr 13 2020 at 13:42, on Zulip):

I still don't understand how "CI network errors" are still a thing

marmeladema (Apr 13 2020 at 13:44, on Zulip):

Can they be rerun?

eddyb (Apr 13 2020 at 13:44, on Zulip):

cc @simulacrum @Pietro Albini

simulacrum (Apr 13 2020 at 13:44, on Zulip):

generally those errors mean we just have to wait unfortunately

marmeladema (Apr 13 2020 at 14:31, on Zulip):

Ok, tests are failing locally on master also. Very weird, but expect for the azure tests, everything looks good for my PR

eddyb (Apr 13 2020 at 14:32, on Zulip):

yeah so something broke locally

eddyb (Apr 13 2020 at 14:32, on Zulip):

maybe Python?

marmeladema (Apr 13 2020 at 14:33, on Zulip):

yeah i'll dig into the htmldocck.py script

eddyb (Apr 13 2020 at 14:34, on Zulip):

I think we changed something about which Python we support

marmeladema (Apr 13 2020 at 14:34, on Zulip):

Whats funny is that i started to remove DUMMY_HIR_ID because of those failing test. I assumed my code for LocalDefId was responsible about those failures lol

marmeladema (Apr 13 2020 at 14:34, on Zulip):

oh right! I saw that!

marmeladema (Apr 13 2020 at 14:35, on Zulip):

You are right, if i force running the tests using python3 it works fine

marmeladema (Apr 13 2020 at 14:35, on Zulip):

silly me

eddyb (Apr 13 2020 at 14:36, on Zulip):

haha

marmeladema (Apr 13 2020 at 14:36, on Zulip):

is there some kind of python venv to remove?

eddyb (Apr 13 2020 at 14:36, on Zulip):

marmeladema said:

Whats funny is that i started to remove DUMMY_HIR_ID because of those failing test. I assumed my code for LocalDefId was responsible about those failures lol

oh I thought it was ICE-ing!

marmeladema (Apr 13 2020 at 14:37, on Zulip):

No, just tests failing in rustdoc ... But anyway, removing DUMMY_HIR_ID is a good thing no?

eddyb (Apr 13 2020 at 14:38, on Zulip):

yeah

eddyb (Apr 13 2020 at 14:38, on Zulip):

just unfortunate about the test failure reason confusion

marmeladema (Apr 13 2020 at 14:39, on Zulip):

I'll do a separate PR for the change in ast_lowering+hir just in case it impacts performance

marmeladema (Apr 13 2020 at 16:08, on Zulip):

So ... I changed the node_id_to_hir_id to a vec of Option<HirId> and unwrap-ing in local_def_id_to_hir_idleads to the following https://paste.ee/p/py82b

marmeladema (Apr 13 2020 at 16:08, on Zulip):

It means that some DUMMY_HIR_ID are leaked somehow

marmeladema (Apr 13 2020 at 16:10, on Zulip):

Well actually i think its self.def_id_to_node_id[id] that returns a node_id that does not have a corresponding hir id

eddyb (Apr 13 2020 at 16:14, on Zulip):

@marmeladema no, that's perfect!

eddyb (Apr 13 2020 at 16:15, on Zulip):

we want only save-analysis to fail, if anything fails

eddyb (Apr 13 2020 at 16:15, on Zulip):

@marmeladema because it does this weird thing where it looks things up by NodeIds

marmeladema (Apr 13 2020 at 16:15, on Zulip):

Here are the 4 tests that fail:

failures:
    [ui] ui/generator/async-generator-issue-67158.rs
    [ui] ui/save-analysis/issue-68621.rs
    [ui] ui/type-alias-impl-trait/issue-63279.rs
    [ui] ui/type-alias-impl-trait/issue-65679-inst-opaque-ty-from-val-twice.rs
marmeladema (Apr 13 2020 at 16:16, on Zulip):

its not only save-analysis unfortunately

eddyb (Apr 13 2020 at 16:19, on Zulip):
#0 [has_typeck_tables] processing `<Struct as Service>::Future::{{opaque}}#0`
eddyb (Apr 13 2020 at 16:20, on Zulip):

heh https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/check/mod.rs#L842

marmeladema (Apr 13 2020 at 16:21, on Zulip):

For ui/type-alias-impl-trait/issue-63279.rsI get

#0 [has_typeck_tables] processing `Closure::{{opaque}}#0`
eddyb (Apr 13 2020 at 16:21, on Zulip):

okay this is suspicious

eddyb (Apr 13 2020 at 16:22, on Zulip):

anyway, you can add a variant of as_local_hir_id that has an opt_ prefix or _opt suffix

eddyb (Apr 13 2020 at 16:22, on Zulip):

that returns Option

eddyb (Apr 13 2020 at 16:22, on Zulip):

and only a few places use

marmeladema (Apr 13 2020 at 16:23, on Zulip):

I can but from what i understand such function would return None, if the def_id is not a local one. Not if the def_id as no hir_id associated

eddyb (Apr 13 2020 at 16:23, on Zulip):

hmm

eddyb (Apr 13 2020 at 16:24, on Zulip):

@marmeladema okay we can manufacture a HirId :P

eddyb (Apr 13 2020 at 16:24, on Zulip):

and hopefully nothing breaks

marmeladema (Apr 13 2020 at 16:25, on Zulip):

lol okay^^

eddyb (Apr 13 2020 at 16:25, on Zulip):
HirId {
    owner: def_id,
    local_id: ItemLocalId::from_u32(0),
}
marmeladema (Apr 13 2020 at 16:25, on Zulip):

I just dont want to be responsible for hiding an underlying bug somewhere^^

eddyb (Apr 13 2020 at 16:26, on Zulip):

eventually all LocalDefId -> HirId conversions will be just this

eddyb (Apr 13 2020 at 16:26, on Zulip):

I even have a branch somewhere that does this for everything except closures because those are a bit of a pain

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

@marmeladema we can open an issue about a LocalDefId existing that doesn't have any HirId

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

and mention it in a comment on the hack

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

Ok, i am fine with that

marmeladema (Apr 13 2020 at 16:32, on Zulip):

Are you already creating the issue or shall I do it?

eddyb (Apr 13 2020 at 16:34, on Zulip):

you can do it if you want

marmeladema (Apr 13 2020 at 16:35, on Zulip):

Ok I'll try then :)

marmeladema (Apr 13 2020 at 16:37, on Zulip):

About https://github.com/rust-lang/rust/pull/71092, is this good to go?

eddyb (Apr 13 2020 at 16:37, on Zulip):

oh yeah okay just r+'d it

marmeladema (Apr 13 2020 at 16:38, on Zulip):

Cool thanks!

marmeladema (Apr 13 2020 at 17:07, on Zulip):

I've removed DUMMY_HIR_ID locally entirely, lets see how many things break :D

marmeladema (Apr 13 2020 at 17:58, on Zulip):

Hum only one failure for:

failures:
    [ui] ui/generator/async-generator-issue-67158.rs
marmeladema (Apr 13 2020 at 17:59, on Zulip):

But it was already broken with the change to the node_id_to_hir_id

marmeladema (Apr 13 2020 at 17:59, on Zulip):

https://paste.ee/p/0cGzW

marmeladema (Apr 13 2020 at 18:02, on Zulip):

Its failing here: https://github.com/rust-lang/rust/blob/master/src/librustc_middle/ty/context.rs#L1129

marmeladema (Apr 13 2020 at 18:02, on Zulip):

Some node_id don't have a corresponding hir_id

marmeladema (Apr 13 2020 at 18:06, on Zulip):

Maybe the trait_map is wrong/corrupted somehow?

eddyb (Apr 13 2020 at 18:11, on Zulip):

huh that's really suspicious

eddyb (Apr 13 2020 at 18:12, on Zulip):

@marmeladema I think you can just ignore NodeIds w/o a HirId

eddyb (Apr 13 2020 at 18:12, on Zulip):

that just means they weren't lowered

eddyb (Apr 13 2020 at 18:13, on Zulip):

it's possibly a bug, but it didn't do anything before anyway

marmeladema (Apr 13 2020 at 18:13, on Zulip):

Ok

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

By the way, I just added one last commit in https://github.com/rust-lang/rust/pull/71092

marmeladema (Apr 13 2020 at 18:17, on Zulip):

@eddyb do you need to approve again in this case? I don't know if how bors work in this case

eddyb (Apr 13 2020 at 18:20, on Zulip):

yupp, thanks for the ping

marmeladema (Apr 13 2020 at 18:37, on Zulip):

Do i have to wait for stage2 to be built to run some tests? It really takes a long time on my laptop.

eddyb (Apr 13 2020 at 18:38, on Zulip):

oh drat I should've asked

eddyb (Apr 13 2020 at 18:38, on Zulip):

@marmeladema ./x.py test --stage 1 src/test/ui

eddyb (Apr 13 2020 at 18:38, on Zulip):

this should cover most of the testing

eddyb (Apr 13 2020 at 18:39, on Zulip):

and set incremental = true in config.toml

marmeladema (Apr 13 2020 at 18:41, on Zulip):

hu ok lets try that

marmeladema (Apr 13 2020 at 18:59, on Zulip):

GHA tests passed

marmeladema (Apr 13 2020 at 21:42, on Zulip):

Thank you so much for the advise for --stage 1 and incremental, its way faster

marmeladema (Apr 13 2020 at 21:42, on Zulip):

And I have a branch passing all tests without DUMMY_HIR_ID

eddyb (Apr 13 2020 at 21:44, on Zulip):

nice :D

marmeladema (Apr 13 2020 at 21:47, on Zulip):

there are actually 3 places where either a DefId or a NodeId does not have a corresponding HirId. There must a bug somewhere for sure, but i am unable to find/fix it with my current understanding of the codebase. In the meantime, I created https://github.com/rust-lang/rust/issues/71104 and i'll update the tickets once I have opened the PR.

marmeladema (Apr 14 2020 at 08:52, on Zulip):

@eddyb this is it: https://github.com/rust-lang/rust/pull/71116

marmeladema (Apr 14 2020 at 08:52, on Zulip):

I don't know if a perf run is worth it?

eddyb (Apr 14 2020 at 08:56, on Zulip):

eh let's just do it

marmeladema (Apr 14 2020 at 11:42, on Zulip):

:fingers_crossed:

marmeladema (Apr 14 2020 at 14:45, on Zulip):

https://perf.rust-lang.org/compare.html?start=513a6473d69b3af34e6cdaa4efb288fe5283c3e9&end=433e90a8704a6c96d2560173ac41b079b0e58442

marmeladema (Apr 14 2020 at 14:46, on Zulip):

Does not seem very good? :thinking:

marmeladema (Apr 14 2020 at 14:59, on Zulip):

@eddyb actually i don't really know how to read the results and what is acceptable

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

@marmeladema there's no change I don't think

eddyb (Apr 14 2020 at 15:02, on Zulip):

the two at the top are just noise

eddyb (Apr 14 2020 at 15:02, on Zulip):

okay the rest have a slight slowdown too, but it's like 0.5%, I think it's acceptable

marmeladema (Apr 14 2020 at 15:04, on Zulip):

Cool! And hopefully the perf will be back with the LocalDefId work

marmeladema (Apr 15 2020 at 10:25, on Zulip):

Merged :tada:

DPC (Apr 15 2020 at 11:21, on Zulip):

if you want more issues to work on, feel free to ping me :slight_smile:

marmeladema (Apr 15 2020 at 11:31, on Zulip):

I can come back to the first one^^ removing DUMMY_HIR_ID was a side project to better finish the original one

marmeladema (Apr 16 2020 at 09:28, on Zulip):

I polished a bit the description for https://github.com/rust-lang/rust/issues/71104 and removed the [DRAFT] tag.

Last update: Jun 04 2020 at 18:00UTC