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

unique: N layers of nested unique.Handle take N GC cycles to reclaim #71846

Open
mknyszek opened this issue Feb 19, 2025 · 1 comment
Open

unique: N layers of nested unique.Handle take N GC cycles to reclaim #71846

mknyszek opened this issue Feb 19, 2025 · 1 comment
Assignees
Labels
BugReport Issues describing a possible bug in the Go implementation. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mknyszek
Copy link
Contributor

Because of the internal representation of unique values, making chains of N unique.Handles requires at minimum N GC cycles to reclaim. The root of the issue is the fact that we store canonical values in something like a map[T]weak.Pointer[T]. They key in the map keeps handles further down the chain alive until they're removed from the map.

We can fix this by using a more bespoke data structure that only stores canonical values as weak.Pointer[T] and nothing else, as in https://go.dev/cl/650256.

Below is a demonstration of the issue.

type nestedHandle struct {
	next Handle[testNestedHandle]
	arr  [6]int
}

var n0, n1, n2, n3 nestedHandle

func TestNestedHandle(t *testing.T) {
	n0 = nestedHandle{arr: [6]int{1, 2, 3, 4, 5, 6}}
	n1 = nestHandle(n0)
	n2 = nestHandle(n1)
	n3 = nestHandle(n2)

	n0 = nestedHandle{}
	n1 = nestedHandle{}
	n2 = nestedHandle{}
	n3 = nestedHandle{}

	runtime.GC() // Today, this will only enable n3 to be reclaimed.
}

func nestHandle(n nestedHandle) nestedHandle {
	return nestedHandle{
		next: unique.Make(n),
		arr:  n.arr,
	}
}

https://go.dev/cl/650256 resolves this issue.

@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 19, 2025
@mknyszek mknyszek added this to the Go1.25 milestone Feb 19, 2025
@mknyszek mknyszek self-assigned this Feb 19, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/650256 mentions this issue: unique: use a bespoke canonicalization map and runtime.AddCleanup

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants