diff --git a/pkg/mlrval/mlrmap.go b/pkg/mlrval/mlrmap.go index a20542c5c..e1ae8632b 100644 --- a/pkg/mlrval/mlrmap.go +++ b/pkg/mlrval/mlrmap.go @@ -63,13 +63,34 @@ func HashRecords(onOff bool) { hashRecords = onOff } +// mlrmapHashThreshold is the field-count at or above which a lazily-hashable +// record builds its key-to-entry index on first lookup. Below this, linear +// search through the (short) linked list is cheaper than allocating and +// populating a map -- and, crucially, records that are never looked up (e.g. +// `mlr cat`) never pay for a map at all. Wide records that do get looked up +// still get hash-accelerated access, preserving the fix for +// https://github.com/johnkerl/miller/issues/1506. +const mlrmapHashThreshold = 12 + type Mlrmap struct { FieldCount int64 Head *MlrmapEntry Tail *MlrmapEntry - // This can be nil if hashRecords is off. + // keysToEntries is the key-to-entry index for hash-accelerated lookups. + // It can be nil in three situations: + // - hashing is disabled entirely (`mlr --no-hash-records`), in which + // case autoHash is false and the index is never built; + // - the map is lazily hashable (autoHash true) but no lookup has yet + // triggered index construction, or the record is narrow enough that + // linear search is preferred; + // - the map is empty. keysToEntries map[string]*MlrmapEntry + + // autoHash, when true, lets findEntry lazily build keysToEntries on the + // first lookup of a sufficiently-wide record. It is false for explicitly + // unhashed maps (`--no-hash-records`). + autoHash bool } type MlrmapEntry struct { @@ -94,7 +115,7 @@ type MlrmapPair struct { func NewMlrmapAsRecord() *Mlrmap { if hashRecords { - return newMlrmapHashed() + return newMlrmapLazyHashed() } return newMlrmapUnhashed() } @@ -102,6 +123,22 @@ func NewMlrmap() *Mlrmap { return newMlrmapHashed() } +// newMlrmapLazyHashed is the default for record-stream data. It allocates no +// key-to-entry index up front; findEntry builds one on demand only when a +// lookup occurs on a wide record (see mlrmapHashThreshold). This avoids a map +// allocation and N map-inserts per record for the common case of streaming +// over many narrow records, while retaining hash-accelerated lookups for wide +// records that are actually queried. +func newMlrmapLazyHashed() *Mlrmap { + return &Mlrmap{ + FieldCount: 0, + Head: nil, + Tail: nil, + keysToEntries: nil, + autoHash: true, + } +} + // Faster on record-stream data as noted above. func newMlrmapUnhashed() *Mlrmap { return &Mlrmap{ diff --git a/pkg/mlrval/mlrmap_accessors.go b/pkg/mlrval/mlrmap_accessors.go index 70e9b0977..6112d83b8 100644 --- a/pkg/mlrval/mlrmap_accessors.go +++ b/pkg/mlrval/mlrmap_accessors.go @@ -194,6 +194,14 @@ func (mlrmap *Mlrmap) findEntry(key string) *MlrmapEntry { if mlrmap.keysToEntries != nil { return mlrmap.keysToEntries[key] } + // Lazily build the key-to-entry index when a lookup happens on a record + // wide enough to benefit. Narrow records (the common case) and records + // that are never looked up stay on the linear-search path and never pay + // for a map. See mlrmapHashThreshold. + if mlrmap.autoHash && mlrmap.FieldCount >= mlrmapHashThreshold { + mlrmap.buildIndex() + return mlrmap.keysToEntries[key] + } for pe := mlrmap.Head; pe != nil; pe = pe.Next { if pe.Key == key { return pe @@ -202,6 +210,20 @@ func (mlrmap *Mlrmap) findEntry(key string) *MlrmapEntry { return nil } +// buildIndex populates keysToEntries from the linked list. Once built, all +// mutators keep it in sync (each guards on keysToEntries != nil). On duplicate +// keys the last entry wins, matching the linear-search semantics of findEntry +// (which returns the first match), since records are deduped on insert. +func (mlrmap *Mlrmap) buildIndex() { + m := make(map[string]*MlrmapEntry, mlrmap.FieldCount) + for pe := mlrmap.Head; pe != nil; pe = pe.Next { + if _, ok := m[pe.Key]; !ok { + m[pe.Key] = pe + } + } + mlrmap.keysToEntries = m +} + // findEntryByPositionalIndex is for '$[1]' etc. in the DSL. // // Notes: @@ -459,7 +481,14 @@ func (mlrmap *Mlrmap) Clear() { } func (mlrmap *Mlrmap) Copy() *Mlrmap { - other := NewMlrmapMaybeHashed(mlrmap.isHashed()) + var other *Mlrmap + if mlrmap.autoHash { + // Preserve lazy-hashing semantics: don't force an eager index on the + // copy just because the source happens to have built one. + other = newMlrmapLazyHashed() + } else { + other = NewMlrmapMaybeHashed(mlrmap.isHashed()) + } for pe := mlrmap.Head; pe != nil; pe = pe.Next { other.PutCopy(pe.Key, pe.Value) }