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

runtime: gcControllerState.heapLive from mgcpacer.go inconsistent with gc-guide's Live Heap #65195

Open
smiletrl opened this issue Jan 22, 2024 · 3 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone

Comments

@smiletrl
Copy link
Contributor

smiletrl commented Jan 22, 2024

Go version

go version devel go1.22-8db131082d Thu Jan 4 23:31:17 2024 +0000 darwin/amd64

Output of go env in your module/workspace:

GO111MODULE='auto'
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/smiletrl/Library/Caches/go-build'
GOENV='/Users/smiletrl/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/smiletrl/go/src/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/smiletrl/go/src'
GOPRIVATE=''
GOPROXY='https://goproxy.cn,direct'
GOROOT='/Users/smiletrl/go/src/github.com/smiletrl/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/smiletrl/go/src/github.com/smiletrl/go/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='devel go1.22-8db131082d Thu Jan 4 23:31:17 2024 +0000'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/smiletrl/go/src/github.com/smiletrl/go/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/sq/2123x_0d19s90tx1q2wpgtxr0000gn/T/go-build2832210919=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I'm looking at https://tip.golang.org/doc/gc-guide, trying to find the parameter to tune the appropriate memory limit. One interesting value is "Live Heap" from this graph.
Screenshot 2024-01-22 at 10 14 32

My understanding is "Live Heap" represents the left heap memory after each GC cycle (after sweep phase, and remain the same as time goes), i.e, the marked memory after each GC (provided GCPercent not changed). This is consistent with this issue #56857, which takes heapMarked as Live Heap value.

However, there's a specific field heapLive, along with heapMarked

type gcControllerState struct {
	// heapLive is the number of bytes considered live by the GC.
	// That is: retained by the most recent GC plus allocated
	// since then. heapLive ≤ memstats.totalAlloc-memstats.totalFree, since
	// heapAlloc includes unmarked objects that have not yet been swept (and
	// hence goes up as we allocate and down as we sweep) while heapLive
	// excludes these objects (and hence only goes up between GCs).
	//
	// To reduce contention, this is updated only when obtaining a span
	// from an mcentral and at this point it counts all of the unallocated
	// slots in that span (which will be allocated before that mcache
	// obtains another span from that mcentral). Hence, it slightly
	// overestimates the "true" live heap size. It's better to overestimate
	// than to underestimate because 1) this triggers the GC earlier than
	// necessary rather than potentially too late and 2) this leads to a
	// conservative GC rate rather than a GC rate that is potentially too
	// low.
	//
	// Whenever this is updated, call traceHeapAlloc() and
	// this gcControllerState's revise() method.
	heapLive atomic.Uint64

	// heapMarked is the number of bytes marked by the previous
	// GC. After mark termination, heapLive == heapMarked, but
	// unlike heapLive, heapMarked does not change until the
	// next mark termination.
	heapMarked uint64
}

heapLive from above gcControllerState context actually grows as new allocate happens, and in most time heapLive > heapMarked. After GC Mark phase work done, heapLive = heapMarked . This is a bit confusing for the two terminologies heapLive and Live Heap.

What did you see happen?

heapLive and Live Heap are very closed naming, but mean something quite different across code and documentation.

What did you expect to see?

Either heapLive or Live Heap gets updated to a better name. At this moment, updating Live heap seems more appropriate IMO, maybe Marked Heap ^?

/gc/heap/live:bytes could probably be renamed to /gc/heap/marked:bytes.

@bcmills bcmills changed the title src/runtime: gcControllerState.heapLive from mgcpacer.go inconsistent with gc-guide's Live Heap runtime: gcControllerState.heapLive from mgcpacer.go inconsistent with gc-guide's Live Heap Jan 22, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 22, 2024
@prattmic
Copy link
Member

cc @mknyszek

@mknyszek
Copy link
Contributor

mknyszek commented Jan 22, 2024

heapLive is an implementation detail that's well-documented, so this seems very minor to me.

Either heapLive or Live Heap gets updated to a better name. At this moment, updating Live heap seems more appropriate IMO, maybe Marked Heap ^?

/gc/heap/live:bytes could probably be renamed to /gc/heap/marked:bytes.

We've mostly settled on "live heap" outside of the runtime. It will be a lot less effort to refactor the runtime than it will be to change the docs and deprecate /gc/heap/live:bytes. (On a more philosophical note, I don't really like "marked" for the metric because it's not very robust to alternative GC schemes. For example, it doesn't really make sense in a deferred reference counting scheme.)

I think the only actionable part of this issue is really just a minor cleanup task to clean up the names in the runtime a little bit.

@mknyszek mknyszek self-assigned this Jan 22, 2024
@mknyszek mknyszek added this to the Backlog milestone Jan 22, 2024
@smiletrl
Copy link
Contributor Author

I'm also available for this issue's work if @mknyszek you have some preferred name change for heapLive ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
Development

No branches or pull requests

4 participants