[rust] Skip unnecessary calls to find_among #202
Conversation
This change ports an optimization from the C stemmer generator to the rust generator. In order to determine whether find_among and find_among_b are worth calling, the generated C checks that the current string is long enough to potentially match the shortest string literal in the among, then does some bitwise magic to check that the characters in the current string at length of the end of the shortest string in the among are the same as the character at that position in at least one of the among strings. If the current string is too short or that character doesn't match any of the amongs at that position, then we can fail-fast instead of calling find_among. This change allows the generated rust stemmers to similarly skip find_among calls where we can quickly determine that find_among would not succeed. In some stemming benchmarks I've been running, this improves the performance of the rust english stemmer by between 40 and 50%.
|
Sounds good, but CI failed as this seems to result in different output for one of the arabic test vocabulary words so there's something not quite right in the implementation. To reproduce locally, try |
|
Ah, I see, I had missed |
|
OK, I think it's passing |
|
Ooh, I think understand it now, it was simplifying the |
|
I guess those The 40-50% gain did seem a bit surprising, but the C optimisation was a long time ago. |
|
Yeah, the I wasn't as surprised with a nominal speedup of that magnitude because we were seeing the rust version as 60-70% slower than the C version in benchmarks, and it seemed possible that this optimization might be the main cause. It's still a big piece but we've found other sources for the remaining performance delta (mostly artifacts of the way we were wrapping the rust code for benchmarking, resulting in extra allocations). Is there anything else you'd like to see changed here before this can be merged? I'm not totally clear on the process here. |
|
Sorry, was just on vacation and not really able to actually look at the changes. Looks good, thanks. |
|
Thanks Olly! |
This change ports an optimization from the C stemmer generator to the rust generator.
In order to determine whether
find_amongandfind_among_bare worth calling, the generated C checks that the current string is long enough to potentially match the shortest string literal in the among, then does some bitwise magic to check that the byte in the current string at the length of the shortest string in the among may match the byte at that position in at least one of the among strings. If the current string is too short or that byte doesn't match any of the amongs at that position, then we can fail-fast instead of callingfind_amongorfind_among_b.This change allows the generated rust stemmers to similarly skip
find_amongandfind_among_bcalls where we can quickly determine that the call could not succeed. In some stemming benchmarks I've been running, this improves the performance of the rust english stemmer by between 40% and 50%.