runtime: make mTreap.find actually find the best fit

This change modifies the implementation of mTreap.find to find the
best-fit span with the lowest possible base address.

Fixes #31616.

Change-Id: Ib4bda0f85d7d0590326f939a243a6e4665f37d3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/173479
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/mgclarge.go b/src/runtime/mgclarge.go
index d816183c0cb7004e8ca8806b83f0f5e48b2f9be5..f33ba2345090df1071cbfc5d8ee7ffc54edeeaac 100644 (file)
--- a/src/runtime/mgclarge.go
+++ b/src/runtime/mgclarge.go
@@ -301,27 +301,30 @@ func (root *mTreap) removeNode(t *treapNode) {
 // find searches for, finds, and returns the treap iterator representing
 // the position of the smallest span that can hold npages. If no span has
 // at least npages it returns an invalid iterator.
-// This is slightly more complicated than a simple binary tree search
-// since if an exact match is not found the next larger node is
-// returned.
-// TODO(mknyszek): It turns out this routine does not actually find the
-// best-fit span, so either fix that or move to something else first, and
-// evaluate the performance implications of doing so.
+// This is a simple binary tree search that tracks the best-fit node found
+// so far. The best-fit node is guaranteed to be on the path to a
+// (maybe non-existent) lowest-base exact match.
 func (root *mTreap) find(npages uintptr) treapIter {
+       var best *treapNode
        t := root.treap
        for t != nil {
                if t.spanKey == nil {
                        throw("treap node with nil spanKey found")
                }
-               if t.npagesKey < npages {
-                       t = t.right
-               } else if t.left != nil && t.left.npagesKey >= npages {
+               // If we found an exact match, try to go left anyway. There could be
+               // a span there with a lower base address.
+               //
+               // Don't bother checking nil-ness of left and right here; even if t
+               // becomes nil, we already know the other path had nothing better for
+               // us anyway.
+               if t.npagesKey >= npages {
+                       best = t
                        t = t.left
                } else {
-                       return treapIter{t}
+                       t = t.right
                }
        }
-       return treapIter{}
+       return treapIter{best}
 }
 
 // removeSpan searches for, finds, deletes span along with

diff --git a/src/runtime/treap_test.go b/src/runtime/treap_test.go
index 49d97699ca7b4af1ee0cb62c0cfe80c4b0aa3e44..76e4829d993432d74a8ee69c961acebbb3e0268b 100644 (file)
--- a/src/runtime/treap_test.go
+++ b/src/runtime/treap_test.go
@@ -58,11 +58,7 @@ func TestTreap(t *testing.T) {
                }
                tr.RemoveSpan(spans[0])
        })
-       t.Run("Find", func(t *testing.T) {
-               // Note that Find doesn't actually find the best-fit
-               // element, so just make sure it always returns an element
-               // that is at least large enough to satisfy the request.
-               //
+       t.Run("FindBestFit", func(t *testing.T) {
                // Run this 10 times, recreating the treap each time.
                // Because of the non-deterministic structure of a treap,
                // we'll be able to test different structures this way.
@@ -72,8 +68,10 @@ func TestTreap(t *testing.T) {
                                tr.Insert(s)
                        }
                        i := tr.Find(5)
-                       if i.Span().Pages() < 5 {
-                               t.Fatalf("expected span of size at least 5, got size %d", i.Span().Pages())
+                       if i.Span().Pages() != 5 {
+                               t.Fatalf("expected span of size 5, got span of size %d", i.Span().Pages())
+                       } else if i.Span().Base() != 0xc0040000 {
+                               t.Fatalf("expected span to have the lowest base address, instead got base %x", i.Span().Base())
                        }
                        for _, s := range spans {
                                tr.RemoveSpan(s)