Skip to content

Commit

Permalink
Merge pull request #4398 from dweymouth/list-reduce-allocs
Browse files Browse the repository at this point in the history 
Reduce allocations in list scroll updates
  • Loading branch information
@dweymouth
dweymouth authored Dec 16, 2023
2 parents c90d255 + c31211e commit 23057e2
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 56 deletions.
160 changes: 114 additions & 46 deletions widget/list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package widget
import (
"fmt"
"math"
"sort"
"sync"

"fyne.io/fyne/v2"
Expand Down Expand Up @@ -122,8 +123,10 @@ func (l *List) RefreshItem(id ListItemID) {
}
l.BaseWidget.Refresh()
lo := l.scroller.Content.(*fyne.Container).Layout.(*listLayout)
visible := lo.visible
if item, ok := visible[id]; ok {
lo.renderLock.RLock() // ensures we are not changing visible info in render code during the search
item, ok := lo.searchVisible(lo.visible, id)
lo.renderLock.RUnlock()
if ok {
lo.setupListItem(item, id, l.focused && l.currentFocus == id)
}
}
Expand Down Expand Up @@ -318,24 +321,25 @@ func (l *List) UnselectAll() {
}
}

func (l *List) visibleItemHeights(itemHeight float32, length int) (visible []float32, offY float32, minRow int) {
// fills l.visibleRowHeights and also returns offY and minRow
func (l *listLayout) calculateVisibleRowHeights(itemHeight float32, length int) (offY float32, minRow int) {
rowOffset := float32(0)
isVisible := false
visible = []float32{}
l.visibleRowHeights = l.visibleRowHeights[:0]

if l.scroller.Size().Height <= 0 {
if l.list.scroller.Size().Height <= 0 {
return
}

// theme.Padding is a slow call, so we cache it
padding := theme.Padding()

if len(l.itemHeights) == 0 {
if len(l.list.itemHeights) == 0 {
paddedItemHeight := itemHeight + padding

offY = float32(math.Floor(float64(l.offsetY/paddedItemHeight))) * paddedItemHeight
offY = float32(math.Floor(float64(l.list.offsetY/paddedItemHeight))) * paddedItemHeight
minRow = int(math.Floor(float64(offY / paddedItemHeight)))
maxRow := int(math.Ceil(float64((offY + l.scroller.Size().Height) / paddedItemHeight)))
maxRow := int(math.Ceil(float64((offY + l.list.scroller.Size().Height) / paddedItemHeight)))

if minRow > length-1 {
minRow = length - 1
Expand All @@ -349,33 +353,32 @@ func (l *List) visibleItemHeights(itemHeight float32, length int) (visible []flo
maxRow = length
}

visible = make([]float32, maxRow-minRow)
for i := 0; i < maxRow-minRow; i++ {
visible[i] = itemHeight
l.visibleRowHeights = append(l.visibleRowHeights, itemHeight)
}
return
}

for i := 0; i < length; i++ {
height := itemHeight
if h, ok := l.itemHeights[i]; ok {
if h, ok := l.list.itemHeights[i]; ok {
height = h
}

if rowOffset <= l.offsetY-height-padding {
if rowOffset <= l.list.offsetY-height-padding {
// before scroll
} else if rowOffset <= l.offsetY {
} else if rowOffset <= l.list.offsetY {
minRow = i
offY = rowOffset
isVisible = true
}
if rowOffset >= l.offsetY+l.scroller.Size().Height {
if rowOffset >= l.list.offsetY+l.list.scroller.Size().Height {
break
}

rowOffset += height + padding
if isVisible {
visible = append(visible, height)
l.visibleRowHeights = append(l.visibleRowHeights, height)
}
}
return
Expand Down Expand Up @@ -523,18 +526,29 @@ func (li *listItemRenderer) Refresh() {
// Declare conformity with Layout interface.
var _ fyne.Layout = (*listLayout)(nil)

type itemAndID struct {
item *listItem
id ListItemID
}

type listLayout struct {
list *List
separators []fyne.CanvasObject
children []fyne.CanvasObject

itemPool *syncPool
visible map[ListItemID]*listItem
renderLock sync.Mutex
itemPool syncPool
visible []itemAndID
slicePool sync.Pool // *[]itemAndID
visibleRowHeights []float32
renderLock sync.RWMutex
}

func newListLayout(list *List) fyne.Layout {
l := &listLayout{list: list, itemPool: &syncPool{}, visible: make(map[ListItemID]*listItem)}
l := &listLayout{list: list}
l.slicePool.New = func() interface{} {
s := make([]itemAndID, 0)
return &s
}
list.offsetUpdated = l.offsetUpdated
return l
}
Expand Down Expand Up @@ -636,25 +650,31 @@ func (l *listLayout) updateList(newOnly bool) {
fyne.LogError("Missing UpdateCell callback required for List", nil)
}

wasVisible := l.visible
// Keep pointer reference for copying slice header when returning to the pool
// https://blog.mike.norgate.xyz/unlocking-go-slice-performance-navigating-sync-pool-for-enhanced-efficiency-7cb63b0b453e
wasVisiblePtr := l.slicePool.Get().(*[]itemAndID)
wasVisible := (*wasVisiblePtr)[:0]
wasVisible = append(wasVisible, l.visible...)

l.list.propertyLock.Lock()
visibleRowHeights, offY, minRow := l.list.visibleItemHeights(l.list.itemMin.Height, length)
offY, minRow := l.calculateVisibleRowHeights(l.list.itemMin.Height, length)
l.list.propertyLock.Unlock()
if len(visibleRowHeights) == 0 && length > 0 { // we can't show anything until we have some dimensions
if len(l.visibleRowHeights) == 0 && length > 0 { // we can't show anything until we have some dimensions
l.renderLock.Unlock() // user code should not be locked
return
}

visible := make(map[ListItemID]*listItem, len(visibleRowHeights))
cells := make([]fyne.CanvasObject, len(visibleRowHeights))
oldVisibleLen := len(l.visible)
l.visible = l.visible[:0]
oldChildrenLen := len(l.children)
l.children = l.children[:0]

y := offY
for index, itemHeight := range visibleRowHeights {
for index, itemHeight := range l.visibleRowHeights {
row := index + minRow
size := fyne.NewSize(width, itemHeight)

c, ok := wasVisible[row]
c, ok := l.searchVisible(wasVisible, row)
if !ok {
c = l.getItem()
if c == nil {
Expand All @@ -667,45 +687,65 @@ func (l *listLayout) updateList(newOnly bool) {
c.Resize(size)

y += itemHeight + separatorThickness
visible[row] = c
cells[index] = c
l.visible = append(l.visible, itemAndID{id: row, item: c})
l.children = append(l.children, c)
}
l.nilOldSliceData(l.children, len(l.children), oldChildrenLen)
l.nilOldVisibleSliceData(l.visible, len(l.visible), oldVisibleLen)

l.visible = visible

for id, old := range wasVisible {
if _, ok := l.visible[id]; !ok {
l.itemPool.Release(old)
for _, wasVis := range wasVisible {
if _, ok := l.searchVisible(l.visible, wasVis.id); !ok {
l.itemPool.Release(wasVis.item)
}
}
l.children = cells

l.updateSeparators()

objects := l.children
objects = append(objects, l.separators...)
l.list.scroller.Content.(*fyne.Container).Objects = objects
c := l.list.scroller.Content.(*fyne.Container)
oldObjLen := len(c.Objects)
c.Objects = c.Objects[:0]
c.Objects = append(c.Objects, l.children...)
c.Objects = append(c.Objects, l.separators...)
l.nilOldSliceData(c.Objects, len(c.Objects), oldObjLen)

// make a local deep copy of l.visible since rest of this function is unlocked
// and cannot safely access l.visible
visiblePtr := l.slicePool.Get().(*[]itemAndID)
visible := (*visiblePtr)[:0]
visible = append(visible, l.visible...)
l.renderLock.Unlock() // user code should not be locked

if newOnly {
for row, obj := range visible {
if _, ok := wasVisible[row]; !ok {
l.setupListItem(obj, row, l.list.focused && l.list.currentFocus == row)
for _, vis := range visible {
if _, ok := l.searchVisible(wasVisible, vis.id); !ok {
l.setupListItem(vis.item, vis.id, l.list.focused && l.list.currentFocus == vis.id)
}
}
} else {
for row, obj := range visible {
l.setupListItem(obj, row, l.list.focused && l.list.currentFocus == row)
for _, vis := range visible {
l.setupListItem(vis.item, vis.id, l.list.focused && l.list.currentFocus == vis.id)
}
}

// nil out all references before returning slices to pool
for i := 0; i < len(wasVisible); i++ {
wasVisible[i].item = nil
}
for i := 0; i < len(visible); i++ {
visible[i].item = nil
}
*wasVisiblePtr = wasVisible // Copy the stack header over to the heap
*visiblePtr = visible
l.slicePool.Put(wasVisiblePtr)
l.slicePool.Put(visiblePtr)
}

func (l *listLayout) updateSeparators() {
if len(l.children) > 1 {
if len(l.separators) > len(l.children) {
l.separators = l.separators[:len(l.children)]
if lenChildren := len(l.children); lenChildren > 1 {
if lenSep := len(l.separators); lenSep > lenChildren {
l.separators = l.separators[:lenChildren]
} else {
for i := len(l.separators); i < len(l.children); i++ {
for i := lenSep; i < lenChildren; i++ {
l.separators = append(l.separators, NewSeparator())
}
}
Expand All @@ -724,3 +764,31 @@ func (l *listLayout) updateSeparators() {
l.separators[i].Show()
}
}

// invariant: visible is in ascending order of IDs
func (l *listLayout) searchVisible(visible []itemAndID, id ListItemID) (*listItem, bool) {
ln := len(visible)
idx := sort.Search(ln, func(i int) bool { return visible[i].id >= id })
if idx < ln && visible[idx].id == id {
return visible[idx].item, true
}
return nil, false
}

func (l *listLayout) nilOldSliceData(objs []fyne.CanvasObject, len, oldLen int) {
if oldLen > len {
objs = objs[:oldLen] // gain view into old data
for i := len; i < oldLen; i++ {
objs[i] = nil
}
}
}

func (l *listLayout) nilOldVisibleSliceData(objs []itemAndID, len, oldLen int) {
if oldLen > len {
objs = objs[:oldLen] // gain view into old data
for i := len; i < oldLen; i++ {
objs[i].item = nil
}
}
}
22 changes: 12 additions & 10 deletions widget/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,22 +275,24 @@ func TestList_Select(t *testing.T) {
assert.Equal(t, float32(0), list.offsetY)
list.Select(50)
assert.Equal(t, 988, int(list.offsetY))
visible := list.scroller.Content.(*fyne.Container).Layout.(*listLayout).visible
assert.Equal(t, visible[50].background.FillColor, theme.SelectionColor())
assert.True(t, visible[50].background.Visible())
lo := list.scroller.Content.(*fyne.Container).Layout.(*listLayout)
visible50, _ := lo.searchVisible(lo.visible, 50)
assert.Equal(t, visible50.background.FillColor, theme.SelectionColor())
assert.True(t, visible50.background.Visible())

list.Select(5)
assert.Equal(t, 195, int(list.offsetY))
visible = list.scroller.Content.(*fyne.Container).Layout.(*listLayout).visible
assert.Equal(t, visible[5].background.FillColor, theme.SelectionColor())
assert.True(t, visible[5].background.Visible())
visible5, _ := lo.searchVisible(lo.visible, 5)
assert.Equal(t, visible5.background.FillColor, theme.SelectionColor())
assert.True(t, visible5.background.Visible())

list.Select(6)
assert.Equal(t, 195, int(list.offsetY))
visible = list.scroller.Content.(*fyne.Container).Layout.(*listLayout).visible
assert.False(t, visible[5].background.Visible())
assert.Equal(t, visible[6].background.FillColor, theme.SelectionColor())
assert.True(t, visible[6].background.Visible())
visible5, _ = lo.searchVisible(lo.visible, 5)
visible6, _ := lo.searchVisible(lo.visible, 6)
assert.False(t, visible5.background.Visible())
assert.Equal(t, visible6.background.FillColor, theme.SelectionColor())
assert.True(t, visible6.background.Visible())
}

func TestList_Unselect(t *testing.T) {
Expand Down

0 comments on commit 23057e2

Please sign in to comment.