Optimize performance of substr_index and add tests

[kevinmingtarja]

Which issue does this PR close?

Closes #9890

Rationale for this change

We were using dynamic dispatch when deciding whether to do string.split() or string.rsplit() (depending on the count argument) because the exact type of iterator is not known at compile time, plus it allocates memory on the heap for the boxed iterator, which introduces considerable overhead.

Instead, we can avoid doing this and handle the two cases with an if else statement, which removes that overhead and potentially enable further optimizations by the compiler.

Another major overhead is pointed to in #9973 (comment). Instead, we can avoid String creation at every iteration of map by using StringBuilder to build the output.

Benchmarks

substr_index_array_array_1000
                        time:   [57.082 µs 57.407 µs 57.770 µs]
                        change: [-53.118% -52.034% -51.077%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe

No more time wasted on malloc and free now if we see the flamegraph:

What changes are included in this PR?

Are these changes tested?

Yes. I added new unit tests as well for substr_index since there were none before.

Are there any user-facing changes?

[kevinmingtarja]

CI failure seems unrelated:

error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `sccache /usr/local/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc - --crate-name ___ --print=file-names -C debuginfo=line-tables-only -C incremental=false --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=split-debuginfo --print=crate-name --print=cfg` (exit status: 2)
  --- stderr
  sccache: error: Timed out waiting for server startup. Maybe the remote service is unreachable?
  Run with SCCACHE_LOG=debug SCCACHE_NO_DAEMON=1 to get more information
Error: Process completed with exit code 101.

[alamb]

CI failure seems unrelated:

Agreed -- I restarted the job

[alamb]

Looks nice a nice improvement to me -- thank you @kevinmingtarja 🙏

I left a suggestion for how to make it even better, but I also think we can do that as a follow on PR

[alamb]

Thank you @kevinmingtarja -- this is looking really close. I think the null/no match cases aren't quite right but it should be easily fixable

Null checks don't see right

[alamb]

Thanks @kevinmingtarja -- this looks really nice 🙏

[alamb]

FYI @Omega359

[comphead]

lgtm thanks @kevinmingtarja I really like the improvement

[Omega359]

Nice work! If there is another function that you think could use a look at for performance I'll be happy to write the benchmark for testing.

[kevinmingtarja]

Thanks for the help and review @alamb, @comphead, and thanks @Omega359 for writing the benchmark!

[alamb]

🚀