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

Fixes missing Reset in ResetVT #84

Merged
merged 4 commits into from
Aug 29, 2023
Merged

Conversation

cyriltovena
Copy link
Contributor

@cyriltovena cyriltovena commented Jul 22, 2023

When using Pooling and so ResetVT we definitively want to reuse slices.

However a bug was recently discovered where it would not zeroed slice items.

#35 introduces a fixes by simply removing slice recycling which I think makes pooling a bit less interesting.

This PR rollback #35 and takes another approach that actually Reset the member even if ResetVT is not available because the feature pool is deactivate for that type.

I can't generate anything unfortunately I don't have the protoc version I'd appreciate help for re-generating those.

@cyriltovena
Copy link
Contributor Author

Please squash merge if approved !

Copy link

@carsonip carsonip left a comment

Choose a reason for hiding this comment

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

Had the same issue with pooling and can confirm that this fix works. Could you please also include "Closes #64" in the PR description?

@vmg vmg merged commit 7252f78 into planetscale:main Aug 29, 2023
@vmg
Copy link
Member

vmg commented Aug 29, 2023

Thank you! Merged and fixed the conflicts manually.

axw added a commit to elastic/apm-aggregation that referenced this pull request Aug 30, 2023
Now that planetscale/vtprotobuf#84 is merged,
revert to using the upstream project. We should update again when
there's a new release.
axw added a commit to elastic/apm-aggregation that referenced this pull request Aug 30, 2023
* Revert to planetscale/vtprotobuf

Now that planetscale/vtprotobuf#84 is merged,
revert to using the upstream project. We should update again when
there's a new release.

* go mod tidy
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.

None yet

3 participants