From e9ec0d1053caed5ec8c54bc6480cbafe6ed329bb Mon Sep 17 00:00:00 2001 From: nullst Date: Thu, 16 Sep 2021 22:16:38 +0300 Subject: [PATCH 1/9] Make fireChange() public --- internal/preferences.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/preferences.go b/internal/preferences.go index b12387858c..749807b036 100644 --- a/internal/preferences.go +++ b/internal/preferences.go @@ -41,7 +41,7 @@ func (p *InMemoryPreferences) WriteValues(fn func(map[string]interface{})) { fn(p.values) p.lock.Unlock() - p.fireChange() + p.FireChange() } func (p *InMemoryPreferences) set(key string, value interface{}) { @@ -50,7 +50,7 @@ func (p *InMemoryPreferences) set(key string, value interface{}) { p.values[key] = value p.lock.Unlock() - p.fireChange() + p.FireChange() } func (p *InMemoryPreferences) get(key string) (interface{}, bool) { @@ -68,7 +68,7 @@ func (p *InMemoryPreferences) remove(key string) { delete(p.values, key) } -func (p *InMemoryPreferences) fireChange() { +func (p *InMemoryPreferences) FireChange() { p.lock.RLock() defer p.lock.RUnlock() From 8cfce11a9810323e0a952d9cab08fc4a0742c17e Mon Sep 17 00:00:00 2001 From: nullst Date: Thu, 16 Sep 2021 22:18:55 +0300 Subject: [PATCH 2/9] Fix #2449 by retrying ignored changes --- app/preferences.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/preferences.go b/app/preferences.go index 44fe3f3a3a..2a815a299b 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -16,6 +16,7 @@ type preferences struct { prefLock sync.RWMutex ignoreChange bool + numIgnoredChanges int app *fyneApp } @@ -28,7 +29,13 @@ func (p *preferences) resetIgnore() { time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer. p.prefLock.Lock() p.ignoreChange = false + changes := p.numIgnoredChanges + p.numIgnoredChanges = 0 p.prefLock.Unlock() + + if changes > 0 { + p.InMemoryPreferences.FireChange() + } }() } @@ -112,9 +119,12 @@ func newPreferences(app *fyneApp) *preferences { } p.AddChangeListener(func() { - p.prefLock.RLock() + p.prefLock.Lock() shouldIgnoreChange := p.ignoreChange - p.prefLock.RUnlock() + if shouldIgnoreChange { + p.numIgnoredChanges++ + } + p.prefLock.Unlock() if shouldIgnoreChange { // callback after loading, no need to save return } From 45cef2522c2ddb2f567d4d51a3473087b3fed112 Mon Sep 17 00:00:00 2001 From: Dmitrii Pirozhkov Date: Sat, 18 Sep 2021 21:01:38 +0300 Subject: [PATCH 3/9] Formatting fix --- app/preferences.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/preferences.go b/app/preferences.go index 2a815a299b..f232cb507b 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -14,8 +14,8 @@ import ( type preferences struct { *internal.InMemoryPreferences - prefLock sync.RWMutex - ignoreChange bool + prefLock sync.RWMutex + ignoreChange bool numIgnoredChanges int app *fyneApp From c0a1ecd005a400729d4750505b570b8548f7aca7 Mon Sep 17 00:00:00 2001 From: Dmitrii Pirozhkov Date: Sat, 18 Sep 2021 21:23:28 +0300 Subject: [PATCH 4/9] Add a comment to exported function FireChange --- internal/preferences.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/preferences.go b/internal/preferences.go index 749807b036..a61ab0d768 100644 --- a/internal/preferences.go +++ b/internal/preferences.go @@ -68,6 +68,7 @@ func (p *InMemoryPreferences) remove(key string) { delete(p.values, key) } +// FireChange causes InMemoryPreferences to activate all its .changeListeners func (p *InMemoryPreferences) FireChange() { p.lock.RLock() defer p.lock.RUnlock() From 137ccdd81e22879db3ba65c0458de3285ee59646 Mon Sep 17 00:00:00 2001 From: Dmitrii Pirozhkov <7746216+nullst@users.noreply.github.com> Date: Fri, 24 Sep 2021 11:57:19 +0300 Subject: [PATCH 5/9] Avoid tautological preferences update after loading file --- app/preferences.go | 37 ++++++++++++++++++++++++------------- app/preferences_other.go | 2 +- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/app/preferences.go b/app/preferences.go index f232cb507b..95c29d2669 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -14,9 +14,10 @@ import ( type preferences struct { *internal.InMemoryPreferences - prefLock sync.RWMutex - ignoreChange bool - numIgnoredChanges int + prefLock sync.RWMutex + loadingInProgress bool + suspendChange bool + numSuspendedChanges int app *fyneApp } @@ -24,13 +25,13 @@ type preferences struct { // Declare conformity with Preferences interface var _ fyne.Preferences = (*preferences)(nil) -func (p *preferences) resetIgnore() { +func (p *preferences) resetSuspend() { go func() { time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer. p.prefLock.Lock() - p.ignoreChange = false - changes := p.numIgnoredChanges - p.numIgnoredChanges = 0 + p.suspendChange = false + changes := p.numSuspendedChanges + p.numSuspendedChanges = 0 p.prefLock.Unlock() if changes > 0 { @@ -45,9 +46,9 @@ func (p *preferences) save() error { func (p *preferences) saveToFile(path string) error { p.prefLock.Lock() - p.ignoreChange = true + p.suspendChange = true p.prefLock.Unlock() - defer p.resetIgnore() + defer p.resetSuspend() err := os.MkdirAll(filepath.Dir(path), 0700) if err != nil { // this is not an exists error according to docs return err @@ -102,9 +103,18 @@ func (p *preferences) loadFromFile(path string) (err error) { }() decode := json.NewDecoder(file) + p.prefLock.Lock() + p.loadingInProgress = true + p.prefLock.Unlock() + p.InMemoryPreferences.WriteValues(func(values map[string]interface{}) { err = decode.Decode(&values) }) + + p.prefLock.Lock() + p.loadingInProgress = false + p.prefLock.Unlock() + return err } @@ -120,12 +130,13 @@ func newPreferences(app *fyneApp) *preferences { p.AddChangeListener(func() { p.prefLock.Lock() - shouldIgnoreChange := p.ignoreChange - if shouldIgnoreChange { - p.numIgnoredChanges++ + shouldIgnoreChange := p.suspendChange || p.loadingInProgress + if p.suspendChange { + p.numSuspendedChanges++ } p.prefLock.Unlock() - if shouldIgnoreChange { // callback after loading, no need to save + + if shouldIgnoreChange { // callback after loading file, or too many updates in a row return } diff --git a/app/preferences_other.go b/app/preferences_other.go index bf45cc281a..0cbc481343 100644 --- a/app/preferences_other.go +++ b/app/preferences_other.go @@ -17,7 +17,7 @@ func (a *fyneApp) storageRoot() string { func (p *preferences) watch() { watchFile(p.storagePath(), func() { p.prefLock.RLock() - shouldIgnoreChange := p.ignoreChange + shouldIgnoreChange := p.suspendChange p.prefLock.RUnlock() if shouldIgnoreChange { return From 3ebbfec61b17d9556643c782fd8cdd24b61d885b Mon Sep 17 00:00:00 2001 From: Dmitrii Pirozhkov <7746216+nullst@users.noreply.github.com> Date: Fri, 24 Sep 2021 11:57:19 +0300 Subject: [PATCH 6/9] Comments about the changes suspension/suppression mechanism --- app/preferences.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/preferences.go b/app/preferences.go index 95c29d2669..f13dabcd13 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -14,8 +14,16 @@ import ( type preferences struct { *internal.InMemoryPreferences - prefLock sync.RWMutex - loadingInProgress bool + prefLock sync.RWMutex + // Normally, any update of preferences is immediately written to disk, + // but the initial operation of loading the preferences file performs many separate + // changes. To avoid "load->changes!->write" pattern (rewriting preferences file + // after each loading), saving file to disk is disabled + // during the loading progress. Access guarded by prefLock. + loadingInProgress bool + // If an application changes its preferences 1000 times per second, we don't want to + // rewrite the preferences file after every update. Instead, a time-out mechanism is + // implemented in resetSuspend(), limiting the number of file operations per second. suspendChange bool numSuspendedChanges int From cc816b7726949b6dca5d1d19655656260b00f9b3 Mon Sep 17 00:00:00 2001 From: Dmitrii Pirozhkov <7746216+nullst@users.noreply.github.com> Date: Sun, 26 Sep 2021 13:17:58 +0300 Subject: [PATCH 7/9] Renamed .ignoreChange variable to better reflect the role it plays --- app/preferences.go | 26 +++++++++----------------- app/preferences_other.go | 2 +- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/app/preferences.go b/app/preferences.go index f13dabcd13..cf89089e94 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -14,17 +14,9 @@ import ( type preferences struct { *internal.InMemoryPreferences - prefLock sync.RWMutex - // Normally, any update of preferences is immediately written to disk, - // but the initial operation of loading the preferences file performs many separate - // changes. To avoid "load->changes!->write" pattern (rewriting preferences file - // after each loading), saving file to disk is disabled - // during the loading progress. Access guarded by prefLock. - loadingInProgress bool - // If an application changes its preferences 1000 times per second, we don't want to - // rewrite the preferences file after every update. Instead, a time-out mechanism is - // implemented in resetSuspend(), limiting the number of file operations per second. - suspendChange bool + prefLock sync.RWMutex + loadingInProgress bool + savedRecently bool numSuspendedChanges int app *fyneApp @@ -33,11 +25,11 @@ type preferences struct { // Declare conformity with Preferences interface var _ fyne.Preferences = (*preferences)(nil) -func (p *preferences) resetSuspend() { +func (p *preferences) resetSavedRecently() { go func() { time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer. p.prefLock.Lock() - p.suspendChange = false + p.savedRecently = false changes := p.numSuspendedChanges p.numSuspendedChanges = 0 p.prefLock.Unlock() @@ -54,9 +46,9 @@ func (p *preferences) save() error { func (p *preferences) saveToFile(path string) error { p.prefLock.Lock() - p.suspendChange = true + p.savedRecently = true p.prefLock.Unlock() - defer p.resetSuspend() + defer p.resetSavedRecently() err := os.MkdirAll(filepath.Dir(path), 0700) if err != nil { // this is not an exists error according to docs return err @@ -138,8 +130,8 @@ func newPreferences(app *fyneApp) *preferences { p.AddChangeListener(func() { p.prefLock.Lock() - shouldIgnoreChange := p.suspendChange || p.loadingInProgress - if p.suspendChange { + shouldIgnoreChange := p.savedRecently || p.loadingInProgress + if p.savedRecently && !p.loadingInProgress { p.numSuspendedChanges++ } p.prefLock.Unlock() diff --git a/app/preferences_other.go b/app/preferences_other.go index 0cbc481343..1175cefdd0 100644 --- a/app/preferences_other.go +++ b/app/preferences_other.go @@ -17,7 +17,7 @@ func (a *fyneApp) storageRoot() string { func (p *preferences) watch() { watchFile(p.storagePath(), func() { p.prefLock.RLock() - shouldIgnoreChange := p.suspendChange + shouldIgnoreChange := p.savedRecently p.prefLock.RUnlock() if shouldIgnoreChange { return From 267f052d37813c857ece2ba6ebb85b8f312f8e7b Mon Sep 17 00:00:00 2001 From: Dmitrii Pirozhkov <7746216+nullst@users.noreply.github.com> Date: Sun, 26 Sep 2021 14:52:01 +0300 Subject: [PATCH 8/9] Call save() directly instead of going through change listeners --- app/preferences.go | 12 ++++++------ internal/preferences.go | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/preferences.go b/app/preferences.go index cf89089e94..c41617797c 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -17,7 +17,7 @@ type preferences struct { prefLock sync.RWMutex loadingInProgress bool savedRecently bool - numSuspendedChanges int + changedDuringSaving bool app *fyneApp } @@ -30,12 +30,12 @@ func (p *preferences) resetSavedRecently() { time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer. p.prefLock.Lock() p.savedRecently = false - changes := p.numSuspendedChanges - p.numSuspendedChanges = 0 + changedDuringSaving := p.changedDuringSaving + p.changedDuringSaving = false p.prefLock.Unlock() - if changes > 0 { - p.InMemoryPreferences.FireChange() + if changedDuringSaving { + p.save() } }() } @@ -132,7 +132,7 @@ func newPreferences(app *fyneApp) *preferences { p.prefLock.Lock() shouldIgnoreChange := p.savedRecently || p.loadingInProgress if p.savedRecently && !p.loadingInProgress { - p.numSuspendedChanges++ + p.changedDuringSaving = true } p.prefLock.Unlock() diff --git a/internal/preferences.go b/internal/preferences.go index a61ab0d768..d6777711d8 100644 --- a/internal/preferences.go +++ b/internal/preferences.go @@ -41,7 +41,7 @@ func (p *InMemoryPreferences) WriteValues(fn func(map[string]interface{})) { fn(p.values) p.lock.Unlock() - p.FireChange() + p.fireChange() } func (p *InMemoryPreferences) set(key string, value interface{}) { @@ -50,7 +50,7 @@ func (p *InMemoryPreferences) set(key string, value interface{}) { p.values[key] = value p.lock.Unlock() - p.FireChange() + p.fireChange() } func (p *InMemoryPreferences) get(key string) (interface{}, bool) { @@ -69,7 +69,7 @@ func (p *InMemoryPreferences) remove(key string) { } // FireChange causes InMemoryPreferences to activate all its .changeListeners -func (p *InMemoryPreferences) FireChange() { +func (p *InMemoryPreferences) fireChange() { p.lock.RLock() defer p.lock.RUnlock() From df48b374121cb2aad853071377e3aaafa6a84ed3 Mon Sep 17 00:00:00 2001 From: Dmitrii Pirozhkov <7746216+nullst@users.noreply.github.com> Date: Mon, 27 Sep 2021 22:55:01 +0300 Subject: [PATCH 9/9] Restore internal/preferences.go to the original state --- internal/preferences.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/preferences.go b/internal/preferences.go index d6777711d8..b12387858c 100644 --- a/internal/preferences.go +++ b/internal/preferences.go @@ -68,7 +68,6 @@ func (p *InMemoryPreferences) remove(key string) { delete(p.values, key) } -// FireChange causes InMemoryPreferences to activate all its .changeListeners func (p *InMemoryPreferences) fireChange() { p.lock.RLock() defer p.lock.RUnlock()