Skip to content
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.

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

bail out call inference when return type is maximized #48878

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Mar 3, 2023

We made the condition to bail out call inference a bit more strict in #48263 by looking at the inferred effects. It turns out that it slows down package loading time as reported at #48811.

This commit simply remove the condition. In particular, the loading time of CairoMakie is reduced to 40s from 48s (on master).

External AbstractInterpreter can use the current condition by overloading bail_out_call.

We made the condition to bail out call inference a bit more strict
in #48263 by looking at the inferred effects. It turns out that it slows
down package loading time as reported at #48811.

This commit simply remove the condition. In particular, the loading time
of `CairoMakie` is reduced to 40s from 48s (on master).

External `AbstractInterpreter` can use the current condition by
overloading `bail_out_call`.
@aviatesk aviatesk requested a review from Keno March 3, 2023 01:25
@Keno
Copy link
Member

Keno commented Mar 3, 2023

Can we look into what exactly is being inferred extra inside CairoMakie? I don't think it's necessarily sensible to base this on this one case alone. The problem with bailing here is that it can really poison the effects of something that could otherwise be concrete-eval-eligible for a massive inference slowdown.

@Keno
Copy link
Member

Keno commented Mar 3, 2023

There's also the issue where we're no longer properly sorting the matches do do the ones most likely to bail first.

@JeffBezanson JeffBezanson added the latency Latency label Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
latency Latency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants