Skip to content

perf(store): measure scoped-list allocation and avoid unnecessary artifact cloning #2337

Description

@DeliciousBuding

Summary

Follow the real memory-read path identified in #2333/#2335 instead of changing SQLite's SQL pool. filterOrdered and listClonedArtifacts currently allocate result capacity from the entire resident order; listClonedArtifacts also clones content-source metadata before deciding whether a row matches the requested run. Validate the actual cost, including dense/unfiltered controls, before changing either behavior.

Scope / File Ownership

One local writer on a worktree from current origin/master:

  • edge-server/internal/store/store_query_bench_test.go: repeatable real Store.ListRuns / Store.ListArtifacts benchmarks.
  • edge-server/internal/store/store_query.go: narrowly measured allocation/clone changes only, if justified; no new indexes, pools or persistence state.
  • edge-server/internal/store/store_query_test.go: reuse/extend existing selection, insertion-order and deep-copy ownership tests if needed.
  • A temporary edge-server/internal/store/scoped_list_experiment_test.go may compare allocation strategies, but is not a permanent parallel implementation. Save its relevant evidence on this issue and remove it before the delivery commit.

Owner: docs/architecture/02-edge-server.md (regular reads delegate to the memory Store; no SQL readers or runtime/pool change).

Workload / Interfaces / Invariants

  • Generated fixtures with 100 / 1,000 resident records. A sparse scope has ten records; the dense scope has the remaining records. Also measure a missing scope and the unfiltered empty scope.
  • Invoke real Store facade methods, record per-call time/allocated bytes/allocation count, and validate complete ordered IDs before/after timing. Artifacts carry generated content-source metadata, not file contents read from a workspace.
  • Preserve empty-scope semantics, non-nil empty results, insertion order, and detached returned artifact ContentSource values. Do not confuse nil-slice JSON behavior with an allocation improvement.
  • Do not optimize sparse cases by silently regressing dense/all-record cases. Quantify candidate tradeoffs; a rejected experiment is an acceptable documented decision, not a reason to keep extra implementation code.

Acceptance

  1. Baseline samples against current production code, including sparse/dense/missing/all controls and the observed clone allocations; no bottleneck claim based only on source inspection.
  2. A production change only after a justified narrow improvement. Matching before/after measurement fixture, non-race runs, declared environment, repetitions and raw samples saved here.
  3. Existing behavior/ownership tests plus any necessary new regression checks pass. Full store-package Linux race, Windows tests, vet/staticcheck and all required PR gates pass.
  4. Remove temporary experimental implementations, retain only a useful reproducible benchmark and justified code change, sync affected documentation, then clean the merged worktree/branch.

Negative Constraints / Dependencies

No API/schema/SQL/pool/debounce changes, production data, deployment, user-workspace reads, new CI jobs, timing thresholds, RSS or production-throughput claims. #2256 is already closed for its original four candidates; this is a new measured follow-up, not a reopened duplicate backlog. EventLog #2304 and upstream vulnerability #1663 remain separate.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions