Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce allocations by avoiding strings.Split #4

Merged
merged 3 commits into from
Sep 3, 2020
Merged

Reduce allocations by avoiding strings.Split #4

merged 3 commits into from
Sep 3, 2020

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Sep 3, 2020 •
edited
Loading

This pull request improve performance of Unmarshal by +49%, by reducing allocations from 62 to 35.

Before

 % go test -bench=. -benchmem .
goos: darwin
goarch: amd64
pkg: github.com/Songmu/go-ltsv
BenchmarkUnmarshalStruct-16       242023              4675 ns/op            2328 B/op         62 allocs/op
BenchmarkMarshalStruct-16        1727038               662 ns/op             240 B/op          7 allocs/op
BenchmarkMarshalMap-16           5341411               222 ns/op             112 B/op          2 allocs/op
PASS
ok      github.com/Songmu/go-ltsv       4.577s

BenchmarkUnmarshalStruct

      flat  flat%   sum%        cum   cum%
  301.57MB 50.30% 50.30%   404.57MB 67.48%  github.com/Songmu/go-ltsv.data2map
  156.50MB 26.10% 76.40%   156.50MB 26.10%  strings.genSplit
      65MB 10.84% 87.24%   599.08MB 99.92%  github.com/Songmu/go-ltsv.BenchmarkUnmarshalStruct
      42MB  7.01% 94.25%   534.07MB 89.07%  github.com/Songmu/go-ltsv.Unmarshal
      29MB  4.84% 99.08%       29MB  4.84%  reflect.(*structType).Field
    3.50MB  0.58% 99.67%     3.50MB  0.58%  strings.(*Builder).grow (inline)
         0     0% 99.67%       29MB  4.84%  reflect.(*rtype).Field
         0     0% 99.67%     3.50MB  0.58%  strings.(*Builder).Grow (inline)
         0     0% 99.67%       87MB 14.51%  strings.Split (inline)
         0     0% 99.67%    69.50MB 11.59%  strings.SplitN (inline)

BenchmarkMarshalStruct

Showing top 10 nodes out of 17
      flat  flat%   sum%        cum   cum%
  213.51MB 32.74% 32.74%   213.51MB 32.74%  strconv.FormatFloat
  173.51MB 26.61% 59.36%   173.51MB 26.61%  bytes.(*Buffer).grow
  135.51MB 20.78% 80.14%   522.52MB 80.14%  github.com/Songmu/go-ltsv.makeStructWriter.func1
  129.51MB 19.86%   100%   129.51MB 19.86%  bytes.NewBuffer (inline)
         0     0%   100%   173.51MB 26.61%  bytes.(*Buffer).WriteString
         0     0%   100%   652.03MB   100%  github.com/Songmu/go-ltsv.BenchmarkMarshalStruct
         0     0%   100%   652.03MB   100%  github.com/Songmu/go-ltsv.Marshal
         0     0%   100%   522.52MB 80.14%  github.com/Songmu/go-ltsv.MarshalTo
         0     0%   100%      133MB 20.40%  github.com/Songmu/go-ltsv.elemWriter.func1
         0     0%   100%   213.51MB 32.74%  github.com/Songmu/go-ltsv.makeFloatWriter.func1

After

 % go test -bench=. -benchmem .
goos: darwin
goarch: amd64
pkg: github.com/Songmu/go-ltsv
BenchmarkUnmarshalStruct-16       393963              3120 ns/op            1656 B/op         35 allocs/op
BenchmarkMarshalStruct-16        1810189               649 ns/op             240 B/op          7 allocs/op
BenchmarkMarshalMap-16           5446528               218 ns/op             112 B/op          2 allocs/op
PASS
ok      github.com/Songmu/go-ltsv       4.774s

