refactor: replace Split in loops with more efficient SplitSeq#2969
refactor: replace Split in loops with more efficient SplitSeq#2969box4wangjing wants to merge 2 commits into
Conversation
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? |
Thank you for your reply. I’ve checked everything, and I only found this one case. @aldas |
|
You could try If think there are couple of places more where this change could be applied. |
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) |
It’s been updated. I've reviewed all occurrences of 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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. 🚀 New features to boost your workflow:
|
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