search

vmware / go-ipfix

An ipfix library in Go
Other
39 stars 27 forks source link

Improve memory performance of collector #331

Closed antoninbas closed 11 months ago

antoninbas commented 11 months ago

Avoid unnecessary slice creation when adding a record to a set. We implement this change for both data record and template records, but in practice most of the gains will come from data records.

Memory allocations (in bytes) are reduced by around 15% for BenchmarkCollector.

antoninbas commented 11 months ago

Before change:

goos: darwin
goarch: amd64
pkg: github.com/vmware/go-ipfix/pkg/test
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkCollector/ipv4-12               304       3809314 ns/op     1832212 B/op      45001 allocs/op
BenchmarkCollector/ipv6-12               304       3852429 ns/op     1922795 B/op      45000 allocs/op
PASS

After change:

goos: darwin
goarch: amd64
pkg: github.com/vmware/go-ipfix/pkg/test
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkCollector/ipv4-12               306       3785402 ns/op     1512218 B/op      44001 allocs/op
BenchmarkCollector/ipv6-12               310       3828923 ns/op     1602773 B/op      44000 allocs/op
PASS
antoninbas commented 11 months ago

@heanlan @dreamtalen let me know if you think this is not a correct change (functionality-wise)

codecov[bot] commented 11 months ago

Codecov Report

Merging #331 (6d7c889) into main (c5e0992) will decrease coverage by 0.39%. The diff coverage is 26.31%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vmware/go-ipfix/pull/331/graphs/tree.svg?width=650&height=150&src=pr&token=448A94JZ59&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware)](https://app.codecov.io/gh/vmware/go-ipfix/pull/331?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) ```diff @@ Coverage Diff @@ ## main #331 +/- ## ========================================== - Coverage 73.25% 72.87% -0.39% ========================================== Files 19 19 Lines 2808 2831 +23 ========================================== + Hits 2057 2063 +6 - Misses 581 599 +18 + Partials 170 169 -1 ``` | [Flag](https://app.codecov.io/gh/vmware/go-ipfix/pull/331/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | Coverage Δ | | |---|---|---| | [integration-tests](https://app.codecov.io/gh/vmware/go-ipfix/pull/331/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | `51.53% <100.00%> (ø)` | | | [unit-tests](https://app.codecov.io/gh/vmware/go-ipfix/pull/331/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | `71.88% <26.31%> (-0.45%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/vmware/go-ipfix/pull/331?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | Coverage Δ | | |---|---|---| | [pkg/collector/process.go](https://app.codecov.io/gh/vmware/go-ipfix/pull/331?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#diff-cGtnL2NvbGxlY3Rvci9wcm9jZXNzLmdv) | `86.03% <100.00%> (ø)` | | | [pkg/entities/set.go](https://app.codecov.io/gh/vmware/go-ipfix/pull/331?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#diff-cGtnL2VudGl0aWVzL3NldC5nbw==) | `66.15% <60.00%> (+3.46%)` | :arrow_up: | | [pkg/entities/record.go](https://app.codecov.io/gh/vmware/go-ipfix/pull/331?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#diff-cGtnL2VudGl0aWVzL3JlY29yZC5nbw==) | `59.25% <13.33%> (-12.56%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/vmware/go-ipfix/pull/331/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware)
antoninbas commented 11 months ago

LGTM

Do we still need to keep the original AddRecord?

Since it's a public API of the library, and there was no strong reason to remove it, I just kept it