BenchmarkUnmarshalStruct

  392.09MB 68.78% 68.78%   392.09MB 68.78%  github.com/Songmu/go-ltsv.data2map
   77.50MB 13.59% 82.37%   570.10MB   100%  github.com/Songmu/go-ltsv.BenchmarkUnmarshalStruct
      48MB  8.42% 90.79%   492.59MB 86.41%  github.com/Songmu/go-ltsv.Unmarshal
   42.50MB  7.46% 98.25%    42.50MB  7.46%  reflect.(*structType).Field
       8MB  1.40% 99.65%        8MB  1.40%  strings.(*Builder).grow (inline)
         0     0% 99.65%    42.50MB  7.46%  reflect.(*rtype).Field
         0     0% 99.65%        8MB  1.40%  strings.(*Builder).Grow (inline)
         0     0% 99.65%        8MB  1.40%  strings.ToLower
         0     0% 99.65%   570.10MB   100%  testing.(*B).launch
         0     0% 99.65%   570.10MB   100%  testing.(*B).runN

BenchmarkMarshalStruct

      flat  flat%   sum%        cum   cum%
  209.01MB 32.13% 32.13%   209.01MB 32.13%  strconv.FormatFloat
  179.01MB 27.52% 59.65%   179.01MB 27.52%  bytes.(*Buffer).grow
  131.51MB 20.22% 79.86%   519.52MB 79.86%  github.com/Songmu/go-ltsv.makeStructWriter.func1
  131.01MB 20.14%   100%   131.01MB 20.14%  bytes.NewBuffer (inline)
         0     0%   100%   179.01MB 27.52%  bytes.(*Buffer).WriteString
         0     0%   100%   650.53MB   100%  github.com/Songmu/go-ltsv.BenchmarkMarshalStruct
         0     0%   100%   650.53MB   100%  github.com/Songmu/go-ltsv.Marshal
         0     0%   100%   519.52MB 79.86%  github.com/Songmu/go-ltsv.MarshalTo
         0     0%   100%      124MB 19.06%  github.com/Songmu/go-ltsv.elemWriter.func1
         0     0%   100%   209.01MB 32.13%  github.com/Songmu/go-ltsv.makeFloatWriter.func1

What's next

The data2map function still allocates much memory but I think this is difficult to fix. Allocation happens at string(data) and l := ltsvMap{} (and its resizing). The problem is that it's hard to pool a map in Golang. The allocation in strconv.FormatFloat of Marshal may be resolved by fmt.Fprintf but cannot be used with writeField.

@itchyny itchyny changed the title Improve performance by reducing allocations of strings.Split Reduce allocations by avoiding strings.Split Sep 3, 2020
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 78.932% when pulling e268038 on itchyny:reduce-allocation into c30af2b on Songmu:master.

itchyny
itchyny commented Sep 3, 2020
View reviewed changes
decode.go
return nil, fmt.Errorf("not a ltsv: %s", d)
}
l[strings.TrimSpace(kv[0])] = strings.TrimSpace(kv[1])
l[strings.TrimSpace(v[:k])] = strings.TrimSpace(v[k+1:])
Copy link
Contributor Author

@itchyny itchyny Sep 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think triming spaces of values is not in the spec 🤔

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly, this is not specified in the specification, but some people may open space like key: value\tkey2: value2, so I use this behavior as it is.

@Songmu Songmu merged commit a608c3f into Songmu:master Sep 3, 2020
@Songmu
Copy link
Owner

Songmu commented Sep 3, 2020

Great work! Thanks!

Songmu added a commit that referenced this pull request May 21, 2022
@Songmu
Checking in changes prior to tagging of version v0.1.0
88e760e 
## [v0.1.0](69c2279...v0.1.0) (2022-05-22)

* introduce Go Modules and GitHub Actions [#6](#6) ([Songmu](https://github.com/Songmu))
* Support encoding/decoding bool value [#5](#5) ([hottestseason](https://github.com/hottestseason))
* Reduce allocations by avoiding strings.Split [#4](#4) ([itchyny](https://github.com/itchyny))
* Make encoding 2x faster [#3](#3) ([harukasan](https://github.com/harukasan))
* introduce teble driven tests [#2](#2) ([Songmu](https://github.com/Songmu))
* care Null number [#1](#1) ([Songmu](https://github.com/Songmu))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Songmu Songmu Songmu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@itchyny @coveralls @Songmu