Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,15 @@ because it turns other people's test suites red.

### Fixed

- **The `Several batches` screen no longer slows down the longer it is
used.** Every batch added, removed or copied, every format chosen and every
press of `Start from a preset` made each box on the screen report a change
one more time, so the switch took 0.7 seconds at its first press and 6
seconds at its twentieth. With a preset switched on, every key typed took
about 0.4 seconds, because the smallest file of every format was worked out
again for each one. Both are now done once, and the window uses less memory
while you type.

- **A preview or a run refused while it was being planned no longer leaves
"Working out what this would cost..." standing over the refusal.**

Expand Down
36 changes: 36 additions & 0 deletions internal/format/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,42 @@ func Register(d Descriptor) {
registry[d.ID] = d
}

// SmallestWithLabel is d.SmallestAccepted(Request{Label: true}), worked out
// once per format per process and remembered.
//
// The request is the same every time, so the answer is too - and finding it
// means planning the format at growing sizes, which for a picture means
// encoding one. The minimal preset asks it of every format at every
// expansion, and the window expands that preset on every change while the
// batch screen builds on it: measured on 2026-09-23, 50.4 MB allocated per
// expansion afresh against 0.51 MB remembered, and a keystroke that cost
// 380 ms in the real window (docs/GUI-MEMORY-2026-09-23.md section 2.3).
//
// Keyed by id, which is safe because Register refuses a second descriptor
// under one. Here rather than beside its caller because this file is already
// where the registry's reads meet its writes: the window settles from its
// worker as well as from its own goroutine. The size is worked out without
// the lock held, because planning an archive reads the registry itself.
func SmallestWithLabel(d Descriptor) int64 {
smallestMu.Lock()
known, ok := smallestKnown[d.ID]
smallestMu.Unlock()
if ok {
return known
}
size := d.SmallestAccepted(Request{Label: true})
smallestMu.Lock()
smallestKnown[d.ID] = size
smallestMu.Unlock()
return size
}

// smallestKnown is what SmallestWithLabel has worked out, by format id.
var (
smallestMu sync.Mutex
smallestKnown = map[string]int64{}
)

