Better indexing for coupled iteration of array with AbstractArray{Bool} #16865
Conversation
|
This addresses my concerns voiced in #16128 (comment), so 👍 We should wait for the tests to run, but I'm in favor of merging this if it fixes the performance. Nice work, @pabloferz! |
|
Turns out that the first thing I tried helps, but is not enough. So, I wrote specialized versions that help even more. I understand this is not ideal (and I can remove the specialized versions) but wanted to let you know anyway. |
|
(FWIW, it would have been nice to use a more detailed commit message, instead of keeping the explanations on GitHub.) |
|
@nalimilan Sorry. I'll keep this in mind. Tests pass, but now I am a bit hesitant. Should I only propose the first commit and see if using -O3 takes care of the rest? Or the latest changes are OK? |
|
Seems fine to me. Let's check the performance: @nanosoldier |
|
Yeah the code near-duplication is a bit unfortunate, but hopefully we can improve the compiler for the general case and make it unnecessary again at some point in the future. |
|
Your benchmark job has completed, but something went wrong when trying to upload the result data. cc @jrevels |
|
Hmm. I think @nanosoldier ran into a bug trying to get the permalink to the report, but all the data did in fact upload correctly. Permalink to the report is here. |
|
Looks like I ran the wrong ones. @nanosoldier |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @jrevels |
|
The tests are here but I don't see anything like it over BaseBenchmarks.jl |
|
Arg, yes, I found some of those Let's just merge. Thanks for this! |
|
No problem! |
This should help with some of the problems with
add1andadd1_logicalin #16128 until something like #15356 gets sorted out.CC @timholy @JeffBezanson