bytes, strings: optimize Trim for ASCII cutsets

[dsnet]

I analyzed usages of strings.Trim (and related functions in both strings and bytes) in all source code known by the module proxy:

• 76.6% have a cutset of len=1, and
• 13.4% have a cutset of len=2.

Given that a vast majority of usages only have a cutset of len=1, I argue we should more heavily optimize for that situation. Currently, there is some optimization for cutsets of len=1, but it’s within the internal makeCutsetFunc function. This is sub-optimal as it incurs an allocation in the internal makeCutsetFunc for the closure over that single byte. I believe we should place special handling of cutsets with len=1 directly in Trim, TrimRight, and TrimLeft.

Example benchmark for strings.TrimRight("hello==", "="):

BenchmarkStd   	18299240	        69.16 ns/op	      24 B/op	       1 allocs/op
BenchmarkOpt   	226575349	         5.330 ns/op	       0 B/op	       0 allocs/op

The suggested optimization results in a >10x speedup for this common case.

P.S. There is currently an optimization for cutsets that are all ASCII. This optimization is still justified to keep as 99.3% of all cutsets are pure ASCII.

[ianlancetaylor]

Out of curiosity, is there any particular byte that predominantly appears in the set of single-byte cutsets?

[dsnet]

Out of curiosity, is there any particular byte that predominantly appears in the set of single-byte cutsets?

The top-ten single-byte cutsets are:

• '/' at 30.2%
• '\n' at 15.7%
• ' ' at 10.1%
• '.' at 7.0%
• '\x00' at 5.7%
• '0' at 5.0%
• ',' at 3.5%
• '=' at 3.4%
• '-' at 2.6%
• '|' at 2.5%
• everything else at 14.3%

[dsnet]

https://go-review.googlesource.com/c/go/+/323315 is an example of speedup in application code if bytes.Trim were to be optimized.

[gopherbot]

Change https://golang.org/cl/323318 mentions this issue: bytes, strings: optimize Trim for single byte cutsets

[nightlyone]

Maybe is is useful to do the same optimization as LastIndexAny and IndexAny where the cutover to use a cutset is 8 bytes.

[dsnet]

@nightlyone, can you elaborate more what you have in mind?

Currently, the logic for LastIndexAny is already fairly complex. Any optimizations of LastIndexAny should be driven by how Go code at scale uses it, which is an analysis I haven't performed yet.

I should note that the we could conceivably delete the optimization in LastIndexAny for a cutset with len=1, which was added in golang.org/cl/156998. I don't find that specific optimization particularly compelling since:

1. LastIndexByte already exists, and
2. a previous commit by mine indicates that only 6% of usages have 1 character cutsets.

[josharian]

I recently cobbled together https://go-review.googlesource.com/c/go/+/271669 to work around that allocation. As I noted there, "another alternative is to duplicate a bunch of the trimming code to work directly on asciiSets", which might be a viable option to consider.

@bradfitz also had some thoughts about this. I've forgotten what they were, but perhaps he remembers. :)

[dsnet]

Interesting idea caching the cutset functions. I think inlining the single-byte handling is still preferred since a quick check against a single byte is dramatically faster than always calling a func(rune) bool.

That said, if it provides any insights, the top most common cutsets with len>1 are:

• "\r\n" at 4.6%
• " \t" at 2.3%
• "\n\r" at 0.8%
• "//" at 0.7%
• ", " at 0.5%
• " \t\r\n" at 0.5%
• "\t\n\f\r " at 0.4%
• " \t\n\f\r" at 0.4%
• "./" at 0.3%
• "01234567" at 0.3%

Observations of the above:

• Most of them deal with ASCII whitespace characters.
• Trimming ASCII digits seems common.
• One of them is useless (e.g., "//" is an identical cutset to "/").

[gopherbot]

Change https://golang.org/cl/325251 mentions this issue: strings,bytes: optimize the Trim/TrimLeft/TrimRight fast paths

[gopherbot]

Change https://golang.org/cl/332771 mentions this issue: strings,bytes: optimize the Trim/TrimLeft/TrimRight fast paths