Skip to content

fix: add StatsWithBuffer method for optimized stats retrieval#106

Merged
safchain merged 1 commit intosafchain:masterfrom
ritwikranjan:fix/ethtool-memory-issue-1
May 29, 2025
Merged

fix: add StatsWithBuffer method for optimized stats retrieval#106
safchain merged 1 commit intosafchain:masterfrom
ritwikranjan:fix/ethtool-memory-issue-1

Conversation

@ritwikranjan
Copy link
Copy Markdown
Contributor

@ritwikranjan ritwikranjan commented May 7, 2025

This pull request refactors the ethtool.go file to improve code maintainability by introducing a new method for buffer management, and ensuring backward compatibility. Below are the most significant changes:

Type Renaming for Consistency:

  • Renamed ethtoolGStrings to EthtoolGStrings and ethtoolStats to EthtoolStats to align with Go naming conventions for exported types. This change affects type definitions and all instances where these types are used. [1] [2] [3]

Buffer Management Enhancements:

  • Introduced a new method StatsWithBuffer that allows pre-allocated buffers (EthtoolGStrings and EthtoolStats) to be passed in, reducing heap allocations and improving performance in Go 1.24+.

Backward Compatibility:

  • Updated the existing Stats method to use the new StatsWithBuffer internally while maintaining its original interface, ensuring backward compatibility with existing code. [1] [2] [3]

…ntain backward compatibility

Signed-off-by: Ritwik Ranjan <ritwikranjan@microsoft.com>
@ritwikranjan ritwikranjan force-pushed the fix/ethtool-memory-issue-1 branch from 06f55ef to 1723619 Compare May 22, 2025 16:51
@ritwikranjan
Copy link
Copy Markdown
Contributor Author

@safchain @halfcrazy I have rebased on top of your changes, this gives the user independence in management of the buffer memory. Let me know if it looks good to you!

@ritwikranjan
Copy link
Copy Markdown
Contributor Author

A soft reminder to look at the PR @safchain / @halfcrazy

@halfcrazy
Copy link
Copy Markdown
Contributor

/lgtm

@safchain safchain merged commit f9520b6 into safchain:master May 29, 2025
2 checks passed
@ritwikranjan
Copy link
Copy Markdown
Contributor Author

@safchain Can we make a release as well?

@safchain
Copy link
Copy Markdown
Owner

Just released the v0.6.1

@ritwikranjan
Copy link
Copy Markdown
Contributor Author

Perfect, thanks for the quick turnaround!

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.

3 participants