Skip to content

use invokelatest to prevent invalidations in TOML #48083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 2, 2023
Merged

use invokelatest to prevent invalidations in TOML #48083

merged 1 commit into from
Jan 2, 2023

Conversation

KristofferC
Copy link
Member

#47876 added some 400 invalidations to using CSV. This removes them.

@KristofferC KristofferC added latency Latency backport 1.9 Change should be backported to release-1.9 labels Jan 2, 2023
@KristofferC KristofferC requested a review from timholy January 2, 2023 15:42
@KristofferC KristofferC merged commit 79e29e3 into master Jan 2, 2023
@KristofferC KristofferC deleted the kc/toml_invalid branch January 2, 2023 22:17
@quinnj
Copy link
Member

quinnj commented Jan 3, 2023

I don't immediately see how #47876 caused CSV invalidations? Can someone enlighten me?

@KristofferC
Copy link
Member Author

all(v -> isa(v, AbstractDict), value) was called with inprecise inference information on value so when someone else defined an all method that previous cached method got invalidated and everything calling that (recursively) gets invalidated. The @invokelatest "severs" the inference chain so that no optimizations are made based on this inprecise inference information and thereby there are no invalidations.

@quinnj
Copy link
Member

quinnj commented Jan 4, 2023

Ah, I see, thanks for the example.

@KristofferC KristofferC mentioned this pull request Jan 10, 2023
Merged
41 tasks
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timholy timholy Awaiting requested review from timholy

Assignees
No one assigned
Labels
latency Latency
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KristofferC @quinnj