Repository navigation
strings,bytes: ToLower(), ToUpper() by using bitwise operator #64361
Copy link
Copy link
Closed as not planned
Milestone
Description
When the string consists of only ASCII characters, we can convert lower case to upper case using bitwise operators. Bitwise operators are more efficient than using addition or subtraction.
Lines 616 to 625 in 3e67f46
| for i := 0; i < len(s); i++ { | |
| c := s[i] | |
| if 'a' <= c && c <= 'z' { | |
| c -= 'a' - 'A' | |
| if pos < i { | |
| b.WriteString(s[pos:i]) | |
| } | |
| b.WriteByte(c) | |
| pos = i + 1 | |
| } |
For example, in Rust, they used bitwise operator to implement the converter.
I propose that we replace the addition and subtraction operators with bitwise operators in the following four places.
Lines 616 to 625 in 3e67f46
| for i := 0; i < len(s); i++ { | |
| c := s[i] | |
| if 'a' <= c && c <= 'z' { | |
| c -= 'a' - 'A' | |
| if pos < i { | |
| b.WriteString(s[pos:i]) | |
| } | |
| b.WriteByte(c) | |
| pos = i + 1 | |
| } |
Lines 656 to 666 in 3e67f46
| for i := 0; i < len(s); i++ { | |
| c := s[i] | |
| if 'A' <= c && c <= 'Z' { | |
| c += 'a' - 'A' | |
| if pos < i { | |
| b.WriteString(s[pos:i]) | |
| } | |
| b.WriteByte(c) | |
| pos = i + 1 | |
| } | |
| } |
Lines 638 to 652 in 3e67f46
| if isASCII { // optimize for ASCII-only byte slices. | |
| if !hasLower { | |
| // Just return a copy. | |
| return append([]byte(""), s...) | |
| } | |
| b := bytealg.MakeNoZero(len(s)) | |
| for i := 0; i < len(s); i++ { | |
| c := s[i] | |
| if 'a' <= c && c <= 'z' { | |
| c -= 'a' - 'A' | |
| } | |
| b[i] = c | |
| } | |
| return b | |
| } |
Lines 669 to 682 in 3e67f46
| if isASCII { // optimize for ASCII-only byte slices. | |
| if !hasUpper { | |
| return append([]byte(""), s...) | |
| } | |
| b := bytealg.MakeNoZero(len(s)) | |
| for i := 0; i < len(s); i++ { | |
| c := s[i] | |
| if 'A' <= c && c <= 'Z' { | |
| c += 'a' - 'A' | |
| } | |
| b[i] = c | |
| } | |
| return b | |
| } |
Reactions are currently unavailable
Activity
Jorropo commentedon Nov 23, 2023
@ksw2000 I don't see in what way this is change the api or behavior, if this is purely an internal change you don't need a proposal and can just submit a patch. 🙂
ksw2000 commentedon Nov 23, 2023
Sorry, this is my first time creating an issue for Golang, and I'm not sure what type of issue to use. Can you please tell me what type of issue I should use for an issue like this or could I just pull request directly?
I am so sorry for my mistake. I will make sure to learn how to use it correctly next time.
panjf2000 commentedon Nov 23, 2023
Taking this out of the proposal milestone as it doesn't involve API changes.
[-]proposal: strings/bytes: ToLower(), ToUpper() by using bitwise operator[/-][+]strings/bytes: ToLower(), ToUpper() by using bitwise operator[/+]panjf2000 commentedon Nov 23, 2023
@ksw2000
You only need a proposal when you're trying to make API changes or language changes, you can just send a CL directly for this without making a proposal since what you're intended to do here doesn't involve API or language changes.
See golang proposal for details.
ksw2000 commentedon Nov 23, 2023
@panjf2000
Thank you very much for your detailed explanation. I will send a CL directly. Should I close this issue?
Jorropo commentedon Nov 23, 2023
I think you should, when you submit your CL don't forget to post benchmark results using benchstat. 🙂
panjf2000 commentedon Nov 23, 2023
Leave this issue and add a line
Fixes #64361in the commit message of your CL.[-]strings/bytes: ToLower(), ToUpper() by using bitwise operator[/-][+]strings,bytes: ToLower(), ToUpper() by using bitwise operator[/+]randall77 commentedon Nov 23, 2023
I'm not sure I buy the premise that bitwise operators are any faster than adding or subtracting.
They are all 1 cycle operations on modern processors.
ksw2000 commentedon Nov 23, 2023
After testing, I found that the improvement is negligible. I will close this issue. Thank you for the discussion.
Jorropo commentedon Nov 23, 2023
This makes the code branchless which helps a auto vectorizers, so this does not apply to gc.
Without counting for memory bandwidth using back of the napkin math, should be 16x faster (since we can load and store 16 bytes per cycle) instead of 1.
randall77 commentedon Nov 24, 2023
@joroppo Branchless would be great if you can do it. How would you do that here, exactly?
jiangbajin commentedon Nov 24, 2023
Jorropo commentedon Nov 24, 2023
I'm fairly confident branch-less would be slower here since it would need to generate masks. But SIMD with masks would be faster than SISD.
I get what you mean now, I thought you could fuse the anding of the masks you have to do in SIMD with the anding of the bits, but however then you need to move the mask to the 6th bit first so it's not actually any faster.
Jorropo commentedon Nov 24, 2023
After trying it to be sure, I'm not able to measure anything.
I had trouble making the compiler generate the most efficient comparison and bit handling so trying in assembly (SISD) branches, cmov and SETcc with rsh they are all bottlenecked on writing to memory anyway and run at the same speed.
ksw2000 commentedon Nov 24, 2023
In
src/strings/strings.goToUpper
by add/sub operation (original method)
by binary operation
ToLower
by add/sub operation (original method)
by binary operation