runtime: correct counters in sweep
authorIan Lance Taylor <ian@gcc.gnu.org>
Thu, 13 Sep 2018 22:06:16 +0000 (22:06 +0000)
committerIan Lance Taylor <ian@gcc.gnu.org>
Thu, 13 Sep 2018 22:06:16 +0000 (22:06 +0000)
    In the sweep code we can sometimes see incorrect counts when
    conservative stack scanning causes us to grey an object that we
    earlier decided could be freed.  We already ignored this check, but
    adjust this case to maintain correct span counts when it happens.
    This gives us slightly more correct numbers in MemStats, and helps
    avoid a rare failure in TestReadMemStats.

    Also fix the free index, and cope with finding a full span when
    allocating a new one.

    Reviewed-on: https://go-review.googlesource.com/134216

From-SVN: r264294

gcc/go/gofrontend/MERGE
libgo/go/runtime/mcentral.go
libgo/go/runtime/mgcsweep.go

index 6650b03ec67a8da762f8cb7b100ef8c2f33ddde3..9a789b970e054e3be3350e71f52330828fc03e1a 100644 (file)
@@ -1,4 +1,4 @@
-f2cd046a4e0d681c3d21ee547b437d3eab8af268
+82d7205ba9e5c1fe38fd24f89a45caf2e974975b
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
index eaabcb9c2993b6853ee713902a34f5317eb3f80f..150f4fd8ae3a9d178f8b30499489e0ca4ba3e698 100644 (file)
@@ -56,6 +56,15 @@ retry:
                        c.empty.insertBack(s)
                        unlock(&c.lock)
                        s.sweep(true)
+
+                       // With gccgo's conservative GC, the returned span may
+                       // now be full. See the comments in mspan.sweep.
+                       if uintptr(s.allocCount) == s.nelems {
+                               s.freeindex = s.nelems
+                               lock(&c.lock)
+                               goto retry
+                       }
+
                        goto havespan
                }
                if s.sweepgen == sg-1 {
index c60214cdaf3dee8ea6b502713ae7bbefaf306b9f..d6be349a959391802fc995b92d5298fd523550db 100644 (file)
@@ -296,7 +296,7 @@ func (s *mspan) sweep(preserve bool) bool {
        }
        nfreed := s.allocCount - nalloc
 
-       // This test is not reliable with gccgo, because of
+       // This check is not reliable with gccgo, because of
        // conservative stack scanning. The test boils down to
        // checking that no new bits have been set in gcmarkBits since
        // the span was added to the sweep count. New bits are set by
@@ -309,16 +309,23 @@ func (s *mspan) sweep(preserve bool) bool {
        // check to be inaccurate, and it will keep an object live
        // unnecessarily, but provided the pointer is not really live
        // it is not otherwise a problem. So we disable the test for gccgo.
-       if false && nalloc > s.allocCount {
-               print("runtime: nelems=", s.nelems, " nalloc=", nalloc, " previous allocCount=", s.allocCount, " nfreed=", nfreed, "\n")
-               throw("sweep increased allocation count")
+       nfreedSigned := int(nfreed)
+       if nalloc > s.allocCount {
+               // print("runtime: nelems=", s.nelems, " nalloc=", nalloc, " previous allocCount=", s.allocCount, " nfreed=", nfreed, "\n")
+               // throw("sweep increased allocation count")
+
+               // For gccgo, adjust the freed count as a signed number.
+               nfreedSigned = int(s.allocCount) - int(nalloc)
+               if uintptr(nalloc) == s.nelems {
+                       s.freeindex = s.nelems
+               }
        }
 
        s.allocCount = nalloc
        wasempty := s.nextFreeIndex() == s.nelems
        s.freeindex = 0 // reset allocation index to start of span.
        if trace.enabled {
-               getg().m.p.ptr().traceReclaimed += uintptr(nfreed) * s.elemsize
+               getg().m.p.ptr().traceReclaimed += uintptr(nfreedSigned) * s.elemsize
        }
 
        // gcmarkBits becomes the allocBits.
@@ -334,7 +341,7 @@ func (s *mspan) sweep(preserve bool) bool {
        // But we need to set it before we make the span available for allocation
        // (return it to heap or mcentral), because allocation code assumes that a
        // span is already swept if available for allocation.
-       if freeToHeap || nfreed == 0 {
+       if freeToHeap || nfreedSigned <= 0 {
                // The span must be in our exclusive ownership until we update sweepgen,
                // check for potential races.
                if s.state != mSpanInUse || s.sweepgen != sweepgen-1 {
@@ -347,8 +354,11 @@ func (s *mspan) sweep(preserve bool) bool {
                atomic.Store(&s.sweepgen, sweepgen)
        }
 
-       if nfreed > 0 && spc.sizeclass() != 0 {
-               c.local_nsmallfree[spc.sizeclass()] += uintptr(nfreed)
+       if spc.sizeclass() != 0 {
+               c.local_nsmallfree[spc.sizeclass()] += uintptr(nfreedSigned)
+       }
+
+       if nfreedSigned > 0 && spc.sizeclass() != 0 {
                res = mheap_.central[spc].mcentral.freeSpan(s, preserve, wasempty)
                // MCentral_FreeSpan updates sweepgen
        } else if freeToHeap {