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

unique: large string still referenced, after interning only a small substring [1.23 backport] #69383

Closed
gopherbot opened this issue Sep 10, 2024 · 2 comments
Closed

unique: large string still referenced, after interning only a small substring [1.23 backport] #69383

gopherbot opened this issue Sep 10, 2024 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases
Milestone
Go1.23.2

Comments

@gopherbot
Copy link
Contributor

@ianlancetaylor requested issue #69370 to be considered for backport to the next 1.23 minor release.

@gopherbot Please open backport to 1.23

Since the unique package is new in 1.23, we should implement this optimization there as well, for consistent behavior.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 10, 2024
@gopherbot gopherbot mentioned this issue Sep 10, 2024
Closed
@gopherbot gopherbot added this to the Go1.23.2 milestone Sep 10, 2024
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/612295 mentions this issue: [release-branch.go1.23] unique: don't retain uncloned input as key

@timothy-king timothy-king added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Sep 11, 2024
gopherbot pushed a commit that referenced this issue Sep 11, 2024
@mknyszek @gopherbot
[release-branch.go1.23] unique: don't retain uncloned input as key
a74951c 
Currently the unique package tries to clone strings that get stored in
its internal map to avoid retaining large strings.

However, this falls over entirely due to the fact that the original
string is *still* stored in the map as a key. Whoops. Fix this by
storing the cloned value in the map instead.

This change also adds a test which fails without this change.

For #69370.
Fixes #69383.

Change-Id: I1a6bb68ed79b869ea12ab6be061a5ae4b4377ddb
Reviewed-on: https://go-review.googlesource.com/c/go/+/610738
Reviewed-by: Michael Pratt <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
(cherry picked from commit 21ac23a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/612295
Auto-Submit: Tim King <[email protected]>
@gopherbot
Copy link
Contributor Author

Closed by merging CL 612295 (commit a74951c) to release-branch.go1.23.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases
Projects
None yet
Milestone
Go1.23.2
Development

No branches or pull requests
2 participants
@timothy-king @gopherbot