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

Improve inferrability of isor #166

Merged
merged 2 commits into from
Aug 14, 2021
Merged

Improve inferrability of isor #166

merged 2 commits into from
Aug 14, 2021

Conversation

timholy
Copy link
Contributor

@timholy timholy commented Aug 13, 2021

This avoids invalidations of in or == for Symbols.

There are several other inferrability issues with this package, unblock being a real champion source, which may essentially be unfixable. Consequently users of this package may have to be prepared for a certain amount of precompile failures.

CC @ChrisRackauckas.

src/match/union.jl Outdated Show resolved Hide resolved
@cstjean cstjean merged commit aa558a8 into FluxML:master Aug 14, 2021
@cstjean
Copy link
Collaborator

cstjean commented Aug 14, 2021

Hi Tim, thank you for the PR. I merged, but I must admit that I don't get it, I'm out of the loop...

This avoids invalidations of in or == for Symbols.

I understand how your PR makes this function more inferrable, but I don't understand how it affects invalidations. Isn't isor meant to be used in macro code, where inferrability / performance is really not a big concern usually. Will I get it after watching https://www.youtube.com/watch?v=rVBgrWYKLHY ?

@timholy timholy deleted the teh/isor branch August 14, 2021 12:35
@timholy
Copy link
Contributor Author

timholy commented Aug 14, 2021

Yes, this is directed at latency reduction, but that's probably not the best choice of video. Instead you might read https://julialang.org/blog/2020/08/invalidations/ or watch https://www.youtube.com/watch?v=7VbXbI6OmYo. What this PR does is limit the "damage" for any package that uses MacroTools if one later loads another package that specializes inor ==. The problem here is that the call to in (which ultimately calls ==) is not inferrable, and thus this macro introduces vulnerability to being invalidated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants