Implement Rabin-Karp algorithm for String#index

[makenowjust]

Benchmark is here.

My result is:

$ crystal run --release index_bench.cr
       index_old 473.18  (  2.11ms) (± 8.92%)  2.72× slower
index_rabin_karp   1.29k (777.94µs) (±11.77%)       fastest

(I recommend you run this benchmark on your environment ;)

[sdogruyol]

Wow, this is amazing! Good job @makenowjust 👍

[yxhuvud]

One thing that would be interesting to see is how it compares for smaller strings. The setup cost for rabin-karp looks to be a lot higher - perhaps it would be a good idea to do approach it similar to sorting, ie if the size is less than a threshold, use index_old. But only benchmarking can show that.

[sdogruyol]

I've updated the benchmark to include a short source with same target https://gist.github.com/sdogruyol/968802c00924528a0b37a73f4f3b4022

             index_old 706.59  (  1.42ms) (± 6.79%) 9894.91× slower
      index_rabin_karp   1.58k (634.75µs) (± 3.35%) 4437.92× slower
           index_short   3.62M ( 276.3ns) (± 5.36%)    1.93× slower
index_short_rabin_karp   6.99M (143.03ns) (± 7.61%)         fastest

It shows that Rabin Karp is still 2x faster.

[makenowjust]

@sdogruyol Thanks!

[bcardiff]

Thanks @makenowjust ⚡️ !

[makenowjust]

@bcardiff Thanks too for fast response ⚡️

[MaxLap]

This seems to be a relatively good improvement to the algorithm. I didn't verify that the specs pass, so maybe something is wrong it. I'm suprised by how much of an improvement this shows, considering that ascii text is the case that should be gaining the less benefit.

I guess the if (byte & 0x80) == 0 || (byte & 0x40) != 0 check is pricier than it looks.

I'm basically just reducing the number of bigloop iterations needed by doing multibytes jumps, the same way it is done when finding the offset.

https://gist.github.com/MaxLap/0ab65d0e1466a7ba1107f66d0983be52

              index_old 265.23  (  3.77ms) (± 2.75%) 20941.18× slower
       index_rabin_karp 852.97  (  1.17ms) (± 5.87%)  6511.56× slower
      index_rabin_karp2   1.15k (873.18µs) (± 7.78%)  4849.77× slower
            index_short   1.96M (509.09ns) (±13.31%)     2.83× slower
 index_short_rabin_karp   3.96M (252.25ns) (±16.32%)     1.40× slower
index_short_rabin_karp2   5.55M (180.05ns) (±17.73%)          fastest

[makenowjust]

@MaxLap Your CPU is...? I think your CPU's no zero comparison is slower than mine. (My CPU is 1.6 GHz Intel Core i5 on MacBook Air, below is my result)

              index_old 604.03  (  1.66ms) (± 2.83%) 10648.60× slower
       index_rabin_karp   1.42k (706.58µs) (± 4.68%)  4544.78× slower
      index_rabin_karp2   1.46k (683.66µs) (± 4.03%)  4397.35× slower
        short_index_old   3.33M (299.97ns) (± 6.84%)     1.93× slower
 short_index_rabin_karp   6.09M (164.22ns) (± 9.57%)     1.06× slower
short_index_rabin_karp2   6.43M (155.47ns) (± 9.18%)          fastest

Of course, it is efficient for multi-byte text. And I rewrite your code by using macros to reduce code duplication (here).

[MaxLap]

I'm on Windows. So the benchmark was through a VM of Ubuntu 12.04 32bit with 2 cores. I7-3517U at 2.4GHz

I tried on another VM, Ubuntu 16.04 64bit. The improvement is less (but still higher than yours)

A bit sad that it's less impressive... Oh well, it's still an improvement!

[akzhan]

btw, @makenowjust, are you plan to merge @MaxLap hint?

[makenowjust]

@akzhan already implemented in #3876.