Skip to content

optimize large slice alloc in Stats#105

Merged
safchain merged 1 commit intosafchain:masterfrom
halfcrazy:stats-perf
May 7, 2025
Merged

optimize large slice alloc in Stats#105
safchain merged 1 commit intosafchain:masterfrom
halfcrazy:stats-perf

Conversation

@halfcrazy
Copy link
Copy Markdown
Contributor

fix #104

Introduce sync pool to optimize large object allocation

$ benchstat bench.out.master bench.out.perf
goos: linux
goarch: amd64
pkg: github.com/safchain/ethtool
cpu: Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
               │ bench.out.master │            bench.out.perf            │
               │      sec/op      │    sec/op     vs base                │
Stats/enp0s3-4       789.0µ ± 11%   398.5µ ± 14%  -49.49% (p=0.002 n=10)
Stats/enp0s8-4       777.9µ ±  8%   373.7µ ±  7%  -51.96% (p=0.000 n=10)
geomean              783.4µ         385.9µ        -50.74%

               │ bench.out.master │            bench.out.perf             │
               │       B/op       │     B/op       vs base                │
Stats/enp0s3-4    1300.243Ki ± 0%   3.576Ki ± 30%  -99.72% (p=0.000 n=10)
Stats/enp0s8-4    1300.238Ki ± 0%   3.896Ki ± 11%  -99.70% (p=0.000 n=10)
geomean              1.270Mi        3.732Ki        -99.71%

               │ bench.out.master │           bench.out.perf           │
               │    allocs/op     │ allocs/op   vs base                │
Stats/enp0s3-4         59.00 ± 0%   51.00 ± 0%  -13.56% (p=0.000 n=10)
Stats/enp0s8-4         59.00 ± 0%   51.00 ± 0%  -13.56% (p=0.000 n=10)
geomean                59.00        51.00       -13.56%

Comment on lines +112 to +127
var (
gstringsPool = sync.Pool{
New: func() interface{} {
// new() will allocate and zero-initialize the struct.
// The large data array within ethtoolGStrings will be zeroed.
return new(ethtoolGStrings)
},
}
statsPool = sync.Pool{
New: func() interface{} {
// new() will allocate and zero-initialize the struct.
// The large data array within ethtoolStats will be zeroed.
return new(ethtoolStats)
},
}
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we in addition to this give control to the customer to pass in the buffer? This implementation will be really good if someone wants a threadsafe operation however it will have overhead if you compare to a static one time buffer allocation.

@ritwikranjan
Copy link
Copy Markdown
Contributor

This is the change i was planning: #106, maybe we can combine the two of them. This change can be done as a default and then we would also expose those buffers for customers to have them manage those variables themselves.

@halfcrazy
Copy link
Copy Markdown
Contributor Author

This is the change i was planning: #106, maybe we can combine the two of them. This change can be done as a default and then we would also expose those buffers for customers to have them manage those variables themselves.

That's ok. The original Stats will use the in-tree buffer pool.

@ritwikranjan
Copy link
Copy Markdown
Contributor

We can merge this anyway, I can rebase my changes on top of it, and we can look if it makes sense or not!
This definitely looks good to me.

Copy link
Copy Markdown

@anubhabMajumdar anubhabMajumdar left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@safchain safchain merged commit c9730e3 into safchain:master May 7, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

High Memory Usage with ethtool.Stats Function in Go 1.24+

4 participants