// SortChoices puts a closed set in the order somebody looks for a value in.
//
// Here rather than in the menu that draws them, and that is the whole point:
Expand Down
6 changes: 5 additions & 1 deletion internal/guard/branching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ const (
// every owner, and the loop left behind is two deep rather than three. The
// four presets that arrived the same day were flattened to hold the number
// where it was - this is the one that went below it.
crowdedDepthFunctions = 50
// Lowered from 50 on 2026-09-23: Fields.listen wrapped each of three kinds
// of control in its own closure and went one deeper when it learnt to wrap
// only once - the three became one function, chainOnce, and listen came
// out of the band.
crowdedDepthFunctions = 49

// An axis this set does not watch. crowding() asks n >= band, so nothing
// reaches it.
Expand Down
151 changes: 151 additions & 0 deletions internal/guard/livecheck_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
package guard

import (
"fmt"
"reflect"
"strings"
"testing"

"fyne.io/fyne/v2"
"fyne.io/fyne/v2/test"
"fyne.io/fyne/v2/widget"

"github.com/donislawdev/TestingFilesGenerator/internal/gui/parts"
"github.com/donislawdev/TestingFilesGenerator/internal/gui/text"
"github.com/donislawdev/TestingFilesGenerator/internal/gui/window"
"github.com/donislawdev/TestingFilesGenerator/internal/recipe"
)

// Every box that is wrong is marked, not the first one.
Expand Down Expand Up @@ -253,6 +258,152 @@ func TestTypingIsStillCheckedAfterARunHasFinished(t *testing.T) {
// has to know. Its own documentation promises the call works "before or after
// the fields exist". Found by an outside review of the whole tree on
// 2026-08-23, docs/CODE-REVIEW-2026-08-23.md section 2.
// A control registered again reports a change once, under the address it was
// registered at last, and counts into the caption drawn with it last.
//
// The batch screen registers every field again on every rebuild, because an
// address carries the batch's position - and it keeps the controls, so what
// was typed survives. listen and counter used to wrap the control's callback
// on every registration, so after k rebuilds one change was reported k times,
// under every address the control had ever had, and k captions nobody could
// see were counted into. Measured in the real window on 2026-09-23: the
// preset switch took 0.7 s at the first press and 6.1 s at the twentieth,
// docs/GUI-MEMORY-2026-09-23.md section 2.2.
func TestAControlRegisteredAgainReportsOnceUnderItsLatestAddress(t *testing.T) {
test.NewApp()
t.Cleanup(func() { test.NewApp() })

box := parts.NewEntry()
menu := parts.NewChooser([]string{"one", "two"}, nil)
toggle := parts.NewToggle(nil)
for _, c := range []struct {
name string
control fyne.CanvasObject
change func()
}{
{"a box", box, func() { box.SetText("1kb") }},
{"a menu", menu, func() { menu.SetSelected("two") }},
{"a switch", toggle, func() { toggle.SetChecked(true) }},
} {
t.Run(c.name, func(t *testing.T) {
fields := parts.NewFields()
var told []string
fields.WhenTypedIn(func(setting string) { told = append(told, setting) })
var drawn []fyne.CanvasObject
for i := 1; i <= 5; i++ {
address := fmt.Sprintf("targets[%d].size", i)
fields.KeepFirst(0)
fields.InBytes(address)
drawn = append(drawn, fields.Add(address, "Size", "", parts.Detail{}, c.control))
}
c.change()
if want := []string{"targets[5].size"}; !reflect.DeepEqual(told, want) {
t.Errorf("one change after five registrations told the screen %q, expected %q", told, want)
}
if c.control != fyne.CanvasObject(box) {
return
}
last, first := byteCountIn(drawn[len(drawn)-1]), byteCountIn(drawn[0])
if last == nil || first == nil {
t.Fatal("a size box was registered and no count of bytes was drawn with it, so this guard is not in the state it asks about")
}
if last.Text == "" {
t.Error("the caption drawn with the box last says nothing about the size typed into it")
}
if first.Text != "" {
t.Errorf("the caption from the first registration, which is no longer on any screen, was counted into: %q", first.Text)
}
})
}
}

// No control stands under two addresses at once, on any screen.
//
// The fix above rests on it. A control registered again reports under the
// address it was registered at LAST, which is right when the second
// registration replaces the first - a rebuild - and wrong if one control were
// ever registered under two addresses in the same pass, because the first
// would then stop hearing about it. Asked of the registry of every work
// screen, the batch screen in the fullest state it has: three batches, the
// files inside an archive, and a preset with its parameters.
func TestNoControlIsRegisteredUnderTwoAddressesAtOnce(t *testing.T) {
host := newFakeHost(t)
gen, pre, rec := window.NewGenerate(host), window.NewPreset(host), window.NewRecipe(host)

body := rec.Object()
for i := 0; i < 2; i++ {
add := buttonNamed(body, text.ButtonAddBatch())
if add == nil {
t.Fatal("the batch screen has no button to add a batch, so this guard cannot reach three")
}
add.OnTapped()
}
chooserIn(t, rec.Fields(), recipe.TargetAddress(1, recipe.KeyFormat)).SetSelected("zip")
contents := buttonNamed(body, text.ButtonAddContents())
if contents == nil {
t.Fatal("a zip batch offers no way to say what it holds, so this guard cannot reach the table of contents")
}
contents.OnTapped()
for _, f := range rec.Fields().All() {
if toggle, is := f.Control.(*parts.Toggle); is && !toggle.Checked {
toggle.SetChecked(true)
break
}
}

for _, s := range []struct {
name string
fields *parts.Fields
must []string
}{
{"single batch", gen.Fields(), []string{"size"}},
{"presets", pre.Fields(), nil},
{"several batches", rec.Fields(), []string{
recipe.TargetAddress(3, recipe.KeyID),
recipe.ContentAddress(1, 1, recipe.KeySize),
recipe.KeyExtends,
}},
} {
t.Run(s.name, func(t *testing.T) {
under := map[fyne.CanvasObject]string{}
settings := map[string]bool{}
for _, f := range s.fields.All() {
settings[f.Setting] = true
for _, c := range reportingControls(f.Control) {
if was, seen := under[c]; seen && was != f.Setting {
t.Errorf("one %T stands under %q and under %q, and a change would be told under the second alone", c, was, f.Setting)
}
under[c] = f.Setting
}
}
for _, want := range s.must {
if !settings[want] {
t.Fatalf("the registry has no %q, so this guard is not in the state it asks about", want)
}
}
if len(under) == 0 {
t.Fatal("no control on this screen reports a change, so nothing was asked")
}
})
}
}

// reportingControls are the controls under one field that tell the screen
// about a change - the three kinds Fields.listen wires.
func reportingControls(o fyne.CanvasObject) []fyne.CanvasObject {
switch it := o.(type) {
case *parts.Entry, *parts.Chooser, *parts.Toggle:
return []fyne.CanvasObject{it}
case *fyne.Container:
var out []fyne.CanvasObject
for _, child := range it.Objects {
out = append(out, reportingControls(child)...)
}
return out
}
return nil
}

func TestAFieldWiredBeforeTheScreenListensReportsOnce(t *testing.T) {
for _, order := range []string{"the field first", "the listener first"} {
t.Run(order, func(t *testing.T) {
Expand Down
38 changes: 38 additions & 0 deletions internal/guard/minimalset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package guard

import (
"bytes"
"runtime"
"strings"
"testing"

Expand Down Expand Up @@ -138,6 +139,43 @@ func TestTheMinimalSetSitsOnEveryFormatsFloor(t *testing.T) {
// The bytes of the files never moved, because a seed comes from the id of a
// target rather than from its place in the list. That is what made this quiet:
// every file was right and only the record of them disagreed.
// The smallest size of each format is worked out once, not at every expansion.
//
// Finding it means planning the format at growing sizes, and for a picture
// that means encoding one. The window expands this set on every change while
// the batch screen builds on it, so a set worked out afresh each time made a
// keystroke there cost 380 ms and ~379 MB of garbage in the real window on
// 2026-09-23 (docs/GUI-MEMORY-2026-09-23.md section 2.3). Measured here the
// same day, least of five: 50.4 MB per expansion afresh, 0.51 MB remembered.
// The line sits a factor of ten from each.
//
// The least of several readings, because the counter is the whole process's
// and a reading can only be too high - the lesson of tools/probes/alloccount.
// That the remembered sizes are the RIGHT ones is
// TestTheMinimalSetSitsOnEveryFormatsFloor's question, not this one's.
func TestTheMinimalSetIsWorkedOutOnceAndNotAtEveryExpansion(t *testing.T) {
const ceiling = 5 << 20
if _, err := preset.Expand("empty-and-minimal", preset.Args{}); err != nil {
t.Fatalf("the set did not expand, so nothing was asked: %v", err)
}
least := ^uint64(0)
for i := 0; i < 5; i++ {
var before, after runtime.MemStats
runtime.ReadMemStats(&before)
if _, err := preset.Expand("empty-and-minimal", preset.Args{}); err != nil {
t.Fatal(err)
}
runtime.ReadMemStats(&after)
if spent := after.TotalAlloc - before.TotalAlloc; spent < least {
least = spent
}
}
if least > ceiling {
t.Errorf("expanding the minimal set a second time allocated %d bytes, over %d - "+
"the smallest size of every format is being worked out again", least, ceiling)
}
}

func TestTheMinimalSetIsTheSameWhateverOrderTheFormatsAreNamedIn(t *testing.T) {
// Two formats far apart in the registry, so a walk that kept the typing
// cannot pass by accident.
Expand Down
7 changes: 7 additions & 0 deletions internal/gui/parts/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ type Entry struct {
// the screen, because the screen is what knows the canvas.
onOurs func(fyne.Shortcut)

// reports and counts are the two things a field does when this box
// changes - tell the screen under which address, and count the bytes into
// which caption. Wired once and re-pointed on every registration, see
// wiredOnce in fields.go.
reports wiredOnce[string]
counts wiredOnce[*ByteCount]

// ring is the edge a field draws round this box when the keyboard is in it
// or a run refused it, so a box carries the same 2 px mark as the menu and
// the switch beside it rather than only the toolkit's own 1 px border -
Expand Down
Loading
Loading