refactor: replace Split in loops with more efficient SplitSeq - #2969
Conversation
Sorry, something went wrong.
Signed-off-by: box4wangjing <box4wangjing@outlook.com>
|
Thanks, there are probably other places where this same logic could be applied - did you look for these also? |
Sorry, something went wrong.
Thank you for your reply. I’ve checked everything, and I only found this one case. @aldas |
Sorry, something went wrong.
|
You could try grep -R --include="*.go" -A 5 "strings.Split" . If think there are couple of places more where this change could be applied. |
Sorry, something went wrong.
Signed-off-by: box4wangjing <box4wangjing@outlook.com>
|
All of the matched case: ➜ echo git:(master) grep -R --include="*.go" -A 5 "strings.Split" .
./middleware/extractor.go: sources := strings.Split(lookups, ",")
./middleware/extractor.go- var extractors = make([]ValuesExtractor, 0)
./middleware/extractor.go- for _, source := range sources {
./middleware/extractor.go: parts := strings.Split(source, ":")
./middleware/extractor.go- if len(parts) < 2 {
./middleware/extractor.go- return nil, fmt.Errorf("extractor source for lookup could not be split into needed parts: %v", source)
./middleware/extractor.go- }
./middleware/extractor.go-
./middleware/extractor.go- switch parts[0] {
--
./router_test.go: assert.ElementsMatch(t, []string{"COPY", "GET", "LOCK", "OPTIONS"}, strings.Split(c.Response().Header().Get(HeaderAllow), ", "))
./router_test.go-}
./router_test.go-
./router_test.go-func TestMethodNotAllowedAndNotFound(t *testing.T) {
./router_test.go- e := New()
./router_test.go-
--
./router_test.go: tokens := strings.SplitSeq(route.Path[1:], "/")
./router_test.go- for token := range tokens {
./router_test.go- if token[0] == ':' {
./router_test.go- assert.Equal(t, c.pathValues.GetOr(token[1:], "---none---"), token)
./router_test.go- }
./router_test.go- }
--
./bind_test.go: *a = StringArray(strings.Split(src, ","))
./bind_test.go- return nil
./bind_test.go-}
./bind_test.go-
./bind_test.go-func (s *Struct) UnmarshalParam(src string) error {
./bind_test.go- *s = Struct{
--
./bind_test.go: var values = strings.Split(value, ",")
./bind_test.go- var numbers = make([]int, 0, len(values))
./bind_test.go-
./bind_test.go- for _, v := range values {
./bind_test.go- n, err := strconv.ParseInt(v, 10, 64)
./bind_test.go- if err != nil {
--
./bind_test.go: var values = strings.Split(param, ",")
./bind_test.go- for _, v := range values {
./bind_test.go- n, err := strconv.ParseInt(v, 10, 64)
./bind_test.go- if err != nil {
./bind_test.go- return fmt.Errorf("'%s' is not an integer", v)
./bind_test.go- }
--
./ip.go: ips := append(strings.Split(strings.Join(xffs, ","), ","), directIP)
./ip.go- for i := len(ips) - 1; i >= 0; i-- {
./ip.go- ips[i] = strings.TrimSpace(ips[i])
./ip.go- ips[i] = strings.TrimPrefix(ips[i], "[")
./ip.go- ips[i] = strings.TrimSuffix(ips[i], "]")
./ip.go- ip := net.ParseIP(ips[i])
--
./binder.go: tmpValues = append(tmpValues, strings.Split(v, delimiter)...)
./binder.go- }
./binder.go-
./binder.go- switch d := dest.(type) {
./binder.go- case *[]string:
./binder.go- *d = tmpValues
➜ echo git:(master) |
Sorry, something went wrong.
It’s been updated. I've reviewed all occurrences of strings.Split in the codebase. Here's why the other cases are not suitable for conversion to SplitSeq, for reference: 1. ip.go:250 - Not suitableips := append(strings.Split(strings.Join(xffs, ","), ","), directIP)
for i := len(ips) - 1; i >= 0; i-- {
ips[i] = strings.TrimSpace(ips[i])
// ...
return strings.TrimSpace(ips[0])
}Reasons:
2. binder.go:424 - Not suitablefor _, v := range values {
tmpValues = append(tmpValues, strings.Split(v, delimiter)...)
}Reason:
3. bind_test.go:120, 1173, 1281 - Not suitable*a = StringArray(strings.Split(src, ","))
var values = strings.Split(value, ",")Reasons:
4. router_test.go:826 - Not suitableassert.ElementsMatch(t, []string{"COPY", "GET", "LOCK", "OPTIONS"},
strings.Split(c.Response().Header().Get(HeaderAllow), ", "))Reason:
5. middleware/extractor.go:91 (inner split) - Not suitablefor source := range sources {
parts := strings.Split(source, ":")
if len(parts) < 2 {
return nil, fmt.Errorf("...")
}
switch parts[0] {
case "query":
extractors = append(extractors, valuesFromQuery(parts[1], limit))
// ...
if len(parts) > 2 {
prefix = parts[2]
}
}Reasons:
SummaryThe only suitable case was the outer loop in middleware/extractor.go:88 where we iterate over sources without needing indexed access or length checks. All other cases require slice-specific operations (indexing, length, append with spread, or direct assignment) that are incompatible with the iterator pattern of SplitSeq. |
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #2969 +/- ##
=======================================
Coverage 93.17% 93.17%
=======================================
Files 43 43
Lines 4501 4501
=======================================
Hits 4194 4194
Misses 189 189
Partials 118 118 ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
strings.SplitSeq (introduced in Go 1.23) returns a lazy sequence (strings.Seq), allowing gopher to iterate over tokens one by one without creating an intermediate slice.
It significantly reduces memory allocations and can improve performance for long strings.
More info: golang/go#61901