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

fix: Reuse buffer in stream reader #200

Merged
merged 2 commits into from
Sep 18, 2023
Merged

fix: Reuse buffer in stream reader #200

merged 2 commits into from
Sep 18, 2023

Conversation

MikeGoldsmith
Copy link
Contributor

Which problem is this PR solving?

After further investigation using profiling, we still see high in-use objects in memory that creep up over an extended time. It looks like each call to bufio.NewReader allocates a new 4mb byte[] which is never cleaned up correctly.

image

This PR moves the bufio.Reader to the the reader struct and reuses it when processing packet data.

Short description of the changes

  • Add bufio.Reader to tcpReader and initalise during NewTcpReader
  • Reuse the buffered reader when processing new packet data by calling Reset with the new data.

Note: We do not cache the io.Reader as this doesn't hold onto the underlying []byte and is cleaned up as expected. See the image above for bytes.NewReader.

How to verify that this has the expected result

Memory consumption over an extended period is more stable and does not incrementally creep up.

@MikeGoldsmith MikeGoldsmith added the type: bug Something isn't working label Sep 18, 2023
@MikeGoldsmith MikeGoldsmith requested a review from a team September 18, 2023 13:57
@MikeGoldsmith MikeGoldsmith self-assigned this Sep 18, 2023
Copy link
Member

@robbkidd robbkidd left a comment

Choose a reason for hiding this comment

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

buffering

Copy link
Contributor

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

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

reduce reuse recycle

@MikeGoldsmith MikeGoldsmith merged commit 0a8216c into main Sep 18, 2023
3 checks passed
@MikeGoldsmith MikeGoldsmith deleted the mike/reuse-buffer branch September 18, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants