diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10e991b..51b2cc2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -693,7 +693,7 @@ jobs: # in somebody else's file. run: | set -euo pipefail - watched='internal/format/registry.go internal/damage/damage.go cmd/tfg/main.go internal/gui/window/run.go internal/audit/parallel.go internal/engine/parallel.go go.mod' + watched='internal/format/registry.go internal/damage/damage.go cmd/tfg/main.go internal/gui/window/run.go internal/gui/run_cgo.go internal/gui/window/tidy.go internal/audit/parallel.go internal/engine/parallel.go go.mod' # On a pull request there is no "before" - the field belongs to a push # - so this asked for something empty and every pull request answered # "touched". That quietly undid the decision of 2026-08-20, because diff --git a/CHANGELOG.md b/CHANGELOG.md index b944048..8038b04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -494,6 +494,15 @@ because it turns other people's test suites red. again for each one. Both are now done once, and the window uses less memory while you type. +- **The window gives memory back once it has been left alone.** After a + spell of work it used to keep about 200 MB for as long as it stood idle. + About a minute and a half after the last setting changed or the last press + of Preview or Generate, the window now gives back what it no longer uses - + 215 MB down to 129 MB in one measurement. Scrolling does not count, and + while Preview or Generate is running it waits. Nothing on the screen moves + when it does. A minimised window gives back less, because the toolkit only + lets go while it draws. + - **Changing a setting of a preset is about four times faster.** With `upload-validation`, changing one of its settings held the window for about 0.15 seconds, and so did every key typed on `Several batches` built diff --git a/internal/guard/concurrency_test.go b/internal/guard/concurrency_test.go index cbbda54..8c847ae 100644 --- a/internal/guard/concurrency_test.go +++ b/internal/guard/concurrency_test.go @@ -44,6 +44,18 @@ var mayBeConcurrent = map[string]string{ // invariant G7 exists to hold. Added 2026-08-05 with the first generate // window, and the owner was told. "internal/gui/window/run.go": "the run happens beside the window, and closing the window waits for it", + // The window's clock. time.AfterFunc calls back on a goroutine of its + // own, which is how a delayed piece of work - the busy face, the wait for + // quiet - waits without holding the window, and the callback hands its + // work straight to the toolkit's thread. Declared 2026-09-24, when the scan + // learned to see a timer's callback: it had been here since the busy face + // got its delay, unlisted. + "internal/gui/run_cgo.go": "the window's clock calls back on a timer's goroutine and hands the work to the toolkit's thread", + // Giving memory back beside the window rather than on its thread: one of + // ten FreeOSMemory calls measured on the window's thread took 2491 ms. + // The goroutine touches nothing of ours. Added 2026-09-24, and THE OWNER + // DECIDED IT - docs/GUI-MEMORY-2026-09-23.md section 4j. + "internal/gui/window/tidy.go": "memory is given back beside the window, so the window never waits on it", // Hashing the files a manifest claims is the work verify and cleanup are // made of, and it is embarrassingly parallel. Added 2026-09-05 and the // owner decided it: O116 turned the same idea down on 2026-08-20 on a @@ -95,7 +107,7 @@ func isCancellation(n ast.Node) bool { } func TestConcurrencyStaysWhereItWasPutOnPurpose(t *testing.T) { - var found []string + var found, idle []string for _, p := range packages(t) { for _, path := range p.files { @@ -104,50 +116,43 @@ func TestConcurrencyStaysWhereItWasPutOnPurpose(t *testing.T) { rel = path } rel = filepath.ToSlash(rel) - if _, allowed := mayBeConcurrent[rel]; allowed { - continue - } fset := token.NewFileSet() file, err := parser.ParseFile(fset, path, nil, 0) if err != nil { t.Fatalf("parsing %s: %v", path, err) } - - ast.Inspect(file, func(n ast.Node) bool { - if n == nil { - return false - } - what := "" - switch node := n.(type) { - case *ast.GoStmt: - what = "starts a goroutine" - case *ast.ChanType: - what = "declares a channel" - case *ast.SendStmt: - what = "sends on a channel" - case *ast.SelectStmt: - // A select waiting only on cancellation is how every long - // loop here notices Ctrl+C. - if onlyCancellation(node) { - return true - } - what = "selects over channels" - case *ast.SelectorExpr: - if id, ok := node.X.(*ast.Ident); ok && (id.Name == "sync" || id.Name == "atomic") { - what = "uses " + id.Name + "." + node.Sel.Name - } - } - if what == "" { - return true + here := concurrencyIn(fset, file) + if _, allowed := mayBeConcurrent[rel]; allowed { + if len(here) == 0 { + idle = append(idle, rel) } - found = append(found, fmt.Sprintf("%s:%d %s", - rel, fset.Position(n.Pos()).Line, what)) - return true - }) + continue + } + for _, one := range here { + found = append(found, rel+":"+one) + } } } + // The other direction, since 2026-09-24. A file declared here that runs + // nothing beside anything is a declaration nobody can check - and it is + // how a decision gets undone quietly: the window gives memory back on a + // goroutine BECAUSE a call on its own thread took 2491 ms once, and taking + // the go statement away would leave the file declared and every guard + // green. A file the build leaves out on this machine is not asked. A file + // that is gone is, because the walk above never reaches it and a deleted + // file would otherwise keep its declaration and its place on the race + // detector's list - an outside review of the pull request named it. + idle = append(idle, declaredWithoutAFile(repoRoot(t), mayBeConcurrent)...) + if len(idle) > 0 { + sort.Strings(idle) + t.Errorf("declared as concurrent and running nothing beside anything:\n %s\n\n"+ + "Either the concurrency moved and the declaration should go with it, or it was taken out and\n"+ + "the reason it was put there - written beside the declaration - no longer holds.", + strings.Join(idle, "\n ")) + } + if len(found) > 0 { sort.Strings(found) t.Errorf("concurrency turned up in %d place(s) outside the files that declare it:\n %s\n\n"+ @@ -157,6 +162,127 @@ func TestConcurrencyStaysWhereItWasPutOnPurpose(t *testing.T) { } } +// declaredWithoutAFile is every declared path with no file under root, each +// with what the system said about it. +func declaredWithoutAFile(root string, declared map[string]string) []string { + var gone []string + for rel := range declared { + if _, err := os.Stat(filepath.Join(root, filepath.FromSlash(rel))); err != nil { + gone = append(gone, rel+" ("+err.Error()+")") + } + } + return gone +} + +// A declaration whose file is gone is reported, and one whose file is there +// is not - asked of a folder made here, because every file the tree declares +// exists and a check that stopped looking would stay green on it. +func TestADeclarationWhoseFileIsGoneIsReported(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "a"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "a", "here.go"), []byte("package a\n"), 0o644); err != nil { + t.Fatal(err) + } + got := declaredWithoutAFile(root, map[string]string{"a/here.go": "", "a/gone.go": ""}) + if len(got) != 1 || !strings.HasPrefix(got[0], "a/gone.go ") { + t.Errorf("declared a/here.go, which exists, and a/gone.go, which does not - reported %q, expected a/gone.go alone", got) + } +} + +// concurrencyIn is every place in one file that runs something beside the +// code around it, as "line what". +// +// A timer's callback since 2026-09-24. time.AfterFunc runs the function it is +// handed on a goroutine of its own, and this scan saw only the go statement - +// so the window's clock, which has handed every delayed piece of work to such +// a goroutine since the busy face got a delay, stood outside the list without +// a word. Found while the window was taught to give memory back +// (docs/GUI-MEMORY-2026-09-23.md section 4j), decided by the owner that day. +func concurrencyIn(fset *token.FileSet, file *ast.File) []string { + var found []string + ast.Inspect(file, func(n ast.Node) bool { + if n == nil { + return false + } + what := "" + switch node := n.(type) { + case *ast.GoStmt: + what = "starts a goroutine" + case *ast.ChanType: + what = "declares a channel" + case *ast.SendStmt: + what = "sends on a channel" + case *ast.SelectStmt: + // A select waiting only on cancellation is how every long + // loop here notices Ctrl+C. + if onlyCancellation(node) { + return true + } + what = "selects over channels" + case *ast.SelectorExpr: + what = concurrentName(node) + } + if what == "" { + return true + } + found = append(found, fmt.Sprintf("%d %s", fset.Position(n.Pos()).Line, what)) + return true + }) + return found +} + +// concurrentName is what a package-qualified name says about running beside +// something, or nothing: a lock or an atomic, or a timer that calls back on +// a goroutine of its own. +func concurrentName(node *ast.SelectorExpr) string { + id, ok := node.X.(*ast.Ident) + if !ok { + return "" + } + switch { + case id.Name == "sync" || id.Name == "atomic": + return "uses " + id.Name + "." + node.Sel.Name + case id.Name == "time" && node.Sel.Name == "AfterFunc": + return "hands a callback to a timer's goroutine (time.AfterFunc)" + } + return "" +} + +// The scan sees each kind of concurrency it names, asked of source written +// here - because the tree may hold none of a kind outside the declared files, +// and a scan that stopped seeing it would then stay green. +func TestTheConcurrencyScanSeesEachKindItLooksFor(t *testing.T) { + for _, c := range []struct{ name, body string }{ + {"a goroutine", `go f()`}, + {"a channel", `var c chan int; _ = c`}, + {"a lock", `var m sync.Mutex; m.Lock()`}, + {"a timer's callback", `time.AfterFunc(0, f)`}, + } { + src := "package x\nfunc f() {}\nfunc g() {\n" + c.body + "\n}\n" + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "x.go", src, 0) + if err != nil { + t.Fatalf("%s: %v", c.name, err) + } + if len(concurrencyIn(fset, file)) == 0 { + t.Errorf("the scan does not see %s: %s", c.name, c.body) + } + } + // And a name from package time that runs nothing beside anything is not + // reported, so the timer rule is about the callback and not the package. + src := "package x\nfunc g() {\n_ = time.Now()\n}\n" + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "x.go", src, 0) + if err != nil { + t.Fatal(err) + } + if got := concurrencyIn(fset, file); len(got) != 0 { + t.Errorf("the scan calls a plain duration concurrency: %v", got) + } +} + // onlyCancellation reports whether every case of a select is either a receive // from something named Done or the default branch. func onlyCancellation(s *ast.SelectStmt) bool { diff --git a/internal/guard/settleonce_test.go b/internal/guard/settleonce_test.go index cd9432a..e1974fe 100644 --- a/internal/guard/settleonce_test.go +++ b/internal/guard/settleonce_test.go @@ -57,7 +57,7 @@ func TestOneChangeOfABoxReadsTheFormOnce(t *testing.T) { switch { case host.settles == 0: t.Fatalf("typing %q into %q on the %s screen told the host of no reading of the form - "+ - "either the box reports no change or the screen reads its form without countedSettle, and this guard sees nothing either way", + "either the box reports no change or the screen reads its form without watchedSettle, and this guard sees nothing either way", v, b.at, b.screen) case host.settles > 1: t.Errorf("typing %q into %q on the %s screen read the form %d times, expected once - "+ diff --git a/internal/guard/tidy_test.go b/internal/guard/tidy_test.go new file mode 100644 index 0000000..7e14cbe --- /dev/null +++ b/internal/guard/tidy_test.go @@ -0,0 +1,161 @@ +package guard + +import ( + "testing" + "time" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/window" +) + +// The window gives memory back once it has been left alone, and only then. +// +// After a spell of work the process kept 189-206 MB for as long as the window +// stood idle: the toolkit lets go of the renderers of what left the screen only +// while drawing a frame, and an idle window draws none. The window now waits +// out a quiet, asks for one frame, and gives the memory back a little later - +// 252-263 MB to 117-119 MB in the real window (docs/GUI-MEMORY-2026-09-23.md +// section 4j). +// +// What this can hold is the timing and the order. That the memory really comes +// back is a property of the toolkit and of Go, and only the real window shows +// it - tools/probes/guilag -quiet -nudge. +// +// The waits are asked about as well as the order. The first has to outlast the +// minute the toolkit keeps a renderer, or the frame comes while they are still +// valid and nothing is let go. The second has to outlast the ten seconds between +// two of the toolkit's cleans, or the memory goes back before the renderers do. +func TestTheWindowGivesMemoryBackOnceItHasBeenLeftAlone(t *testing.T) { + host := newFakeHost(t) + window.Open(host) + host.quiet = &quietClock{} + content := tabNamed(t, host.content, text.TabOneTarget()) + + fill(t, content, text.FieldSeed(), "5") + quiet := host.quiet.waiting() + if len(quiet) != 1 { + t.Fatalf("typing into a box left %d wait(s) for quiet, expected one - the window cannot tell when it has been left alone", len(quiet)) + } + if quiet[0].after <= time.Minute { + t.Errorf("the window waits %v of quiet, and the toolkit keeps a renderer for a minute - the frame would come while they are still valid", quiet[0].after) + } + + host.quiet.fireAll() + if host.releases != 0 { + t.Fatal("memory was given back at the end of the quiet, before the frame that lets the renderers go") + } + frame := host.quiet.waiting() + if len(frame) != 1 { + t.Fatalf("after the quiet the window waits for %d thing(s), expected the one release", len(frame)) + } + if frame[0].after <= 10*time.Second { + t.Errorf("memory goes back %v after the frame, and the toolkit cleans at most once in ten seconds - it would sometimes go back before the renderers", frame[0].after) + } + + host.quiet.fireAll() + if host.releases != 1 { + t.Fatalf("the quiet and the frame were waited out and memory was given back %d time(s), expected once", host.releases) + } + host.quiet.fireAll() + if host.releases != 1 || len(host.quiet.waiting()) != 0 { + t.Errorf("with nothing done since, memory was given back %d time(s) and %d wait(s) are left - it has to happen once per quiet, not keep going", + host.releases, len(host.quiet.waiting())) + } +} + +// Anything done during the wait starts the quiet over, and takes back a +// release already waiting. +func TestSomethingDoneDuringTheWaitStartsTheQuietOver(t *testing.T) { + host := newFakeHost(t) + window.Open(host) + host.quiet = &quietClock{} + content := tabNamed(t, host.content, text.TabOneTarget()) + + fill(t, content, text.FieldSeed(), "5") + host.quiet.fireAll() // the quiet waited out, so the release is waiting + if len(host.quiet.waiting()) != 1 { + t.Fatal("after the quiet no release is waiting, so this guard is not in the state it asks about") + } + + fill(t, content, text.FieldSeed(), "6") + host.quiet.fireAll() // the new quiet, which has to come first + if host.releases != 0 { + t.Error("something was typed while the release was waiting and memory was given back anyway, in the middle of somebody working") + } + host.quiet.fireAll() + if host.releases != 1 { + t.Errorf("after the new quiet and its frame memory was given back %d time(s), expected once", host.releases) + } +} + +// A wait called off on its way does nothing when it arrives. +// +// Calling the clock off is not enough, and an outside review of the pull +// request named why: the real window's clock hands what it fires to the +// toolkit's queue (desktop.Later, time.AfterFunc then fyne.Do), and a call +// already queued still runs. If somebody types in that gap, the quiet they +// broke arrived anyway - it gave memory back twelve seconds later, in the +// middle of their work, and took the place of the new quiet's handle, so +// closing the window could no longer call that one off. The same gap as the +// busy face's (TestAFaceAskedForByEarlierWorkNeverDressesLaterWork). +// +// Played out with the held clock: the first quiet is kept aside, a second key +// calls it off, and then it is fired as if the queue had just got round to it. +func TestAWaitCalledOffOnItsWayDoesNothingWhenItArrives(t *testing.T) { + host := newFakeHost(t) + window.Open(host) + host.quiet = &quietClock{} + content := tabNamed(t, host.content, text.TabOneTarget()) + + fill(t, content, text.FieldSeed(), "5") + first := host.quiet.waiting() + if len(first) != 1 { + t.Fatalf("typing into a box left %d wait(s) for quiet, expected one to keep aside", len(first)) + } + fill(t, content, text.FieldSeed(), "6") + if !first[0].calledOff { + t.Fatal("a second key did not call the first quiet off, so there is no call on its way to ask about") + } + + first[0].then() // the first quiet arriving from the queue after all + if got := len(host.quiet.waiting()); got != 1 { + t.Errorf("a quiet called off on its way arrived and %d wait(s) are left, expected the one quiet of the second key", got) + } + host.quiet.fireAll() // the second key's quiet + if host.releases != 0 { + t.Error("memory was given back after a quiet somebody had broken, in the middle of their work") + } + host.quiet.fireAll() + if host.releases != 1 { + t.Errorf("after the second key's quiet and its frame memory was given back %d time(s), expected once", host.releases) + } +} + +// No memory goes back while work owns the window - the run is what is using it. +func TestNoMemoryIsGivenBackWhileWorkIsGoing(t *testing.T) { + host, content, hold := heldScreen(t) + host.quiet = &quietClock{} + fill(t, content, text.FieldOutputDir(), t.TempDir()) + press(t, content, text.ButtonGenerate()) + + hold.look(func() { + if len(host.quiet.waiting()) == 0 { + t.Fatal("the run started and no wait for quiet was asked for, so this guard asks about nothing") + } + host.quiet.fireAll() // the quiet ends during the run + host.quiet.fireAll() + if host.releases != 0 { + t.Error("memory was given back while a run owned the window") + } + if len(host.quiet.waiting()) == 0 { + t.Error("the quiet ended during a run and nothing waits for the next one, so memory is never given back after it") + } + }) + join(host) + + host.quiet.fireAll() + host.quiet.fireAll() + if host.releases != 1 { + t.Errorf("the run ended, the quiet was waited out, and memory was given back %d time(s), expected once", host.releases) + } +} diff --git a/internal/guard/window_test.go b/internal/guard/window_test.go index e673b11..b483f11 100644 --- a/internal/guard/window_test.go +++ b/internal/guard/window_test.go @@ -98,8 +98,69 @@ type fakeHost struct { // settles and expansions count what Settling and ExpandingPreset were told. settles int expansions int + + // quiet is the clock the window waits out its quiet on, when a guard holds + // it - see QuietLater. Nil means the wait never ends, and releases counts + // the times the window gave memory back. + quiet *quietClock + releases int +} + +// quietClock keeps every request for later, each with whether it was called +// off - a list rather than one slot, because what the guard asks is whether a +// request already waiting was taken back when something happened. +type quietClock struct { + pending []*quietRequest + asked int +} + +type quietRequest struct { + after time.Duration + then func() + calledOff bool +} + +// fireAll runs every request waiting that was not called off, in the order +// they were asked for, and forgets them. What they ask for while running +// waits for the next call. +func (c *quietClock) fireAll() { + due := c.pending + c.pending = nil + for _, r := range due { + if !r.calledOff { + r.then() + } + } } +// waiting is the requests not called off. +func (c *quietClock) waiting() []*quietRequest { + var out []*quietRequest + for _, r := range c.pending { + if !r.calledOff { + out = append(out, r) + } + } + return out +} + +// QuietLater is the window's clock for waiting out its quiet (tidy.go), kept +// apart from Later so that a request asked for on every key cannot take the +// place of the busy face a guard is holding. Never fires unless a guard holds +// it - no guard here lasts the seventy seconds a real window waits. +func (h *fakeHost) QuietLater(after time.Duration, then func()) func() { + if h.quiet == nil { + return func() {} + } + r := &quietRequest{after: after, then: then} + h.quiet.pending = append(h.quiet.pending, r) + h.quiet.asked++ + return func() { r.calledOff = true } +} + +// ReleasingMemory counts the window giving memory back. +func (h *fakeHost) ReleasingMemory() { h.releases++ } + // heldClock keeps what the window asked for later, so a guard can look at the // screen BEFORE the busy face arrives and then let it arrive. type heldClock struct { diff --git a/internal/gui/window/generate.go b/internal/gui/window/generate.go index 2d22ce2..b0e90bb 100644 --- a/internal/gui/window/generate.go +++ b/internal/gui/window/generate.go @@ -202,7 +202,7 @@ type Generate struct { // NewGenerate builds the screen. links are the buttons to the other screens. func NewGenerate(host Host, links ...fyne.CanvasObject) *Generate { g := &Generate{runner: newRunner(host.Later), host: host, tips: parts.NewTips(), settingsFolded: true} - g.runner.settle = countedSettle(host, g.settle) + g.runner.settle = watchedSettle(host, g.runner, g.settle) g.runner.offer.through(host) // This screen is one target and draws its boxes under the bare key, so a // refusal that arrives carrying a position belongs to the box of that name. diff --git a/internal/gui/window/open.go b/internal/gui/window/open.go index b6d0ec0..5b7131c 100644 --- a/internal/gui/window/open.go +++ b/internal/gui/window/open.go @@ -114,7 +114,10 @@ func Open(h Host) fyne.Size { // on the signal handler in cmd/tfg, and closing a window is not a signal - // so without this the run would carry on with nobody watching it, or die in // the middle of a file. - closeCleanly(h, []interface{ Stop() }{gen, pre, rec}, working, &showing) + // One wait for quiet for the whole window, told by every screen, and + // stopped with them when the window closes - see tidy.go. + quiet := tidyWhenLeftAlone(h, gen.runner, pre.runner, rec.runner) + closeCleanly(h, []interface{ Stop() }{gen, pre, rec, quiet}, working, &showing) offerSettling(h, []interface{ Settled() }{gen, pre, rec}) offerHolding(h, []interface{ HoldBeforeFinishing(func()) }{gen, pre, rec}) @@ -450,30 +453,37 @@ func offerHolding(h Host, screens []interface{ HoldBeforeFinishing(func()) }) { } } -// countedSettle is a screen's reading of its form, told to a host that counts -// the readings and handed back untouched to any other. -// -// The same shape as the two above - an optional interface, checked rather than -// required, and nothing in the shipped program implements it. It is here for a -// number a guard cannot read off the screen: how many times one change of a -// box read the form. It was two until 2026-09-23, once for the line and once -// for the box, and on the preset screen every reading expanded the preset - -// 271-295 ms of the window's thread per key for upload-validation, measured -// in docs/GUI-MEMORY-2026-09-23.md section 4h. -func countedSettle(h Host, settle settler) settler { - c, ok := h.(interface{ Settling() }) - if !ok { - return settle - } +// watchedSettle is a screen's reading of its form, with two things told about +// it on the way. +// +// The window's wait for quiet is told, so that memory is given back only once +// the window has been left alone - see tidy.go. Every change somebody makes and +// every run reads the form, so this is the one place that hears all of them. +// +// A host that counts the readings is told as well. The same shape as the two +// above - an optional interface, checked rather than required, and nothing in +// the shipped program implements it. It is here for a number a guard cannot +// read off the screen: how many times one change of a box read the form. It +// was two until 2026-09-23, once for the line and once for the box, and on the +// preset screen every reading expanded the preset - 271-295 ms of the +// window's thread per key for upload-validation, measured in +// docs/GUI-MEMORY-2026-09-23.md section 4h. +func watchedSettle(h Host, r *runner, settle settler) settler { + c, counts := h.(interface{ Settling() }) return func() ([]engine.Target, engine.Options, error) { - c.Settling() + if counts { + c.Settling() + } + if r.touched != nil { + r.touched() + } return settle() } } // tellExpanding says to a host that counts them that a preset is being // expanded rather than taken from what the screen remembers - see -// lastExpansion. The same kind of seam as countedSettle, for the other half +// lastExpansion. The same kind of seam as watchedSettle, for the other half // of the same question. func tellExpanding(h Host) { if c, ok := h.(interface{ ExpandingPreset() }); ok { diff --git a/internal/gui/window/preset.go b/internal/gui/window/preset.go index d1395f2..aa0dc62 100644 --- a/internal/gui/window/preset.go +++ b/internal/gui/window/preset.go @@ -60,7 +60,7 @@ type Preset struct { func NewPreset(host Host, links ...fyne.CanvasObject) *Preset { p := &Preset{runner: newRunner(host.Later), host: host, tips: parts.NewTips()} p.runner.offer.through(host) - p.runner.settle = countedSettle(host, p.settle) + p.runner.settle = watchedSettle(host, p.runner, p.settle) // No readdress here, and that is the boundary of this screen rather than an // omission. The other two screens draw boxes for the settings of a target, // so a refusal carrying a position has a box to be moved onto. This one diff --git a/internal/gui/window/recipe.go b/internal/gui/window/recipe.go index bdf2d2b..bcb1199 100644 --- a/internal/gui/window/recipe.go +++ b/internal/gui/window/recipe.go @@ -162,7 +162,7 @@ type content struct { // also what the single batch screen shows, so the two read as one tool. func NewRecipe(host Host, links ...fyne.CanvasObject) *Recipe { r := &Recipe{runner: newRunner(host.Later), host: host, tips: parts.NewTips()} - r.runner.settle = countedSettle(host, r.settle) + r.runner.settle = watchedSettle(host, r.runner, r.settle) r.runner.offer.through(host) // A refusal about a size belongs on the box the switch is showing. r.runner.readdress = r.readdressSizeWay diff --git a/internal/gui/window/run.go b/internal/gui/window/run.go index 8a5e24a..a353c59 100644 --- a/internal/gui/window/run.go +++ b/internal/gui/window/run.go @@ -199,6 +199,12 @@ type runner struct { // the real one, so the run says which number it made up. The command line // prints these as "note:" lines and this is the window's half of it. notes []string + + // touched is told at every reading of the form - see watchedSettle - so + // that the window gives memory back once it has been left alone. Every + // change somebody makes and every run reads the form. Nil on a screen + // built on its own. See tidy.go. + touched func() } // Fields is every box on this screen, for a guard to compare against the tree. diff --git a/internal/gui/window/tidy.go b/internal/gui/window/tidy.go new file mode 100644 index 0000000..0a6ed10 --- /dev/null +++ b/internal/gui/window/tidy.go @@ -0,0 +1,155 @@ +package window + +import ( + "runtime/debug" + "time" +) + +// Giving memory back to the system once the window has been left alone. +// +// After a spell of work - batches added and taken away, a base preset switched, +// a run - the process kept 189-206 MB for as long as the window stood idle, +// measured over five minutes on 2026-09-23 (docs/GUI-MEMORY-2026-09-23.md +// section 4j). Two things hold it, and neither is ours to change: +// +// - The toolkit keeps the renderers of objects that have left the screen for +// a minute, and destroys them only while drawing a frame (Fyne 2.8.1, +// internal/cache/base.go Clean). A window nobody touches draws no frames, +// so they stay. +// - Go gives the pages a collection freed back to the system slowly, and runs +// a collection in the quiet only every two minutes. +// +// So after tidyAfterQuiet of nothing, one frame is asked for - the root of the +// canvas marked for redrawing, which touches no widget and moves nothing: the +// scroll, the keyboard, what every box, menu and switch holds were compared +// either side and were the same. tidyAfterFrame later the memory is given +// back. Measured: 252-263 MB to 117-119 MB, in four runs of four. +// +// Given back beside the window rather than on its thread, and that is the +// owner's decision of 2026-09-24: one of ten calls of FreeOSMemory measured on +// the window's thread took 2491 ms against 4.6-16.2 ms for the rest, for a +// reason nobody found, and it would land on somebody coming back to the +// window. So this file starts a goroutine, and it is declared as concurrent +// for that reason. The goroutine touches nothing of ours. +// +// What it does not do: a minimised window draws no frames either, so there the +// renderers stay and only what Go holds goes back. + +// tidyAfterQuiet is how long nothing has to happen. Past the minute the +// toolkit keeps a renderer for (Fyne cache.ValidDuration), so the ones that +// left the screen with the last change have expired by then. +const tidyAfterQuiet = 70 * time.Second + +// tidyAfterFrame is how long after the frame the memory goes back. The toolkit +// cleans at most once in ten seconds and a clean that comes sooner waits for +// the next, so two seconds - what the first measurement used - would sometimes +// give back before the renderers were gone. +const tidyAfterFrame = 12 * time.Second + +// tidyWhenLeftAlone gives the window its one wait for quiet and has every +// screen tell it at every reading of the form - which every change and every +// run makes. +func tidyWhenLeftAlone(h Host, screens ...*runner) *tidy { + t := newTidy(h, func() bool { return anyBusy(screens) }) + for _, r := range screens { + r.touched = t.touch + } + return t +} + +// anyBusy says whether work owns any of these screens. +func anyBusy(screens []*runner) bool { + for _, r := range screens { + if r.busy.occupied { + return true + } + } + return false +} + +// tidy is the one quiet period being waited out, for the whole window. +type tidy struct { + host Host + // busy says whether any screen has work going. Memory is not given back + // under a run - the run is what is using it - and the wait starts again, + // so a run longer than the wait is followed by a release once it ends. + busy func() bool + // later is the host's clock, or a clock of its own a guard hands in - see + // newTidy. + later later + callOff func() + // wait counts the waits called off, so that one called off on its way + // does nothing when it arrives. Calling the clock off is not enough: the + // real window's clock hands what it fires to the toolkit's queue, and a + // call already queued when somebody types still runs. The same as + // busy.epoch, and an outside review of the pull request named it. + wait int +} + +// newTidy builds the wait for one window. +// +// On the host's clock unless the host has a separate one for this. A guard's +// host holds ONE pending request - the busy face's, which several guards hold +// and fire - and a quiet period asked for on every key would take its place. +// So the guards' host keeps these apart, and the program's host, whose clock +// is a timer per request, needs nothing of the kind. +func newTidy(h Host, busy func() bool) *tidy { + t := &tidy{host: h, busy: busy, later: h.Later} + if q, ok := h.(interface { + QuietLater(after time.Duration, then func()) func() + }); ok { + t.later = q.QuietLater + } + return t +} + +// touch is told that something happened, and starts the quiet over. +func (t *tidy) touch() { + t.Stop() + t.after(tidyAfterQuiet, t.frame) +} + +// Stop calls off whatever is being waited for, a call already on its way +// included. Closing the window stops it, along with every screen. +func (t *tidy) Stop() { + t.wait++ + if t.callOff != nil { + t.callOff() + t.callOff = nil + } +} + +// after asks the clock for then, which arrives only if nothing called the +// wait off in the meantime. +func (t *tidy) after(d time.Duration, then func()) { + mine := t.wait + t.callOff = t.later(d, func() { + if t.wait != mine { + return + } + t.callOff = nil + then() + }) +} + +// frame asks the toolkit to draw, so that it lets go of what has expired. Even +// under a run, which costs one frame and nothing else - whether to give memory +// back is asked once, in release. +func (t *tidy) frame() { + if c := t.host.Canvas(); c != nil && c.Content() != nil { + c.Refresh(c.Content()) + } + t.after(tidyAfterFrame, t.release) +} + +// release gives the memory back, beside the window. +func (t *tidy) release() { + if t.busy() { + t.touch() + return + } + if r, ok := t.host.(interface{ ReleasingMemory() }); ok { + r.ReleasingMemory() + } + go debug.FreeOSMemory() +}