-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
recursive implementation for in(::Any, ::Tuple)
#54411
Conversation
The |
We may want a cutoff length for this, as the present loop-based implementation is faster for large tuples. julia> t = ntuple(x->'A', 1000);
julia> @btime 'C' in $t;
354.118 ns (0 allocations: 0 bytes) This PR julia> @btime 'C' in $t;
2.889 μs (0 allocations: 0 bytes) |
This comment was marked as outdated.
This comment was marked as outdated.
Can you explain the advantages of using |
Also xref: #54026 |
This comment was marked as resolved.
This comment was marked as resolved.
Avoids relying on `tail(::Tuple)`, thus it can be performant for moderate length tuples. Before this change the recursive implementation could only be used for tuples of length less than about thirty, while now the recursion is used for lengths up to 600. This means that `in` for larger tuples than before will both be faster and amenable to constant folding. This introduces the `TupleView` type, which will hopefully be used in a few other places in the future, too. It's meant to help with recursing over every tuple element in a compiler-friendly way. In particular, `TupleView` could be useful for `_isdisjoint`, `isdisjoint(::Tuple, ::Tuple)` and other places where `tail(::Tuple)` is currently used.
Performance comparisons show 3x-4x improvement: Before:
After:
|
The benchmark isn't fair because this PR increases the threshold for tuple lengths that permit unrolling. Essentially, the performance difference we're seeing is between the loop implementation (from the master) and the recursive implementation (in this PR) for the very large (100-length) tuples. If comparisons are needed, they should be made between recursive implementations. Something like this: julia> @benchmark i ∈ t setup=(n = rand((2:10...,25,100)); t = ntuple(n) do _; rand(1:10); end; i = rand(1:10))
BenchmarkTools.Trial: 10000 samples with 998 evaluations.
Range (min … max): 17.117 ns … 83.709 ns ┊ GC (min … max): 0.00% … 0.00%
Time (median): 22.253 ns ┊ GC (median): 0.00%
Time (mean ± σ): 22.440 ns ± 2.913 ns ┊ GC (mean ± σ): 0.00% ± 0.00%
▁▁ ▃▂▄▁▁▂█▇▄
▁▁▂▂▂▄▄▄▃▅██▅▅▇█████████▇▇▇▇▄▃▃▅▅▃▃▃▄▃▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▃
17.1 ns Histogram: frequency by time 31.6 ns <
Memory estimate: 0 bytes, allocs estimate: 0. |
Increasing the threshold is the point of this PR (now that the competing PR is already merged).
That obviously makes this PR's perf wins even greater, by a lot. The recursive implementation on master allocates when given tuples larger than intended (because of relying on
|
let f = is_missing | any_missing, rest = tail(v) | ||
_in_tupleview(x, rest, f) |
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.
Probably I should just get rid of the let
and inline f
and rest
.
Closing in favor of an upcoming, more comprehensive PR. |
Avoids relying on
tail(::Tuple)
, thus it can be performant for moderate length tuples. Before this change the recursive implementation could only be used for tuples of length less than about thirty, while now the recursion is used for lengths up to 600.This means that
in
for larger tuples than before will both be faster and amenable to constant folding.This introduces the
TupleView
type, which will hopefully be used in a few other places in the future, too. It's meant to help with recursing over every tuple element in a compiler-friendly way. In particular,TupleView
could be useful for_isdisjoint
,isdisjoint(::Tuple, ::Tuple)
and other places wheretail(::Tuple)
is currently used.