From c9cf8c78590273b056ebb74917d12856bba2c2f0 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Sun, 2 Feb 2025 15:38:22 +0100 Subject: [PATCH 1/2] Avoid allocating for each binding trigger --- data/binding/binding.go | 33 ++++++++++++++++++++++----------- data/binding/binding_test.go | 10 +++++----- data/binding/lists_test.go | 10 +++++----- data/binding/trees_test.go | 10 +++++----- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/data/binding/binding.go b/data/binding/binding.go index a0bd790e70..f50b8e575c 100644 --- a/data/binding/binding.go +++ b/data/binding/binding.go @@ -9,7 +9,6 @@ import ( "sync" "fyne.io/fyne/v2" - "fyne.io/fyne/v2/internal/async" ) var ( @@ -58,31 +57,43 @@ func (l *listener) DataChanged() { } type base struct { - listeners async.Map[DataListener, bool] + listenerLock sync.Mutex + listeners []DataListener lock sync.RWMutex } // AddListener allows a data listener to be informed of changes to this item. func (b *base) AddListener(l DataListener) { - b.listeners.Store(l, true) + b.listenerLock.Lock() + b.listeners = append(b.listeners, l) + b.listenerLock.Unlock() queueItem(l.DataChanged) } // RemoveListener should be called if the listener is no longer interested in being informed of data change events. func (b *base) RemoveListener(l DataListener) { - b.listeners.Delete(l) + b.listenerLock.Lock() + defer b.listenerLock.Unlock() + + for i, listener := range b.listeners { + if listener == l { + // Delete without preserving order: + lastIndex := len(b.listeners) - 1 + b.listeners[i] = b.listeners[lastIndex] + b.listeners[lastIndex] = nil + b.listeners = b.listeners[:lastIndex] + return + } + } + } func (b *base) trigger() { - var listeners []DataListener - b.listeners.Range(func(listener DataListener, _ bool) bool { - listeners = append(listeners, listener) - return true - }) - queueItem(func() { - for _, listen := range listeners { + b.listenerLock.Lock() + defer b.listenerLock.Unlock() + for _, listen := range b.listeners { listen.DataChanged() } }) diff --git a/data/binding/binding_test.go b/data/binding/binding_test.go index 9f5fbe2f00..f5073d09c6 100644 --- a/data/binding/binding_test.go +++ b/data/binding/binding_test.go @@ -12,14 +12,14 @@ type simpleItem struct { func TestBase_AddListener(t *testing.T) { data := &simpleItem{} - assert.Equal(t, 0, data.listeners.Len()) + assert.Equal(t, 0, len(data.listeners)) called := false fn := NewDataListener(func() { called = true }) data.AddListener(fn) - assert.Equal(t, 1, data.listeners.Len()) + assert.Equal(t, 1, len(data.listeners)) assert.True(t, called) } @@ -29,11 +29,11 @@ func TestBase_RemoveListener(t *testing.T) { called = true }) data := &simpleItem{} - data.listeners.Store(fn, true) + data.listeners = append(data.listeners, fn) - assert.Equal(t, 1, data.listeners.Len()) + assert.Equal(t, 1, len(data.listeners)) data.RemoveListener(fn) - assert.Equal(t, 0, data.listeners.Len()) + assert.Equal(t, 0, len(data.listeners)) data.trigger() assert.False(t, called) diff --git a/data/binding/lists_test.go b/data/binding/lists_test.go index ed3632100a..3b6a31c82c 100644 --- a/data/binding/lists_test.go +++ b/data/binding/lists_test.go @@ -12,14 +12,14 @@ type simpleList struct { func TestListBase_AddListener(t *testing.T) { data := &simpleList{} - assert.Equal(t, 0, data.listeners.Len()) + assert.Equal(t, 0, len(data.listeners)) called := false fn := NewDataListener(func() { called = true }) data.AddListener(fn) - assert.Equal(t, 1, data.listeners.Len()) + assert.Equal(t, 1, len(data.listeners)) data.trigger() assert.True(t, called) @@ -55,11 +55,11 @@ func TestListBase_RemoveListener(t *testing.T) { called = true }) data := &simpleList{} - data.listeners.Store(fn, true) + data.listeners = append(data.listeners, fn) - assert.Equal(t, 1, data.listeners.Len()) + assert.Equal(t, 1, len(data.listeners)) data.RemoveListener(fn) - assert.Equal(t, 0, data.listeners.Len()) + assert.Equal(t, 0, len(data.listeners)) data.trigger() assert.False(t, called) diff --git a/data/binding/trees_test.go b/data/binding/trees_test.go index 9106bc10f2..dcd2ad97bf 100644 --- a/data/binding/trees_test.go +++ b/data/binding/trees_test.go @@ -8,14 +8,14 @@ import ( func TestTreeBase_AddListener(t *testing.T) { data := newSimpleTree() - assert.Equal(t, 0, data.listeners.Len()) + assert.Equal(t, 0, len(data.listeners)) called := false fn := NewDataListener(func() { called = true }) data.AddListener(fn) - assert.Equal(t, 1, data.listeners.Len()) + assert.Equal(t, 1, len(data.listeners)) data.trigger() assert.True(t, called) @@ -52,11 +52,11 @@ func TestTreeBase_RemoveListener(t *testing.T) { called = true }) data := newSimpleTree() - data.listeners.Store(fn, true) + data.listeners = append(data.listeners, fn) - assert.Equal(t, 1, data.listeners.Len()) + assert.Equal(t, 1, len(data.listeners)) data.RemoveListener(fn) - assert.Equal(t, 0, data.listeners.Len()) + assert.Equal(t, 0, len(data.listeners)) data.trigger() assert.False(t, called) From 0d994345e9f002805f806f2d2648059f16298465 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Tue, 4 Feb 2025 21:19:26 +0100 Subject: [PATCH 2/2] Queue all listener changes onto main --- data/binding/binding.go | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/data/binding/binding.go b/data/binding/binding.go index f50b8e575c..5807b1043c 100644 --- a/data/binding/binding.go +++ b/data/binding/binding.go @@ -57,42 +57,37 @@ func (l *listener) DataChanged() { } type base struct { - listenerLock sync.Mutex - listeners []DataListener + listeners []DataListener lock sync.RWMutex } // AddListener allows a data listener to be informed of changes to this item. func (b *base) AddListener(l DataListener) { - b.listenerLock.Lock() - b.listeners = append(b.listeners, l) - b.listenerLock.Unlock() - queueItem(l.DataChanged) + queueItem(func() { + b.listeners = append(b.listeners, l) + l.DataChanged() + }) } // RemoveListener should be called if the listener is no longer interested in being informed of data change events. func (b *base) RemoveListener(l DataListener) { - b.listenerLock.Lock() - defer b.listenerLock.Unlock() - - for i, listener := range b.listeners { - if listener == l { - // Delete without preserving order: - lastIndex := len(b.listeners) - 1 - b.listeners[i] = b.listeners[lastIndex] - b.listeners[lastIndex] = nil - b.listeners = b.listeners[:lastIndex] - return + queueItem(func() { + for i, listener := range b.listeners { + if listener == l { + // Delete without preserving order: + lastIndex := len(b.listeners) - 1 + b.listeners[i] = b.listeners[lastIndex] + b.listeners[lastIndex] = nil + b.listeners = b.listeners[:lastIndex] + return + } } - } - + }) } func (b *base) trigger() { queueItem(func() { - b.listenerLock.Lock() - defer b.listenerLock.Unlock() for _, listen := range b.listeners { listen.DataChanged() }