proto: optimize Properties.Parse() using IndexByte - #779
Conversation
d480764 to
e30260b
Compare
| value := f[9:] | ||
| if eq := strings.IndexByte(value, '='); eq != -1 { | ||
| p.OrigName = value[:eq] | ||
| } else { | ||
| p.OrigName = value | ||
| } |
There was a problem hiding this comment.
should we just do https://pkg.go.dev/strings#CutPrefix here and below?
it would be nicer if we could do this as part of the switch, but I think that's a no?
There was a problem hiding this comment.
yah sure, I can switch for that. I don't know how it'd be part of the switch though.
| if comma == -1 { | ||
| tagField = s[pos:] | ||
| pos = len(s) |
There was a problem hiding this comment.
why is this here, shouldn't this be an error?
(didn't previous code reqire at least 2 commas?(
There was a problem hiding this comment.
hm I somehow missed this. yeah this code isn't super easy to read hm. I can try out the splitN optimization.
as for this code, we're looking at the second comma here so we can get the tag field. the original code checked if there were at least 2 items after splitting by commas. the check on line 206 accomplishes the same thing (2 items == 1 comma).
Replace strings.Split with strings.IndexByte for better performance: - Eliminates slice allocation for field parsing - Single-pass parsing without intermediate slices - Fixes embedded/customtype/casttype parsing edge cases - Maintains full backward compatibility Benchmark results show improved performance while maintaining identical functionality and passing all tests. Made-with: Cursor
e30260b to
652d6f1
Compare
| case strings.HasPrefix(f, "embedded="): | ||
| p.OrigName = strings.Split(f, "=")[1] | ||
| value, _ := strings.CutPrefix(f, "embedded=") | ||
| if eq := strings.IndexByte(value, '='); eq != -1 { |
There was a problem hiding this comment.
why do we need all this indexing? shouldn't but prefix be enough?
2 participants
Replace strings.Split with strings.IndexByte for better performance: