-
-
Couldn't load subscription status.
- Fork 5.7k
Reduce number of getindex(::Type, ...) methods
#44127
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
Conversation
|
This is good; I think the only downside is that we lose the vararg tuple elision after 32: ( |
|
I guess we can use for loop |
71c4298 to
b8bbc31
Compare
|
Updated with @N5N3's idea to keep the |
Previously, there were special cases for `T[]`, `T[a]`, `T[a,b]` and `T[a,b,c]`. Together with the general case for more elements, that meant five methods to consider in cases like `T[x...]` where the length of `x` was not known at compile time. That was beyond the inference limit and such a call would be inferred as `Any`. So this change gets rid of all the special cases. The loop-based general case worked well if all arguments were of the same type, but otherwise suffered from type-instability inside the loop. Without the special cases for low element count this would be hit more often, so for the non-homogenous case, the loop is replaced with a call to `afoldl` that basically unrolls the loop for up to 32 elements.
b8bbc31 to
f9db5d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. Seems a clever solution to this
|
CI is all green after another rebase, feedback is generally positive. I'm going to merge tomorrow unless anyone objects (or beats me to it). |
I spoke too soon, buildbot/tester_linux64 timed out again. But I've seen that in other PRs, too, so let me declare it unrelated. |
|
Why is this getting backported? |
|
There is some leeway for PRs that were more or less finished when feature freeze hit and just needed review/CI to go through. |
Previously, there were special cases for `T[]`, `T[a]`, `T[a,b]` and `T[a,b,c]`. Together with the general case for more elements, that meant five methods to consider in cases like `T[x...]` where the length of `x` was not known at compile time. That was beyond the inference limit and such a call would be inferred as `Any`. So this change gets rid of all the special cases. The loop-based general case worked well if all arguments were of the same type, but otherwise suffered from type-instability inside the loop. Without the special cases for low element count this would be hit more often, so for the non-homogeneous case, the loop is replaced with a call to `afoldl` that basically unrolls the loop for up to 32 elements. (cherry picked from commit b8e5d7e)
Previously, there were special cases for `T[]`, `T[a]`, `T[a,b]` and `T[a,b,c]`. Together with the general case for more elements, that meant five methods to consider in cases like `T[x...]` where the length of `x` was not known at compile time. That was beyond the inference limit and such a call would be inferred as `Any`. So this change gets rid of all the special cases. The loop-based general case worked well if all arguments were of the same type, but otherwise suffered from type-instability inside the loop. Without the special cases for low element count this would be hit more often, so for the non-homogeneous case, the loop is replaced with a call to `afoldl` that basically unrolls the loop for up to 32 elements.
Previously, there were special cases for `T[]`, `T[a]`, `T[a,b]` and `T[a,b,c]`. Together with the general case for more elements, that meant five methods to consider in cases like `T[x...]` where the length of `x` was not known at compile time. That was beyond the inference limit and such a call would be inferred as `Any`. So this change gets rid of all the special cases. The loop-based general case worked well if all arguments were of the same type, but otherwise suffered from type-instability inside the loop. Without the special cases for low element count this would be hit more often, so for the non-homogeneous case, the loop is replaced with a call to `afoldl` that basically unrolls the loop for up to 32 elements.
Previously, there were special cases for
T[],T[a],T[a,b]andT[a,b,c]. Together with the general case for more elements, that meant five methods to consider in cases likeT[x...]where the length ofxwas not known at compile time. That was beyond the inference limit and such a call would be inferred asAny. So this change gets rid of all the special cases.The loop-based general case worked well if all arguments were of the same type, but otherwise suffered from type-instability inside the loop. Without the special cases for low element count this would be hit more often, so the loop is replaced with a call to
afoldlthat basically unrolls the loop for up to 32 elements.Motivated by JuliaMath/FFTW.jl#231.