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.

Sign up for GitHub

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

Jump to bottom

Use modelpb.<Type>FromVTPool wherever possible #130

Closed
marclop opened this issue Jul 28, 2023 · 6 comments
Closed

Use modelpb.<Type>FromVTPool wherever possible #130

marclop opened this issue Jul 28, 2023 · 6 comments

Comments

@marclop
Copy link
Contributor

marclop commented Jul 28, 2023

Description

Since we added back pooling in #128, we should start using the pooled modelpb.<Type>FromVTPool wherever possible and indicate that clients should use ReturnToVTPool after they're done processing an event.
@marclop marclop changed the title Use modelpb.<Type>FromVTPool wherever possible Use modelpb.<Type>FromVTPool wherever possible Jul 28, 2023
@marclop
Copy link
Contributor Author

marclop commented Jul 28, 2023

I started looking at changing at least the Intake/v2 decoding.

@marclop
Copy link
Contributor Author

marclop commented Aug 1, 2023

I started looking at this but stopped since I don't have the bandwidth to take this on ATM. Feel free to pick this up as desired.

@kruskall kruskall mentioned this issue Sep 1, 2023
Closed
@kruskall kruskall mentioned this issue Sep 13, 2023
Merged
@kruskall
Copy link
Member

Upon further investigation it looks like we cannot use this because the base event is cloned and CloneVT does not use the pool.

We would pool one event and return len(batch) events to the pool, leading to increased memory usage.

@kruskall
Copy link
Member

Blocked by planetscale/vtprotobuf#96 and planetscale/vtprotobuf#95

@kruskall kruskall mentioned this issue Sep 14, 2023
Closed
@simitt simitt added the blocked label Sep 14, 2023
@kruskall
Copy link
Member

We should evaluate whether we can replace some of the maps with slices as they get reused when using the pool, while maps are just thrown away.

This was referenced Sep 20, 2023
Merged
Merged
Merged
Merged
@simitt simitt removed the blocked label Feb 9, 2024
@axw
Copy link
Member

axw commented Mar 22, 2024

Is there anything left to do here?

@axw axw closed this as completed Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: use vtpool wherever possible kruskall/apm-data
4 participants
@axw @simitt @marclop @kruskall