gannan08
commented
3 years ago Thank you for the changes. The original code was written in a way so that it used the same variable names as the old permuter implementation. However, the old code has been removed.
I'm happy to see that you did not view any difference in performance either.
If one can make the optimization with their eyes, then the compiler can surely do the same and more.
TLDR; All this does is adjust some variable names and remove an intermediate variable.
Research
So, in studying this. I found that in the full rdf test suite, we do fall into this loop sometimes, but with the dataset there we never match the conditional on L87.
In particular, this loop is never activated at all in the hot path for a merge event operation, however, it is activated in the hot path for
test044_canonize.I performed micro and macro benchmarks after cutting out the loop entirely, which had no benefit, which may mean it is getting highly optimized already.
For future research... it would be nice to discover if we can eliminate this additional iteration over all the elements since as a practical matter, it is just waste. I wonder if our dataset is such that it's not required.
Also, this is a poster child for SIMD optimization which we may well be getting for free if rust vectorizes this code.