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.

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

Avoid using binary.Read #140

Closed
vincentbernat opened this issue Jan 16, 2023 · 3 comments
Closed

Avoid using binary.Read #140

vincentbernat opened this issue Jan 16, 2023 · 3 comments
Labels
enhancement New feature or request ipfix Related to IPFIX protocol sflow Related to sFlow protocol

Comments

@vincentbernat
Copy link
Contributor

Hey!

binary.Read() accounts for about 25% of CPU time when decoding. It's slow because:

  • it requires using the bytes.Buffer abstraction
  • it uses reflection sometimes

I think a quick win would be to scrap the function from Go source code and make it work on a slice instead. And remove the part using the reflection (and fix the caller site in Goflow).

@lspgn
Copy link
Member

lspgn commented Mar 7, 2023

Hello!
Thank you for this suggestion.
Did you do pprof? Would the gain in packets decoded be 25% too?
Will have a look at the PR.

Do you know of any other packet decoding libraries used this approach? Also found a blogpost that seems to mention this.

@lspgn lspgn added enhancement New feature or request ipfix Related to IPFIX protocol sflow Related to sFlow protocol labels Mar 7, 2023
@vincentbernat
Copy link
Contributor Author

I did pprof for Akvorado:

goos: linux
goarch: amd64
pkg: akvorado/inlet/flow
cpu: AMD Ryzen 5 5600X 6-Core Processor
                            │ bench-3.txt  │             bench-4.txt             │
                            │    sec/op    │   sec/op     vs base                │
Netflow/with_encoding-12       7.758µ ± 3%   7.365µ ± 2%   -5.07% (p=0.000 n=10)
Netflow/without_encoding-12    7.304µ ± 2%   6.931µ ± 3%   -5.11% (p=0.000 n=10)
Sflow/with_encoding-12        14.256µ ± 1%   9.834µ ± 2%  -31.02% (p=0.000 n=10)
Sflow/without_encoding-12     13.559µ ± 2%   9.353µ ± 2%  -31.02% (p=0.000 n=10)

@lspgn
Copy link
Member

lspgn commented Sep 4, 2023

Merged #204 for v2 as well.
Thank you!

@lspgn lspgn closed this as completed Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ipfix Related to IPFIX protocol sflow Related to sFlow protocol
Projects
None yet
Development

No branches or pull requests

2 participants