Skip to content
Open
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
161 changes: 161 additions & 0 deletions pkg/runtime/persistence_observer_bench_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package runtime

import (
"context"
"database/sql"
"sync/atomic"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
_ "modernc.org/sqlite"

"github.com/docker/docker-agent/pkg/chat"
"github.com/docker/docker-agent/pkg/session"
)

// streamingBenchChunks is the number of AgentChoice deltas exercised per
// benchmark iteration — roughly the order of magnitude of a long assistant turn.
const streamingBenchChunks = 500

// countingStore wraps an in-memory store and records AddMessage / UpdateMessage
// calls so tests can pin the per-chunk persistence contract.
type countingStore struct {
*session.InMemorySessionStore

Check failure on line 24 in pkg/runtime/persistence_observer_bench_test.go

View workflow job for this annotation

GitHub Actions / lint

there must be an empty line separating embedded fields from regular fields (embeddedstructfieldcheck)
addCalls atomic.Int64
updateCalls atomic.Int64
}

func newCountingStore() *countingStore {
return &countingStore{
InMemorySessionStore: session.NewInMemorySessionStore().(*session.InMemorySessionStore),
}
}

func (s *countingStore) AddMessage(ctx context.Context, sessionID string, msg *session.Message) (int64, error) {
s.addCalls.Add(1)
return s.InMemorySessionStore.AddMessage(ctx, sessionID, msg)
}

func (s *countingStore) UpdateMessage(ctx context.Context, messageID int64, msg *session.Message) error {
s.updateCalls.Add(1)
return s.InMemorySessionStore.UpdateMessage(ctx, messageID, msg)
}

func setupPersistenceObserverBench(tb testing.TB) (*PersistenceObserver, *session.Session, *session.InMemorySessionStore) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the third return value is discarded by the only caller. With the bounded-store fix suggested in the other comment, the session return becomes the unused one instead. Returning (obs, store) and letting benchmarks create their own sessions would keep the helper minimal.

tb.Helper()
store := session.NewInMemorySessionStore().(*session.InMemorySessionStore)
obs := newPersistenceObserver(store)
require.NotNil(tb, obs)

sess := session.New(session.WithID("bench-session"), session.WithUserMessage("hi"))
require.NoError(tb, store.AddSession(context.Background(), sess))

Check failure on line 52 in pkg/runtime/persistence_observer_bench_test.go

View workflow job for this annotation

GitHub Actions / lint

use of `context.Background` forbidden because "do not use context.Background() in tests, use t.Context()" (forbidigo)
return obs, sess, store
}

func emitStreamingChunks(ctx context.Context, obs *PersistenceObserver, sess *session.Session, chunks int) {
for range chunks {
obs.OnEvent(ctx, sess, AgentChoice("root", sess.ID, "tok"))
}
}

func finalizeStreamingMessage(ctx context.Context, obs *PersistenceObserver, sess *session.Session) {
obs.OnEvent(ctx, sess, MessageAdded(sess.ID, &session.Message{
AgentName: "root",
Message: chat.Message{
Role: chat.MessageRoleAssistant,
Content: "done",
},
}, "root"))
}

// TestPersistenceObserver_UpdateCountPerChunk documents that each streaming
// delta triggers a store write: one AddMessage on the first chunk, then one
// UpdateMessage per subsequent chunk until MessageAddedEvent finalises the row.
func TestPersistenceObserver_UpdateCountPerChunk(t *testing.T) {
t.Parallel()

const chunks = 100
ctx := t.Context()

store := newCountingStore()
obs := newPersistenceObserver(store)
require.NotNil(t, obs)

sess := session.New(session.WithID("s1"), session.WithUserMessage("hi"))
require.NoError(t, store.AddSession(ctx, sess))

emitStreamingChunks(ctx, obs, sess, chunks)

assert.Equal(t, int64(1), store.addCalls.Load(), "first chunk should INSERT")
assert.Equal(t, int64(chunks-1), store.updateCalls.Load(), "each later chunk should UPDATE")

finalizeStreamingMessage(ctx, obs, sess)
// MessageAdded with an existing streaming row issues one more UpdateMessage.
assert.Equal(t, int64(chunks), store.updateCalls.Load())
}

