packing: Add Rollback/CommitExntension

This enables to dry-run to extend pages without cloning.

Fixes #990

internal/packing/packing.go

@@ -27,6 +27,8 @@ type Page struct {
 	root    *Node
 	size    int
 	maxSize int
+
+	rollbackExtension func()
 }
 
 func NewPage(initSize int, maxSize int) *Page {
@@ -208,6 +210,10 @@ func walk(n *Node, f func(n *Node) error) error {
 }
 
 func (p *Page) Extend(count int) bool {
+	if p.rollbackExtension != nil {
+		panic("packing: Extend cannot be called just after Extend")
+	}
+
 	if p.size >= p.maxSize {
 		return false
 	}
@@ -230,6 +236,8 @@ func (p *Page) Extend(count int) bool {
 		return nil
 	})
 	if aborted {
+		origRoot := *p.root
+
 		leftUpper := p.root
 		leftLower := &Node{
 			x:      0,
@@ -264,47 +272,56 @@ func (p *Page) Extend(count int) bool {
 		}
 		left.parent = p.root
 		right.parent = p.root
+
+		origSize := p.size
+		p.rollbackExtension = func() {
+			p.size = origSize
+			p.root = &origRoot
+		}
 	} else {
+		origSize := p.size
+		origWidths := map[*Node]int{}
+		origHeights := map[*Node]int{}
+
 		for _, n := range edgeNodes {
 			if n.x+n.width == p.size {
+				origWidths[n] = n.width
 				n.width += newSize - p.size
 			}
 			if n.y+n.height == p.size {
+				origHeights[n] = n.height
 				n.height += newSize - p.size
 			}
 		}
+
+		p.rollbackExtension = func() {
+			p.size = origSize
+			for n, w := range origWidths {
+				n.width = w
+			}
+			for n, h := range origHeights {
+				n.height = h
+			}
+		}
 	}
 
 	p.size = newSize
 	return true
 }
 
-func (n *Node) clone() *Node {
+// RollbackExtension rollbacks Extend call once.
-	if n == nil {
+func (p *Page) RollbackExtension() {
-		return nil
+	if p.rollbackExtension == nil {
+		panic("packing: RollbackExtension cannot be called without Extend")
 	}
-	cloned := &Node{
+	p.rollbackExtension()
-		x:      n.x,
+	p.rollbackExtension = nil
-		y:      n.y,
-		width:  n.width,
-		height: n.height,
-		used:   n.used,
-		child0: n.child0.clone(),
-		child1: n.child1.clone(),
-	}
-	if cloned.child0 != nil {
-		cloned.child0.parent = cloned
-	}
-	if cloned.child1 != nil {
-		cloned.child1.parent = cloned
-	}
-	return cloned
 }
 
-func (p *Page) Clone() *Page {
+// CommitExtension commits Extend call.
-	return &Page{
+func (p *Page) CommitExtension() {
-		root:    p.root.clone(),
+	if p.rollbackExtension == nil {
-		size:    p.size,
+		panic("packing: RollbackExtension cannot be called without Extend")
-		maxSize: p.maxSize,
 	}
+	p.rollbackExtension = nil
 }

internal/packing/packing_test.go

@@ -261,15 +261,31 @@ func TestExtend(t *testing.T) {
 	if p.Size() != s*2 {
 		t.Errorf("p.Size(): got: %d, want: %d", p.Size(), s*2)
 	}
-	if p.Alloc(s*3/2, s*2) == nil {
+	n0 := p.Alloc(s*3/2, s*2)
+	if n0 == nil {
 		t.Errorf("p.Alloc failed: width: %d, height: %d", s*3/2, s*2)
 	}
-	if p.Alloc(s/2, s*3/2) == nil {
+	n1 := p.Alloc(s/2, s*3/2)
+	if n1 == nil {
 		t.Errorf("p.Alloc failed: width: %d, height: %d", s/2, s*3/2)
 	}
 	if p.Alloc(1, 1) != nil {
 		t.Errorf("p.Alloc must fail: width: %d, height: %d", 1, 1)
 	}
+	p.Free(n1)
+	p.Free(n0)
+
+	p.RollbackExtension()
+
+	if got, want := p.Size(), s; got != want {
+		t.Errorf("p.Size(): got: %d, want: %d", got, want)
+	}
+	if p.Alloc(s*3/2, s*2) != nil {
+		t.Errorf("p.Alloc(%d, %d) must fail but not", s*3/2, s*2)
+	}
+	if p.Alloc(s/2, s*3/2) != nil {
+		t.Errorf("p.Alloc(%d, %d) must fail but not", s/2, s*3/2)
+	}
 }
 
 func TestExtend2(t *testing.T) {
@@ -285,13 +301,30 @@ func TestExtend2(t *testing.T) {
 	if p.Size() != s*2 {
 		t.Errorf("p.Size(): got: %d, want: %d", p.Size(), s*2)
 	}
-	if p.Alloc(s, s*2) == nil {
+
+	n3 := p.Alloc(s, s*2)
+	if n3 == nil {
 		t.Errorf("p.Alloc failed: width: %d, height: %d", s, s*2)
 	}
-	if p.Alloc(s, s) == nil {
+	n4 := p.Alloc(s, s)
+	if n4 == nil {
 		t.Errorf("p.Alloc failed: width: %d, height: %d", s, s)
 	}
 	if p.Alloc(s, s) != nil {
 		t.Errorf("p.Alloc must fail: width: %d, height: %d", s, s)
 	}
+	p.Free(n4)
+	p.Free(n3)
+
+	p.RollbackExtension()
+
+	if got, want := p.Size(), s; got != want {
+		t.Errorf("p.Size(): got: %d, want: %d", got, want)
+	}
+	if p.Alloc(s, s*2) != nil {
+		t.Errorf("p.Alloc(%d, %d) must fail but not", s, s*2)
+	}
+	if p.Alloc(s, s) != nil {
+		t.Errorf("p.Alloc(%d, %d) must fail but not", s, s)
+	}
 }

internal/shareable/image.go

@@ -96,23 +96,21 @@ func (b *backend) TryAlloc(width, height int) (*packing.Node, bool) {
 		return n, true
 	}
 
-	// Simulate the extending the page and calculate the appropriate page size.
+	nExtended := 1
-	// By simulating, we can avoid unnecessary extention of underlying textures.
-	page := b.page.Clone()
-	nExtended := 0
 	for {
-		if !page.Extend(1) {
+		if !b.page.Extend(nExtended) {
 			// The page can't be extended any more. Return as failure.
 			return nil, false
 		}
 		nExtended++
-		if n := page.Alloc(width, height); n != nil {
+		if n := b.page.Alloc(width, height); n != nil {
 			// The page is just for emulation, so we don't have to free it.
+			b.page.CommitExtension()
 			break
 		}
+		b.page.RollbackExtension()
 	}
 
-	b.page.Extend(nExtended)
 	s := b.page.Size()
 	b.restorable = b.restorable.Extend(s, s)