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
This commit is contained in:
divVerent 2023-04-08 06:03:16 -07:00 committed by GitHub
parent aeca79494f
commit 574925cf7a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -107,32 +107,6 @@ func square(width, height int) float64 {
return float64(height) / float64(width) 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 { func alloc(n *Node, width, height int) *Node {
if n.width < width || n.height < height { if n.width < width || n.height < height {
return nil return nil
@ -178,7 +152,17 @@ func alloc(n *Node, width, height int) *Node {
parent: n, 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 { if n.child0 == nil || n.child1 == nil {
panic("packing: both two children must not be nil at alloc") 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") panic("packing: width and height must > 0")
} }
if !p.extendFor(width, height) {
return nil
}
if p.root == nil { if p.root == nil {
p.root = &Node{ p.root = &Node{
width: p.width, width: p.width,
@ -217,7 +197,7 @@ func (p *Page) Alloc(width, height int) *Node {
if height < minSize { if height < minSize {
height = minSize height = minSize
} }
return alloc(p.root, width, height) return p.extendForAndAlloc(width, height)
} }
func (p *Page) Free(node *Node) { func (p *Page) Free(node *Node) {
@ -255,13 +235,13 @@ func walk(n *Node, f func(n *Node) error) error {
return nil return nil
} }
func (p *Page) extendFor(width, height int) bool { func (p *Page) extendForAndAlloc(width, height int) *Node {
if p.canAlloc(p.root, width, height) { if n := alloc(p.root, width, height); n != nil {
return true return n
} }
if p.width >= p.maxSize && p.height >= p.maxSize { 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), ... // (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) rollback := p.extend(newWidth, newHeight)
if p.canAlloc(p.root, width, height) { if n := alloc(p.root, width, height); n != nil {
return true return n
} }
rollback() rollback()
// If the allocation failed even with a maximized page, give up the allocation. // If the allocation failed even with a maximized page, give up the allocation.
if newWidth >= p.maxSize && newHeight >= p.maxSize { if newWidth >= p.maxSize && newHeight >= p.maxSize {
return false return nil
} }
} }
} }