Optimizations on to_indices #227
Conversation
Codecov Report
@@ Coverage Diff @@
## master JuliaArrays/ArrayInterface.jl#227 +/- ##
==========================================
+ Coverage 87.81% 87.93% +0.11%
==========================================
Files 11 11
Lines 1789 1591 -198
==========================================
- Hits 1571 1399 -172
+ Misses 218 192 -26
Continue to review full report at Codecov.
|
Although including bounds checking in the `to_indices` should avoid revisiting every argument site, the compiler has trouble optimizing it when done all at once. Now we do boundschecking prior to `to_indices` like in Base.
|
@ChrisRackauckas and/or @chriselrod, I know the inner If we're worried about breaking changes with this, then we should probably address some additional breaking changes before releasing 4.0 (like JuliaArrays/StaticArrayInterface.jl#10). I could make a project task list with some clear goals that we could agree on for this. |
After a lot of testing I found out that we can just rely on `ndims_index` for aligning axes in a generated function and then flatten out tuples returned from `to_index`. This greatly simplifies the explanation of internals and it also makes accomodating new indexing types simpler.
This is a performance optimized version of
to_indices. I've provided the justification for most changes fromBase.to_indicesin the "# Extended help" section of the doc string. It also includes some minimal benchmarking to drive the point home. I might change a few minor bits of code once I get some sleep, but it can work for any of the indexing argument types in currently registered packages and can do so faster thanBase.to_indicesin all cases (as far as I'm aware). I'm sure there are parts I haven't documented or explained well, but I think it's a pretty decent start for now.The biggest hang ups I anticipate are potentially breaking changes (still need to check if we can take away the 3 arg version of
to_indices) and the two large@generatedmethods working behind the scenes (_splat_indicesand__to_indices). I think we can solve any problems that might come up with regards to accommodating new indexing types using traits.