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

introduce go 1.21 slices package behavior in utils #9894

Closed

Conversation

francoposa
Copy link
Member

@francoposa francoposa commented Nov 13, 2024

What this PR does

The information in the header explains:

Package slicesleak is intended to provide access to the Go stdlib slices package
as it existed in Go 1.21, prior to the acceptance of Proposal 63393.

Proposal 63393, implemented in commit e21dc702d54e85381a97259db7deec710108279b
changed the behavior of the slices package to avoid what can be memory leaks in some cases,
by zeroing the slice elements discarded by Delete, DeleteFunc, Compact, CompactFunc, Replace.
Clearing deleted slice elements has a performance overhead and may be undesirable
in cases where the slice is expected to expand to reuse the memory again.

References:

Which issue(s) this PR fixes or relates to

We wanted to use some cleaner interfaces for the ugly standard slices deletes in this PR: #9888
but the post-go1.21 version of the slices implementation had a small but measurable impact on performance.

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@francoposa francoposa requested a review from a team as a code owner November 13, 2024 19:02
Copy link
Contributor

@jhesketh jhesketh left a comment

Choose a reason for hiding this comment

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

I'm not sure how I feel about copying this without tests. I understand why, but if we want to maintain an alternative set of slice functions then I think we need some protections and need to consider writing our own tests for them.

Do we have benchmarks on the performance differences? Are there other alternatives that doesn't increase our maintainership burden.

@francoposa francoposa closed this Nov 14, 2024
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.

2 participants