Skip to content

Occurrences are fully materialized before search begins (analysis: not fixable without changing output semantics) #20

Description

@hammadmajid

Recorded from the concurrency audit for the record. Not scheduled — it needs a product decision, not a defect fix.

Observation

cmd/grg/main.go:107-113

var occurrences []model.BlobOccurrence
err = walker.Walk(func(occ model.BlobOccurrence) error {
    ...
    occurrences = append(occurrences, occ)

The entire history is materialized into one slice — each model.BlobOccurrence retaining a *model.CommitMetadata — before a single byte is searched. Memory scales with history size, and it lengthens the window in which cancellation latency scales with repository size.

Why the obvious fix does not work

Streaming occurrences into Pipeline.ExecuteContext would require dispatching a blob as soon as it is first seen. But ExecuteContext deduplicates by blob OID and attaches every occurrence of that OID to the resulting BlobResult (internal/search/pipeline.go:90-105), and the aggregator's selectOccurrences (internal/aggregator/aggregator.go:283-313) collapses those to the introducing commit unless --expand-commits is set.

A blob's full occurrence set is not known until the walk completes. Dispatching early would emit a result carrying only the occurrences seen so far, which changes what grg reports for the introducing commit. That is an output-semantics change, not an internal refactor.

Options, if this is ever worth doing

  1. Two-phase with a compact intermediate. Keep the full walk but store (blobOID, commitSHA, path) triples with interned commit metadata instead of retaining *CommitMetadata per occurrence. Cuts memory substantially, changes no semantics.
  2. Stream with deferred provenance. Dispatch on first sight, then attach late occurrences to an already-produced result before rendering. Requires the aggregator to hold results until the walk finishes, so peak memory moves rather than shrinks.
  3. Accept a semantics change under an opt-in flag: report the first-seen occurrence only, and stream end to end.

Option 1 is the only one that is purely a win. Someone needs to decide whether the memory pressure is real on the repositories people actually use grg on before any of this is worth building.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions