From 574925cf7a72deaf73be4c481348a7a44f7b7e19 Mon Sep 17 00:00:00 2001
From: divVerent <divVerent@gmail.com>
Date: Sat, 8 Apr 2023 06:03:16 -0700
Subject: [PATCH] packing: remove redundant canAlloc checking and alloc
 allocating. (#2629)

By removing redundant work done in canAlloc and just calling alloc right away,
this removes 35% contribution to AAAAXY loading CPU time on the profile on
https://user-images.githubusercontent.com/251568/230496805-c5e32c19-9258-49c8-800b-a3f0bc3b072d.svg,
or - as measured via stopwatch - brings loading time on Moto G7 Play from
17.22s to 14.94s after already applying #2627.

This should be safe as there is no case in which alloc succeeds and the
allocated region isn't used; also, there is no case in which alloc mutates the
tree when it doesn't actually succeed (comment added in one place to justify
this).

Closes #2628
---
 internal/packing/packing.go | 58 ++++++++++++-------------------------
 1 file changed, 19 insertions(+), 39 deletions(-)

diff --git a/internal/packing/packing.go b/internal/packing/packing.go
index b15f5b1a3..fe827d73e 100644
--- a/internal/packing/packing.go
+++ b/internal/packing/packing.go
@@ -107,32 +107,6 @@ func square(width, height int) float64 {
 	return float64(height) / float64(width)
 }
 
-func (p *Page) canAlloc(n *Node, width, height int) bool {
-	if p.root == nil {
-		return p.width >= width && p.height >= height
-	}
-	return canAlloc(p.root, width, height)
-}
-
-func canAlloc(n *Node, width, height int) bool {
-	if n.width < width || n.height < height {
-		return false
-	}
-	if n.used {
-		return false
-	}
-	if n.child0 == nil && n.child1 == nil {
-		return true
-	}
-	if canAlloc(n.child0, width, height) {
-		return true
-	}
-	if canAlloc(n.child1, width, height) {
-		return true
-	}
-	return false
-}
-
 func alloc(n *Node, width, height int) *Node {
 	if n.width < width || n.height < height {
 		return nil
@@ -178,7 +152,17 @@ func alloc(n *Node, width, height int) *Node {
 				parent: n,
 			}
 		}
-		return alloc(n.child0, width, height)
+		// Note: it now MUST fit, due to above preconditions (repeated here).
+		if n.child0.width < width || n.child0.height < height {
+			panic(fmt.Sprintf("packing: the newly created child node (%d, %d) unexpectedly does not contain the requested size (%d, %d)", n.child0.width, n.child0.height, width, height))
+		}
+		// Thus, alloc can't return nil, but it may do another split along the other dimension
+		// to get a node with the exact size (width, height).
+		node := alloc(n.child0, width, height)
+		if node == nil {
+			panic(fmt.Sprintf("packing: could not allocate the requested size (%d, %d) in the newly created child node (%d, %d)", width, height, n.child0.width, n.child0.height))
+		}
+		return node
 	}
 	if n.child0 == nil || n.child1 == nil {
 		panic("packing: both two children must not be nil at alloc")
@@ -201,10 +185,6 @@ func (p *Page) Alloc(width, height int) *Node {
 		panic("packing: width and height must > 0")
 	}
 
-	if !p.extendFor(width, height) {
-		return nil
-	}
-
 	if p.root == nil {
 		p.root = &Node{
 			width:  p.width,
@@ -217,7 +197,7 @@ func (p *Page) Alloc(width, height int) *Node {
 	if height < minSize {
 		height = minSize
 	}
-	return alloc(p.root, width, height)
+	return p.extendForAndAlloc(width, height)
 }
 
 func (p *Page) Free(node *Node) {
@@ -255,13 +235,13 @@ func walk(n *Node, f func(n *Node) error) error {
 	return nil
 }
 
-func (p *Page) extendFor(width, height int) bool {
-	if p.canAlloc(p.root, width, height) {
-		return true
+func (p *Page) extendForAndAlloc(width, height int) *Node {
+	if n := alloc(p.root, width, height); n != nil {
+		return n
 	}
 
 	if p.width >= p.maxSize && p.height >= p.maxSize {
-		return false
+		return nil
 	}
 
 	// (1, 0), (0, 1), (2, 0), (1, 1), (0, 2), (3, 0), (2, 1), (1, 2), (0, 3), ...
@@ -284,14 +264,14 @@ func (p *Page) extendFor(width, height int) bool {
 			}
 
 			rollback := p.extend(newWidth, newHeight)
-			if p.canAlloc(p.root, width, height) {
-				return true
+			if n := alloc(p.root, width, height); n != nil {
+				return n
 			}
 			rollback()
 
 			// If the allocation failed even with a maximized page, give up the allocation.
 			if newWidth >= p.maxSize && newHeight >= p.maxSize {
-				return false
+				return nil
 			}
 		}
 	}