Merge pull request #123 from bnema/fix/benchmark-fixture-leak
fix(daemon): release benchmark fixtures per iteration
2 files · +119 -27
M internal/usecase/daemon/daemon_test.go +18 -4
| @@ -64,12 +64,19 @@ func (t *signalTimer) Reset(time.Duration) bool { return false } | |||
| 64 | 64 | func (t *signalTimer) Stop() bool { return true } | |
| 65 | 65 | ||
| 66 | 66 | func newTestDaemon(t testing.TB, ptys ports.PTYFactory, clk ports.Clock) *Daemon { | |
| 67 | + | t.Helper() | |
| 68 | + | return newTestDaemonWithCleanup(t, ptys, clk, true) | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | func newTestDaemonWithCleanup(t testing.TB, ptys ports.PTYFactory, clk ports.Clock, registerCleanup bool) *Daemon { | |
| 67 | 72 | t.Helper() | |
| 68 | 73 | d := New(ptys, clk, slog.New(slog.NewTextHandler(io.Discard, nil))) | |
| 69 | 74 | d.serveCtx, d.serveCancel = context.WithCancel(context.Background()) | |
| 70 | 75 | d.hardCtx, d.hardCancel = context.WithCancel(context.Background()) | |
| 71 | - | t.Cleanup(d.serveCancel) | |
| 72 | - | t.Cleanup(d.hardCancel) | |
| 76 | + | if registerCleanup { | |
| 77 | + | t.Cleanup(d.serveCancel) | |
| 78 | + | t.Cleanup(d.hardCancel) | |
| 79 | + | } | |
| 73 | 80 | return d | |
| 74 | 81 | } | |
| 75 | 82 | ||
| @@ -368,7 +375,12 @@ func newCapturingTransport(t testing.TB) (*portsmocks.MockTransport, chan ports. | |||
| 368 | 375 | ||
| 369 | 376 | func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |
| 370 | 377 | t.Helper() | |
| 371 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 378 | + | return newManualSessionWithPTYsCleanup(t, true, ptys...) | |
| 379 | + | } | |
| 380 | + | ||
| 381 | + | func newManualSessionWithPTYsCleanup(t testing.TB, registerCleanup bool, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |
| 382 | + | t.Helper() | |
| 383 | + | d := newTestDaemonWithCleanup(t, nil, stubClock{}, registerCleanup) | |
| 372 | 384 | tr, sends := newCapturingTransport(t) | |
| 373 | 385 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 374 | 386 | ac.initOverlays() | |
| @@ -387,7 +399,9 @@ func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *sessio | |||
| 387 | 399 | ac.setSession(sess) | |
| 388 | 400 | ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, nil) | |
| 389 | 401 | d.sessions[sess.id] = sess | |
| 390 | - | t.Cleanup(cancel) | |
| 402 | + | if registerCleanup { | |
| 403 | + | t.Cleanup(cancel) | |
| 404 | + | } | |
| 391 | 405 | return d, sess, ac, sends | |
| 392 | 406 | } | |
| 393 | 407 | ||
| @@ -64,12 +64,19 @@ func (t *signalTimer) Reset(time.Duration) bool { return false } | |||||
| 64 | func (t *signalTimer) Stop() bool { return true } | 64 | func (t *signalTimer) Stop() bool { return true } | ||
| 65 | 65 | ||||
| 66 | func newTestDaemon(t testing.TB, ptys ports.PTYFactory, clk ports.Clock) *Daemon { | 66 | func newTestDaemon(t testing.TB, ptys ports.PTYFactory, clk ports.Clock) *Daemon { | ||
| 67 | + | t.Helper() | |||
| 68 | + | return newTestDaemonWithCleanup(t, ptys, clk, true) | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | func newTestDaemonWithCleanup(t testing.TB, ptys ports.PTYFactory, clk ports.Clock, registerCleanup bool) *Daemon { | |||
| 67 | t.Helper() | 72 | t.Helper() | ||
| 68 | d := New(ptys, clk, slog.New(slog.NewTextHandler(io.Discard, nil))) | 73 | d := New(ptys, clk, slog.New(slog.NewTextHandler(io.Discard, nil))) | ||
| 69 | d.serveCtx, d.serveCancel = context.WithCancel(context.Background()) | 74 | d.serveCtx, d.serveCancel = context.WithCancel(context.Background()) | ||
| 70 | d.hardCtx, d.hardCancel = context.WithCancel(context.Background()) | 75 | d.hardCtx, d.hardCancel = context.WithCancel(context.Background()) | ||
| 71 | - | t.Cleanup(d.serveCancel) | 76 | + | if registerCleanup { |
| 72 | - | t.Cleanup(d.hardCancel) | 77 | + | t.Cleanup(d.serveCancel) |
| 78 | + | t.Cleanup(d.hardCancel) | |||
| 79 | + | } | |||
| 73 | return d | 80 | return d | ||
| 74 | } | 81 | } | ||
| 75 | 82 | ||||
| @@ -368,7 +375,12 @@ func newCapturingTransport(t testing.TB) (*portsmocks.MockTransport, chan ports. | |||||
| 368 | 375 | ||||
| 369 | func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | 376 | func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | ||
| 370 | t.Helper() | 377 | t.Helper() | ||
| 371 | - | d := newTestDaemon(t, nil, stubClock{}) | 378 | + | return newManualSessionWithPTYsCleanup(t, true, ptys...) |
| 379 | + | } | |||
| 380 | + | ||||
| 381 | + | func newManualSessionWithPTYsCleanup(t testing.TB, registerCleanup bool, ptys ...ports.PTY) (*Daemon, *session, *attachedClient, chan ports.Frame) { | |||
| 382 | + | t.Helper() | |||
| 383 | + | d := newTestDaemonWithCleanup(t, nil, stubClock{}, registerCleanup) | |||
| 372 | tr, sends := newCapturingTransport(t) | 384 | tr, sends := newCapturingTransport(t) | ||
| 373 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | 385 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | ||
| 374 | ac.initOverlays() | 386 | ac.initOverlays() | ||
| @@ -387,7 +399,9 @@ func newManualSessionWithPTYs(t testing.TB, ptys ...ports.PTY) (*Daemon, *sessio | |||||
| 387 | ac.setSession(sess) | 399 | ac.setSession(sess) | ||
| 388 | ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, nil) | 400 | ac.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: ac}, nil) | ||
| 389 | d.sessions[sess.id] = sess | 401 | d.sessions[sess.id] = sess | ||
| 390 | - | t.Cleanup(cancel) | 402 | + | if registerCleanup { |
| 403 | + | t.Cleanup(cancel) | |||
| 404 | + | } | |||
| 391 | return d, sess, ac, sends | 405 | return d, sess, ac, sends | ||
| 392 | } | 406 | } | ||
| 393 | 407 | ||||
M internal/usecase/daemon/performance_benchmark_test.go +101 -23
| @@ -2,6 +2,7 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "io" | |
| 7 | 8 | "sync" | |
| @@ -149,6 +150,38 @@ func TestPerformanceFixtureCellLimitedHistory(t *testing.T) { | |||
| 149 | 150 | require.LessOrEqual(t, cells, capCells) | |
| 150 | 151 | } | |
| 151 | 152 | ||
| 153 | + | func TestPerformanceFixtureExplicitCloseReleasesIterationState(t *testing.T) { | |
| 154 | + | fixture := newPerformanceFixtureWithCleanup(t, performanceConfig{}, false) | |
| 155 | + | d, sess := fixture.d, fixture.sess | |
| 156 | + | serveCtx, hardCtx := d.serveCtx, d.hardCtx | |
| 157 | + | killErr := errors.New("scripted kill failure") | |
| 158 | + | fixture.killSession = func(sess *session, reason uint8, purge bool) error { | |
| 159 | + | require.NoError(t, d.killSession(sess, reason, purge)) | |
| 160 | + | return killErr | |
| 161 | + | } | |
| 162 | + | ||
| 163 | + | err := fixture.close() | |
| 164 | + | ||
| 165 | + | require.ErrorIs(t, err, killErr) | |
| 166 | + | require.Nil(t, fixture.d) | |
| 167 | + | require.Nil(t, fixture.sess) | |
| 168 | + | require.Nil(t, fixture.activePane) | |
| 169 | + | require.Error(t, sess.ctx.Err()) | |
| 170 | + | require.Error(t, serveCtx.Err()) | |
| 171 | + | require.Error(t, hardCtx.Err()) | |
| 172 | + | d.mu.Lock() | |
| 173 | + | sessionIDs := make([]domain.SessionID, 0, len(d.sessions)) | |
| 174 | + | for id := range d.sessions { | |
| 175 | + | sessionIDs = append(sessionIDs, id) | |
| 176 | + | } | |
| 177 | + | d.mu.Unlock() | |
| 178 | + | require.NotContains(t, sessionIDs, sess.id) | |
| 179 | + | d.snapshotWorkerMu.Lock() | |
| 180 | + | workerCancel := d.snapshotWorkerCancel | |
| 181 | + | d.snapshotWorkerMu.Unlock() | |
| 182 | + | require.Nil(t, workerCancel) | |
| 183 | + | } | |
| 184 | + | ||
| 152 | 185 | func TestCountingOutputTransportCountsOpaquePayloadAndRejectsShortPayload(t *testing.T) { | |
| 153 | 186 | transport := &countingOutputTransport{} | |
| 154 | 187 | ||
| @@ -506,29 +539,40 @@ func benchmarkIncrementalSnapshotRepository(b *testing.B, scenario snapshotBench | |||
| 506 | 539 | b.ReportAllocs() | |
| 507 | 540 | b.ResetTimer() | |
| 508 | 541 | for operation := 0; b.Loop(); operation++ { | |
| 509 | - | // A new fixture is deliberately outside the timed region. It gives every | |
| 510 | - | // iteration the same filesystem state: initial has no generation, | |
| 511 | - | // unchanged/visible/tail/sealed begin after one persisted generation. | |
| 512 | - | // Reusing one fixture would eventually turn a tail into a sealed chunk | |
| 513 | - | // and dilute initial-write bytes over later idempotent publications. | |
| 514 | - | b.StopTimer() | |
| 515 | - | fixture := newPerformanceFixture(b, performanceConfig{size: scenario.size, panes: 1, historyRows: scenario.historyRows}) | |
| 516 | - | validateSnapshotBenchmarkFixture(b, fixture, scenario) | |
| 517 | - | generation := uint64(1) | |
| 518 | - | if scenario.baseline { | |
| 542 | + | func() { | |
| 543 | + | // A new fixture gives every iteration the same filesystem state: initial | |
| 544 | + | // has no generation, while changed scenarios begin after one persisted | |
| 545 | + | // generation. Keep its expensive 10k-row construction timed so benchmark | |
| 546 | + | // calibration accounts for real wall time instead of scheduling thousands | |
| 547 | + | // of hidden setup iterations. | |
| 548 | + | // | |
| 549 | + | // b.Cleanup retains callbacks until the sub-benchmark ends; deferring an | |
| 550 | + | // explicit close avoids retaining every fixture and OOM. | |
| 551 | + | fixture := newPerformanceFixtureWithCleanup(b, performanceConfig{size: scenario.size, panes: 1, historyRows: scenario.historyRows}, false) | |
| 552 | + | b.StopTimer() | |
| 553 | + | defer func() { | |
| 554 | + | b.StopTimer() | |
| 555 | + | if err := fixture.close(); err != nil { | |
| 556 | + | b.Fatalf("close performance fixture: %v", err) | |
| 557 | + | } | |
| 558 | + | }() | |
| 559 | + | validateSnapshotBenchmarkFixture(b, fixture, scenario) | |
| 560 | + | generation := uint64(1) | |
| 561 | + | if scenario.baseline { | |
| 562 | + | publishBenchmarkSnapshot(b, fixture, generation) | |
| 563 | + | generation++ | |
| 564 | + | fixture.snapshots.reset() | |
| 565 | + | } | |
| 566 | + | b.StartTimer() | |
| 567 | + | if scenario.mutate != nil { | |
| 568 | + | scenario.mutate(fixture, operation) | |
| 569 | + | } | |
| 519 | 570 | publishBenchmarkSnapshot(b, fixture, generation) | |
| 520 | - | generation++ | |
| 521 | - | fixture.snapshots.reset() | |
| 522 | - | } | |
| 523 | - | b.StartTimer() | |
| 524 | - | if scenario.mutate != nil { | |
| 525 | - | scenario.mutate(fixture, operation) | |
| 526 | - | } | |
| 527 | - | publishBenchmarkSnapshot(b, fixture, generation) | |
| 528 | - | b.StopTimer() | |
| 571 | + | b.StopTimer() | |
| 529 | 572 | ||
| 530 | - | metrics = addCountingSnapshotMetrics(metrics, fixture.snapshots.metrics()) | |
| 531 | - | peakCacheBytes = max(peakCacheBytes, benchmarkSnapshotCacheBytes(fixture.sess)) | |
| 573 | + | metrics = addCountingSnapshotMetrics(metrics, fixture.snapshots.metrics()) | |
| 574 | + | peakCacheBytes = max(peakCacheBytes, benchmarkSnapshotCacheBytes(fixture.sess)) | |
| 575 | + | }() | |
| 532 | 576 | // B.Loop requires the timer to be running at the next iteration. | |
| 533 | 577 | b.StartTimer() | |
| 534 | 578 | } | |
| @@ -952,6 +996,7 @@ type performanceFixture struct { | |||
| 952 | 996 | resizedSize domain.Size | |
| 953 | 997 | pty *scriptedPerformancePTY | |
| 954 | 998 | clock *performanceClock | |
| 999 | + | killSession func(*session, uint8, bool) error | |
| 955 | 1000 | resizeRequests uint64 | |
| 956 | 1001 | resizeCommits uint64 | |
| 957 | 1002 | resizeCommitNanos uint64 | |
| @@ -964,6 +1009,11 @@ type performanceFixture struct { | |||
| 964 | 1009 | } | |
| 965 | 1010 | ||
| 966 | 1011 | func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceFixture { | |
| 1012 | + | t.Helper() | |
| 1013 | + | return newPerformanceFixtureWithCleanup(t, config, true) | |
| 1014 | + | } | |
| 1015 | + | ||
| 1016 | + | func newPerformanceFixtureWithCleanup(t testing.TB, config performanceConfig, registerCleanup bool) *performanceFixture { | |
| 967 | 1017 | t.Helper() | |
| 968 | 1018 | if !config.size.Valid() { | |
| 969 | 1019 | config.size = domain.Size{Cols: 80, Rows: 24} | |
| @@ -980,7 +1030,7 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||
| 980 | 1030 | pty := &scriptedPerformancePTY{} | |
| 981 | 1031 | ptys := make([]ports.PTY, config.tabs) | |
| 982 | 1032 | ptys[0] = pty | |
| 983 | - | d, sess, ac, _ := newManualSessionWithPTYs(t, ptys...) | |
| 1033 | + | d, sess, ac, _ := newManualSessionWithPTYsCleanup(t, registerCleanup, ptys...) | |
| 984 | 1034 | clock := &performanceClock{} | |
| 985 | 1035 | d.clock = clock | |
| 986 | 1036 | output := &countingOutputTransport{} | |
| @@ -999,12 +1049,15 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||
| 999 | 1049 | snapshots: &countingSnapshotRepository{}, | |
| 1000 | 1050 | pty: pty, | |
| 1001 | 1051 | clock: clock, | |
| 1052 | + | killSession: d.killSession, | |
| 1002 | 1053 | liveWrites: [][]byte{[]byte("\x1b[1;1HA\x1b[2;2HA"), []byte("\x1b[1;1HB\x1b[2;2HB")}, | |
| 1003 | 1054 | resizeSizes: [2]domain.Size{{Cols: 100, Rows: 30}, config.size}, | |
| 1004 | 1055 | } | |
| 1005 | 1056 | WithSnapshotRepository(fixture.snapshots, nil)(d) | |
| 1006 | 1057 | d.startSnapshotEncodeWorker() | |
| 1007 | - | t.Cleanup(d.stopSnapshotEncodeWorker) | |
| 1058 | + | if registerCleanup { | |
| 1059 | + | t.Cleanup(d.stopSnapshotEncodeWorker) | |
| 1060 | + | } | |
| 1008 | 1061 | for tabIndex, tb := range sess.tabs { | |
| 1009 | 1062 | fixture.configureTab(tb, tabIndex, config.panes, config.historyRows, tabSize(config.size)) | |
| 1010 | 1063 | } | |
| @@ -1029,6 +1082,31 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||
| 1029 | 1082 | return fixture | |
| 1030 | 1083 | } | |
| 1031 | 1084 | ||
| 1085 | + | func (f *performanceFixture) close() error { | |
| 1086 | + | if f.d == nil { | |
| 1087 | + | return nil | |
| 1088 | + | } | |
| 1089 | + | d, sess := f.d, f.sess | |
| 1090 | + | sess.mu.Lock() | |
| 1091 | + | sess.ephemeral = true | |
| 1092 | + | sess.mu.Unlock() | |
| 1093 | + | killErr := f.killSession(sess, ports.ReasonServerShutdown, false) | |
| 1094 | + | d.stopSnapshotEncodeWorker() | |
| 1095 | + | d.serveCancel() | |
| 1096 | + | d.hardCancel() | |
| 1097 | + | ||
| 1098 | + | f.d = nil | |
| 1099 | + | f.sess = nil | |
| 1100 | + | f.ac = nil | |
| 1101 | + | f.output = nil | |
| 1102 | + | f.snapshots = nil | |
| 1103 | + | f.activePane = nil | |
| 1104 | + | f.pty = nil | |
| 1105 | + | f.clock = nil | |
| 1106 | + | f.killSession = nil | |
| 1107 | + | return killErr | |
| 1108 | + | } | |
| 1109 | + | ||
| 1032 | 1110 | func (f *performanceFixture) configureTab(tb *tab, tabIndex, paneCount, historyRows int, size domain.Size) { | |
| 1033 | 1111 | f.t.Helper() | |
| 1034 | 1112 | tb.mu.Lock() | |
| @@ -2,6 +2,7 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 5 | "fmt" | 6 | "fmt" | ||
| 6 | "io" | 7 | "io" | ||
| 7 | "sync" | 8 | "sync" | ||
| @@ -149,6 +150,38 @@ func TestPerformanceFixtureCellLimitedHistory(t *testing.T) { | |||||
| 149 | require.LessOrEqual(t, cells, capCells) | 150 | require.LessOrEqual(t, cells, capCells) | ||
| 150 | } | 151 | } | ||
| 151 | 152 | ||||
| 153 | + | func TestPerformanceFixtureExplicitCloseReleasesIterationState(t *testing.T) { | |||
| 154 | + | fixture := newPerformanceFixtureWithCleanup(t, performanceConfig{}, false) | |||
| 155 | + | d, sess := fixture.d, fixture.sess | |||
| 156 | + | serveCtx, hardCtx := d.serveCtx, d.hardCtx | |||
| 157 | + | killErr := errors.New("scripted kill failure") | |||
| 158 | + | fixture.killSession = func(sess *session, reason uint8, purge bool) error { | |||
| 159 | + | require.NoError(t, d.killSession(sess, reason, purge)) | |||
| 160 | + | return killErr | |||
| 161 | + | } | |||
| 162 | + | ||||
| 163 | + | err := fixture.close() | |||
| 164 | + | ||||
| 165 | + | require.ErrorIs(t, err, killErr) | |||
| 166 | + | require.Nil(t, fixture.d) | |||
| 167 | + | require.Nil(t, fixture.sess) | |||
| 168 | + | require.Nil(t, fixture.activePane) | |||
| 169 | + | require.Error(t, sess.ctx.Err()) | |||
| 170 | + | require.Error(t, serveCtx.Err()) | |||
| 171 | + | require.Error(t, hardCtx.Err()) | |||
| 172 | + | d.mu.Lock() | |||
| 173 | + | sessionIDs := make([]domain.SessionID, 0, len(d.sessions)) | |||
| 174 | + | for id := range d.sessions { | |||
| 175 | + | sessionIDs = append(sessionIDs, id) | |||
| 176 | + | } | |||
| 177 | + | d.mu.Unlock() | |||
| 178 | + | require.NotContains(t, sessionIDs, sess.id) | |||
| 179 | + | d.snapshotWorkerMu.Lock() | |||
| 180 | + | workerCancel := d.snapshotWorkerCancel | |||
| 181 | + | d.snapshotWorkerMu.Unlock() | |||
| 182 | + | require.Nil(t, workerCancel) | |||
| 183 | + | } | |||
| 184 | + | ||||
| 152 | func TestCountingOutputTransportCountsOpaquePayloadAndRejectsShortPayload(t *testing.T) { | 185 | func TestCountingOutputTransportCountsOpaquePayloadAndRejectsShortPayload(t *testing.T) { | ||
| 153 | transport := &countingOutputTransport{} | 186 | transport := &countingOutputTransport{} | ||
| 154 | 187 | ||||
| @@ -506,29 +539,40 @@ func benchmarkIncrementalSnapshotRepository(b *testing.B, scenario snapshotBench | |||||
| 506 | b.ReportAllocs() | 539 | b.ReportAllocs() | ||
| 507 | b.ResetTimer() | 540 | b.ResetTimer() | ||
| 508 | for operation := 0; b.Loop(); operation++ { | 541 | for operation := 0; b.Loop(); operation++ { | ||
| 509 | - | // A new fixture is deliberately outside the timed region. It gives every | 542 | + | func() { |
| 510 | - | // iteration the same filesystem state: initial has no generation, | 543 | + | // A new fixture gives every iteration the same filesystem state: initial |
| 511 | - | // unchanged/visible/tail/sealed begin after one persisted generation. | 544 | + | // has no generation, while changed scenarios begin after one persisted |
| 512 | - | // Reusing one fixture would eventually turn a tail into a sealed chunk | 545 | + | // generation. Keep its expensive 10k-row construction timed so benchmark |
| 513 | - | // and dilute initial-write bytes over later idempotent publications. | 546 | + | // calibration accounts for real wall time instead of scheduling thousands |
| 514 | - | b.StopTimer() | 547 | + | // of hidden setup iterations. |
| 515 | - | fixture := newPerformanceFixture(b, performanceConfig{size: scenario.size, panes: 1, historyRows: scenario.historyRows}) | 548 | + | // |
| 516 | - | validateSnapshotBenchmarkFixture(b, fixture, scenario) | 549 | + | // b.Cleanup retains callbacks until the sub-benchmark ends; deferring an |
| 517 | - | generation := uint64(1) | 550 | + | // explicit close avoids retaining every fixture and OOM. |
| 518 | - | if scenario.baseline { | 551 | + | fixture := newPerformanceFixtureWithCleanup(b, performanceConfig{size: scenario.size, panes: 1, historyRows: scenario.historyRows}, false) |
| 552 | + | b.StopTimer() | |||
| 553 | + | defer func() { | |||
| 554 | + | b.StopTimer() | |||
| 555 | + | if err := fixture.close(); err != nil { | |||
| 556 | + | b.Fatalf("close performance fixture: %v", err) | |||
| 557 | + | } | |||
| 558 | + | }() | |||
| 559 | + | validateSnapshotBenchmarkFixture(b, fixture, scenario) | |||
| 560 | + | generation := uint64(1) | |||
| 561 | + | if scenario.baseline { | |||
| 562 | + | publishBenchmarkSnapshot(b, fixture, generation) | |||
| 563 | + | generation++ | |||
| 564 | + | fixture.snapshots.reset() | |||
| 565 | + | } | |||
| 566 | + | b.StartTimer() | |||
| 567 | + | if scenario.mutate != nil { | |||
| 568 | + | scenario.mutate(fixture, operation) | |||
| 569 | + | } | |||
| 519 | publishBenchmarkSnapshot(b, fixture, generation) | 570 | publishBenchmarkSnapshot(b, fixture, generation) | ||
| 520 | - | generation++ | 571 | + | b.StopTimer() |
| 521 | - | fixture.snapshots.reset() | |||
| 522 | - | } | |||
| 523 | - | b.StartTimer() | |||
| 524 | - | if scenario.mutate != nil { | |||
| 525 | - | scenario.mutate(fixture, operation) | |||
| 526 | - | } | |||
| 527 | - | publishBenchmarkSnapshot(b, fixture, generation) | |||
| 528 | - | b.StopTimer() | |||
| 529 | 572 | ||||
| 530 | - | metrics = addCountingSnapshotMetrics(metrics, fixture.snapshots.metrics()) | 573 | + | metrics = addCountingSnapshotMetrics(metrics, fixture.snapshots.metrics()) |
| 531 | - | peakCacheBytes = max(peakCacheBytes, benchmarkSnapshotCacheBytes(fixture.sess)) | 574 | + | peakCacheBytes = max(peakCacheBytes, benchmarkSnapshotCacheBytes(fixture.sess)) |
| 575 | + | }() | |||
| 532 | // B.Loop requires the timer to be running at the next iteration. | 576 | // B.Loop requires the timer to be running at the next iteration. | ||
| 533 | b.StartTimer() | 577 | b.StartTimer() | ||
| 534 | } | 578 | } | ||
| @@ -952,6 +996,7 @@ type performanceFixture struct { | |||||
| 952 | resizedSize domain.Size | 996 | resizedSize domain.Size | ||
| 953 | pty *scriptedPerformancePTY | 997 | pty *scriptedPerformancePTY | ||
| 954 | clock *performanceClock | 998 | clock *performanceClock | ||
| 999 | + | killSession func(*session, uint8, bool) error | |||
| 955 | resizeRequests uint64 | 1000 | resizeRequests uint64 | ||
| 956 | resizeCommits uint64 | 1001 | resizeCommits uint64 | ||
| 957 | resizeCommitNanos uint64 | 1002 | resizeCommitNanos uint64 | ||
| @@ -964,6 +1009,11 @@ type performanceFixture struct { | |||||
| 964 | } | 1009 | } | ||
| 965 | 1010 | ||||
| 966 | func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceFixture { | 1011 | func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceFixture { | ||
| 1012 | + | t.Helper() | |||
| 1013 | + | return newPerformanceFixtureWithCleanup(t, config, true) | |||
| 1014 | + | } | |||
| 1015 | + | ||||
| 1016 | + | func newPerformanceFixtureWithCleanup(t testing.TB, config performanceConfig, registerCleanup bool) *performanceFixture { | |||
| 967 | t.Helper() | 1017 | t.Helper() | ||
| 968 | if !config.size.Valid() { | 1018 | if !config.size.Valid() { | ||
| 969 | config.size = domain.Size{Cols: 80, Rows: 24} | 1019 | config.size = domain.Size{Cols: 80, Rows: 24} | ||
| @@ -980,7 +1030,7 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||||
| 980 | pty := &scriptedPerformancePTY{} | 1030 | pty := &scriptedPerformancePTY{} | ||
| 981 | ptys := make([]ports.PTY, config.tabs) | 1031 | ptys := make([]ports.PTY, config.tabs) | ||
| 982 | ptys[0] = pty | 1032 | ptys[0] = pty | ||
| 983 | - | d, sess, ac, _ := newManualSessionWithPTYs(t, ptys...) | 1033 | + | d, sess, ac, _ := newManualSessionWithPTYsCleanup(t, registerCleanup, ptys...) |
| 984 | clock := &performanceClock{} | 1034 | clock := &performanceClock{} | ||
| 985 | d.clock = clock | 1035 | d.clock = clock | ||
| 986 | output := &countingOutputTransport{} | 1036 | output := &countingOutputTransport{} | ||
| @@ -999,12 +1049,15 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||||
| 999 | snapshots: &countingSnapshotRepository{}, | 1049 | snapshots: &countingSnapshotRepository{}, | ||
| 1000 | pty: pty, | 1050 | pty: pty, | ||
| 1001 | clock: clock, | 1051 | clock: clock, | ||
| 1052 | + | killSession: d.killSession, | |||
| 1002 | liveWrites: [][]byte{[]byte("\x1b[1;1HA\x1b[2;2HA"), []byte("\x1b[1;1HB\x1b[2;2HB")}, | 1053 | liveWrites: [][]byte{[]byte("\x1b[1;1HA\x1b[2;2HA"), []byte("\x1b[1;1HB\x1b[2;2HB")}, | ||
| 1003 | resizeSizes: [2]domain.Size{{Cols: 100, Rows: 30}, config.size}, | 1054 | resizeSizes: [2]domain.Size{{Cols: 100, Rows: 30}, config.size}, | ||
| 1004 | } | 1055 | } | ||
| 1005 | WithSnapshotRepository(fixture.snapshots, nil)(d) | 1056 | WithSnapshotRepository(fixture.snapshots, nil)(d) | ||
| 1006 | d.startSnapshotEncodeWorker() | 1057 | d.startSnapshotEncodeWorker() | ||
| 1007 | - | t.Cleanup(d.stopSnapshotEncodeWorker) | 1058 | + | if registerCleanup { |
| 1059 | + | t.Cleanup(d.stopSnapshotEncodeWorker) | |||
| 1060 | + | } | |||
| 1008 | for tabIndex, tb := range sess.tabs { | 1061 | for tabIndex, tb := range sess.tabs { | ||
| 1009 | fixture.configureTab(tb, tabIndex, config.panes, config.historyRows, tabSize(config.size)) | 1062 | fixture.configureTab(tb, tabIndex, config.panes, config.historyRows, tabSize(config.size)) | ||
| 1010 | } | 1063 | } | ||
| @@ -1029,6 +1082,31 @@ func newPerformanceFixture(t testing.TB, config performanceConfig) *performanceF | |||||
| 1029 | return fixture | 1082 | return fixture | ||
| 1030 | } | 1083 | } | ||
| 1031 | 1084 | ||||
| 1085 | + | func (f *performanceFixture) close() error { | |||
| 1086 | + | if f.d == nil { | |||
| 1087 | + | return nil | |||
| 1088 | + | } | |||
| 1089 | + | d, sess := f.d, f.sess | |||
| 1090 | + | sess.mu.Lock() | |||
| 1091 | + | sess.ephemeral = true | |||
| 1092 | + | sess.mu.Unlock() | |||
| 1093 | + | killErr := f.killSession(sess, ports.ReasonServerShutdown, false) | |||
| 1094 | + | d.stopSnapshotEncodeWorker() | |||
| 1095 | + | d.serveCancel() | |||
| 1096 | + | d.hardCancel() | |||
| 1097 | + | ||||
| 1098 | + | f.d = nil | |||
| 1099 | + | f.sess = nil | |||
| 1100 | + | f.ac = nil | |||
| 1101 | + | f.output = nil | |||
| 1102 | + | f.snapshots = nil | |||
| 1103 | + | f.activePane = nil | |||
| 1104 | + | f.pty = nil | |||
| 1105 | + | f.clock = nil | |||
| 1106 | + | f.killSession = nil | |||
| 1107 | + | return killErr | |||
| 1108 | + | } | |||
| 1109 | + | ||||
| 1032 | func (f *performanceFixture) configureTab(tb *tab, tabIndex, paneCount, historyRows int, size domain.Size) { | 1110 | func (f *performanceFixture) configureTab(tb *tab, tabIndex, paneCount, historyRows int, size domain.Size) { | ||
| 1033 | f.t.Helper() | 1111 | f.t.Helper() | ||
| 1034 | tb.mu.Lock() | 1112 | tb.mu.Lock() | ||