Skip to content

Commit 2644b76

Browse files
aclementsadg
authored andcommitted
runtime: fix pagesInUse accounting
When we grow the heap, we create a temporary "in use" span for the memory acquired from the OS and then free that span to link it into the heap. Hence, we (1) increase pagesInUse when we make the temporary span so that (2) freeing the span will correctly decrease it. However, currently step (1) increases pagesInUse by the number of pages requested from the heap, while step (2) decreases it by the number of pages requested from the OS (the size of the temporary span). These aren't necessarily the same, since we round up the number of pages we request from the OS, so steps 1 and 2 don't necessarily cancel out like they're supposed to. Over time, this can add up and cause pagesInUse to underflow and wrap around to 2^64. The garbage collector computes the sweep ratio from this, so if this happens, the sweep ratio becomes effectively infinite, causing the first allocation on each P in a sweep cycle to sweep the entire heap. This makes sweeping effectively STW. Fix this by increasing pagesInUse in step 1 by the number of pages requested from the OS, so that the two steps correctly cancel out. We add a test that checks that the running total matches the actual state of the heap. Fixes #15022. For 1.6.x. Change-Id: Iefd9d6abe37d0d447cbdbdf9941662e4f18eeffc Reviewed-on: https://go-review.googlesource.com/21280 Run-TryBot: Austin Clements <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Russ Cox <[email protected]> Reviewed-on: https://go-review.googlesource.com/21456 Reviewed-by: Andrew Gerrand <[email protected]>
1 parent bfe54b9 commit 2644b76

File tree

3 files changed

+34
-1
lines changed

3 files changed

+34
-1
lines changed

src/runtime/export_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,19 @@ func SetTracebackEnv(level string) {
171171
setTraceback(level)
172172
traceback_env = traceback_cache
173173
}
174+
175+
func CountPagesInUse() (pagesInUse, counted uintptr) {
176+
stopTheWorld("CountPagesInUse")
177+
178+
pagesInUse = uintptr(mheap_.pagesInUse)
179+
180+
for _, s := range h_allspans {
181+
if s.state == mSpanInUse {
182+
counted += s.npages
183+
}
184+
}
185+
186+
startTheWorld()
187+
188+
return
189+
}

src/runtime/gc_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,20 @@ func testIfaceEqual(x interface{}) {
473473
a = true
474474
}
475475
}
476+
477+
func TestPageAccounting(t *testing.T) {
478+
// Grow the heap in small increments. This used to drop the
479+
// pages-in-use count below zero because of a rounding
480+
// mismatch (golang.org/issue/15022).
481+
const blockSize = 64 << 10
482+
blocks := make([]*[blockSize]byte, (64<<20)/blockSize)
483+
for i := range blocks {
484+
blocks[i] = new([blockSize]byte)
485+
}
486+
487+
// Check that the running page count matches reality.
488+
pagesInUse, counted := runtime.CountPagesInUse()
489+
if pagesInUse != counted {
490+
t.Fatalf("mheap_.pagesInUse is %d, but direct count is %d", pagesInUse, counted)
491+
}
492+
}

src/runtime/mheap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ func (h *mheap) grow(npage uintptr) bool {
671671
}
672672
atomic.Store(&s.sweepgen, h.sweepgen)
673673
s.state = _MSpanInUse
674-
h.pagesInUse += uint64(npage)
674+
h.pagesInUse += uint64(s.npages)
675675
h.freeSpanLocked(s, false, true, 0)
676676
return true
677677
}

0 commit comments

Comments
 (0)