// TestPersistenceObserver_StreamingContentAccumulates verifies mid-stream
// persistence keeps the growing assistant text in the store.
func TestPersistenceObserver_StreamingContentAccumulates(t *testing.T) {
t.Parallel()

ctx := t.Context()
store := session.NewInMemorySessionStore()
obs := newPersistenceObserver(store)
require.NotNil(t, obs)

sess := session.New(session.WithID("s1"), session.WithUserMessage("hi"))
require.NoError(t, store.AddSession(ctx, sess))

obs.OnEvent(ctx, sess, AgentChoice("root", sess.ID, "hel"))
obs.OnEvent(ctx, sess, AgentChoice("root", sess.ID, "lo"))

reloaded, err := store.GetSession(ctx, sess.ID)
require.NoError(t, err)
require.Len(t, reloaded.Messages, 2) // user + streaming assistant
require.NotNil(t, reloaded.Messages[1].Message)
assert.Equal(t, "hello", reloaded.Messages[1].Message.Message.Content)
}

func BenchmarkPersistenceObserver_StreamingChunks(b *testing.B) {
ctx := b.Context()
obs, sess, _ := setupPersistenceObserverBench(b)

b.ReportAllocs()
b.ResetTimer()
for range b.N {
emitStreamingChunks(ctx, obs, sess, streamingBenchChunks)
finalizeStreamingMessage(ctx, obs, sess)
}
Comment on lines +127 to +130

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shared session makes ns/op a function of b.N: each iteration leaves one extra message row in bench-session, and InMemorySessionStore.UpdateMessage scans every message of every session on each call, so later iterations pay O(iterations) per chunk.

Measured on this branch (darwin/arm64):

benchtime ns/op
200x 195,198
2000x 624,997
8000x 2,398,435

The PR's headline number (~2.0 ms/op) is therefore an artifact of the iteration count picked by the harness, not a baseline that future optimizations can be compared against. The "in-memory drift" caveat in the description understates this: the number is not noisy, it is unbounded.

Keeping the store bounded flattens the result (~175 µs/op at 200x, 2000x and 8000x, verified):

for i := range b.N {
    sess := session.New(session.WithID(strconv.Itoa(i)), session.WithUserMessage("hi"))
    if err := store.AddSession(ctx, sess); err != nil {
        b.Fatal(err)
    }
    emitStreamingChunks(ctx, obs, sess, streamingBenchChunks)
    finalizeStreamingMessage(ctx, obs, sess)
    if err := store.DeleteSession(ctx, sess.ID); err != nil {
        b.Fatal(err)
    }
}

Requires the strconv import and the store returned by setupPersistenceObserverBench. Note that a fresh session per iteration alone is not enough: UpdateMessage ranges over all sessions, so the store must not accumulate them, hence the DeleteSession. If per-iteration setup cost is a concern, it is ~2 allocations against ~2,500 per iteration, so it does not move the numbers.

}

func BenchmarkPersistenceObserver_StreamingChunks_SQLite(b *testing.B) {
ctx := b.Context()
store := openBenchSQLiteStore(b)
obs := newPersistenceObserver(store)
require.NotNil(b, obs)

sess := session.New(session.WithID("bench-sqlite"), session.WithUserMessage("hi"))
require.NoError(b, store.AddSession(ctx, sess))

b.ReportAllocs()
b.ResetTimer()
for range b.N {
emitStreamingChunks(ctx, obs, sess, streamingBenchChunks)
finalizeStreamingMessage(ctx, obs, sess)
}
}

func openBenchSQLiteStore(b *testing.B) *session.SQLiteSessionStore {
b.Helper()
db, err := sql.Open("sqlite", ":memory:")
require.NoError(b, err)
b.Cleanup(func() { _ = db.Close() })
db.SetMaxOpenConns(1)

store, err := session.NewSQLiteSessionStoreFromDB(b.Context(), db)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: migration INFO logs are emitted on every harness re-invocation of the benchmark function, interleaving with the -bench output. Not counted in ns/op since setup runs before b.ResetTimer, but redirecting the default slog handler to io.Discard for the benchmark would keep the output clean for tools that parse it.

require.NoError(b, err)
b.Cleanup(func() { _ = store.Close() })
return store
}
Loading