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

x/website: gc-guide: document performance characteristics of weak pointer implementation #71779

Open
Fabianexe opened this issue Feb 16, 2025 · 3 comments
Open

x/website: gc-guide: document performance characteristics of weak pointer implementation #71779

Fabianexe opened this issue Feb 16, 2025 · 3 comments
Assignees
@mknyszek
Labels
BugReport Issues describing a possible bug in the Go implementation. Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@Fabianexe
Copy link

Go version

go version go1.24.0 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/foo/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/foo/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/85/pvl_f0rs14n1c5n76py38wg40000gn/T/go-build2635512519=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/foo/Repo/memory/go.mod'
GOMODCACHE='/Users/foo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/foo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/foo/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Run simple benchmark like:

func BenchmarkWeakPointer(b *testing.B) {
	l := []int{1, 10, 100, 1000, 10000, 100000, 1000000, 10000000}
	for _, n := range l {
		pointers := make([]weak.Pointer[int], n)

		b.Run(strconv.Itoa(n), func(b *testing.B) {
			for b.Loop() {
				runtime.GC()
			}
		})
		runtime.KeepAlive(pointers)
		runtime.GC()
	}
}

What did you see happen?

The GC cycles takes longer as there are more live weak pointers on the heap.

What did you expect to see?

The whole "Objects that are only pointed to by weak pointers are not considered reachable" narrative leads me to believe that the number of live weak pointers has no influence on the GC. After all, the GC mark phase should be able to ignore all weak pointers if they have no impact on reachability.

If the mark phase of the GC does, in fact, need to consider weak pointers, I believe this should at least be mentioned in the GoDoc for weak pointers. A warning indicating that the GC may take weak pointers into account would be helpful.
@gabyhelp
Copy link

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Feb 16, 2025
@Jorropo
Copy link
Member

Jorropo commented Feb 16, 2025

We can't have zero cost weak pointers because we can't not have any kind of tracking otherwise .Value would not be able to know when to return nil.

Do you want to a CL tweaking the documentation ?

@mknyszek mknyszek changed the title weak: WeakPointers slows down the GC weak: pointers are still pointers under the hood that the GC has to scan Feb 18, 2025
@mknyszek mknyszek changed the title weak: pointers are still pointers under the hood that the GC has to scan weak: weak pointers are still pointers under the hood that the GC has to scan Feb 18, 2025
@mknyszek
Copy link
Contributor

From my perspective, "Objects that are only pointed to by weak pointers are not considered reachable" is very clearly about only about reachability and not the implementation. As @Jorropo points out, any weak pointer implementation will require tracking by the GC, even one that doesn't affect your benchmark. (I could write a different benchmark that shows little cost with the current implementation, and a high cost in an alternative implementation -- in the end, I don't think it matters that much to the API docs which are about semantics, and the semantics are already subtle.)

I think the right place for discussing implementation details and performance characteristics is the GC guide (https://go.dev/doc/gc-guide), which now has a section about weak pointers. If you would like to send a patch, feel free (https://go.dev/doc/contribute). It's in the https://go.googlesource.com/website repository under _content/doc.

@mknyszek mknyszek changed the title weak: weak pointers are still pointers under the hood that the GC has to scan weak: document performance characteristics of weak pointer implementation Feb 18, 2025
@mknyszek mknyszek added Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done. labels Feb 18, 2025
@mknyszek mknyszek added this to the Backlog milestone Feb 18, 2025
@mknyszek mknyszek changed the title weak: document performance characteristics of weak pointer implementation x/website: gc-guide: document performance characteristics of weak pointer implementation Feb 18, 2025
@mknyszek mknyszek modified the milestones: Backlog, Unreleased Feb 18, 2025
@mknyszek mknyszek self-assigned this Feb 21, 2025
@findleyr findleyr removed the website label Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mknyszek mknyszek

Labels
BugReport Issues describing a possible bug in the Go implementation. Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@mknyszek @Fabianexe @gopherbot @Jorropo @findleyr @gabyhelp