Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bytes: generic Index too slow #22578

Closed
randall77 opened this issue Nov 4, 2017 · 4 comments
Closed

bytes: generic Index too slow #22578

randall77 opened this issue Nov 4, 2017 · 4 comments

Comments

@randall77
Copy link
Contributor

For non-amd64/s390x, we have a generic version of Index, written in Go.
It can be slow because it uses a combination of IndexByte (for the first byte of the needle string) and Equal. When the first byte appears in most of the haystack string, it can be really slow.
For some reason this code doesn't use Rabin-Karp, the standard solution to this problem. The amd64 and s390x cases do.
I think we just need to update the generic Index to use Rabin-Karp.

Noticed while looking at CL 76050.

@randall77 randall77 added this to the Go1.10 milestone Nov 4, 2017
@randall77 randall77 self-assigned this Nov 4, 2017
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/76070 mentions this issue: bytes,strings: in generic Index, use mix of IndexByte and Rabin-Karp

@randall77
Copy link
Contributor Author

And strings.Index has the opposite problem, it uses Rabin-Karp but never IndexByte, even when it would be faster.

@dongweigogo
Copy link

And strings.Index has the opposite problem, it uses Rabin-Karp but never IndexByte, even when it would be faster.

I've seen many gophers complaining about that.

@vielmetti
Copy link

Also noted here:

https://blog.cloudflare.com/arm-takes-wing/

Go regexp performance is not very good on Falkor. In the medium and hard tests it takes second place, thanks to the higher core count, but Skylake is significantly faster still.

Doing some profiling shows that a lot of the time is spent in the function bytes.IndexByte. This function has an assembly implementation for amd64 (runtime.indexbytebody), but generic implementation for Go. The easy regexp tests spend most of time in this function, which explains the even wider gap.

@golang golang locked and limited conversation to collaborators Nov 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants