From d65e8837aa7bb1a6abb6892b9f2664e1b7edb02e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Thu, 22 Dec 2022 00:48:00 +0100 Subject: [PATCH] cache: generic withSnapshot, some cleanup --- api/graphql/graphql_test.go | 5 ++- cache/bug_cache.go | 2 +- cache/cached.go | 47 +------------------------ cache/repo_cache_test.go | 2 +- cache/subcache.go | 7 ++-- cache/with_snapshot.go | 56 ++++++++++++++++++++++++++++++ commands/execenv/env.go | 8 ++--- commands/webui.go | 16 ++++++++- entities/bug/snapshot.go | 4 +++ entities/bug/with_snapshot.go | 53 ---------------------------- entity/dag/op_set_metadata_test.go | 6 ++++ entity/dag/operation.go | 2 ++ 12 files changed, 98 insertions(+), 110 deletions(-) create mode 100644 cache/with_snapshot.go delete mode 100644 entities/bug/with_snapshot.go diff --git a/api/graphql/graphql_test.go b/api/graphql/graphql_test.go index e28ce8ab90c88744b3632b5ac80c224e2605ab87..a8dfad3f1825a33426bb4775539d33e07b46c240 100644 --- a/api/graphql/graphql_test.go +++ b/api/graphql/graphql_test.go @@ -19,8 +19,11 @@ func TestQueries(t *testing.T) { random_bugs.FillRepoWithSeed(repo, 10, 42) mrc := cache.NewMultiRepoCache() - _, _, err := mrc.RegisterDefaultRepository(repo) + _, events, err := mrc.RegisterDefaultRepository(repo) require.NoError(t, err) + for event := range events { + require.NoError(t, event.Err) + } handler := NewHandler(mrc, nil) diff --git a/cache/bug_cache.go b/cache/bug_cache.go index 7b3fa114764e4fab7099abd8c8db3d9258c339b4..3466f18626953b6547e03f28fa57f567a9f7cd0c 100644 --- a/cache/bug_cache.go +++ b/cache/bug_cache.go @@ -28,7 +28,7 @@ func NewBugCache(b *bug.Bug, repo repository.ClockedRepo, getUserIdentity getUse repo: repo, entityUpdated: entityUpdated, getUserIdentity: getUserIdentity, - entity: &bug.WithSnapshot{Bug: b}, + entity: &withSnapshot[*bug.Snapshot, bug.Operation]{Interface: b}, }, } } diff --git a/cache/cached.go b/cache/cached.go index 5a31ee59bea25e2c5a678c7a66708a96ff9048ee..ce1d66372062b162f3aad7a98bd0ddcdd294af5c 100644 --- a/cache/cached.go +++ b/cache/cached.go @@ -9,52 +9,7 @@ import ( "github.com/MichaelMure/git-bug/util/lamport" ) -// type withSnapshot[SnapT dag.Snapshot, OpT dag.OperationWithApply[SnapT]] struct { -// dag.Interface[SnapT, OpT] -// snap dag.Snapshot -// } -// -// -// func (ws *withSnapshot[SnapT, OpT]) Compile() dag.Snapshot { -// if ws.snap == nil { -// snap := ws.Interface.Compile() -// ws.snap = snap -// } -// return ws.snap -// } -// -// // Append intercept Bug.Append() to update the snapshot efficiently -// func (ws *withSnapshot[SnapT, OpT]) Append(op OpT) { -// ws.Interface.Append(op) -// -// if ws.snap == nil { -// return -// } -// -// op.Apply(ws.snap) -// ws.snap. = append(ws.snap.Operations, op) -// } -// -// // Commit intercept Bug.Commit() to update the snapshot efficiently -// func (ws *withSnapshot[SnapT, OpT]) Commit(repo repository.ClockedRepo) error { -// err := ws.Interface.Commit(repo) -// -// if err != nil { -// ws.snap = nil -// return err -// } -// -// // Commit() shouldn't change anything of the bug state apart from the -// // initial ID set -// -// if ws.snap == nil { -// return nil -// } -// -// ws.snap.id = ws.Interface.Id() -// return nil -// } - +// CachedEntityBase provide the base function of an entity managed by the cache. type CachedEntityBase[SnapT dag.Snapshot, OpT dag.Operation] struct { repo repository.ClockedRepo entityUpdated func(id entity.Id) error diff --git a/cache/repo_cache_test.go b/cache/repo_cache_test.go index 395872f7621af2eabde7d79434461c9348636db1..939cb1544939e82cfcd1455f4772cbc10a9a6e03 100644 --- a/cache/repo_cache_test.go +++ b/cache/repo_cache_test.go @@ -98,7 +98,7 @@ func TestCache(t *testing.T) { cache, events, err = NewRepoCache(repo) noBuildEventErrors(t, events) require.NoError(t, err) - require.Empty(t, cache.bugs.cached) + require.Len(t, cache.bugs.cached, 0) require.Len(t, cache.bugs.excerpts, 2) require.Len(t, cache.identities.cached, 0) require.Len(t, cache.identities.excerpts, 2) diff --git a/cache/subcache.go b/cache/subcache.go index 0678988a6d93e0c93c9b8d436be3be906aea6e28..af75938a97f473ef535d912535a5df27ed2f6b67 100644 --- a/cache/subcache.go +++ b/cache/subcache.go @@ -198,9 +198,10 @@ func (sc *SubCache[EntityT, ExcerptT, CacheT]) Build() error { return e.Err } - // TODO: doesn't actually record in cache, should we? cached := sc.makeCached(e.Entity, sc.entityUpdated) sc.excerpts[e.Entity.Id()] = sc.makeExcerpt(cached) + // might as well keep them in memory + sc.cached[e.Entity.Id()] = cached indexData := sc.makeIndexData(cached) if err := indexer(e.Entity.Id().String(), indexData); err != nil { @@ -417,12 +418,12 @@ func (sc *SubCache[EntityT, ExcerptT, CacheT]) MergeAll(remote string) <-chan en switch result.Status { case entity.MergeStatusNew, entity.MergeStatusUpdated: e := result.Entity.(EntityT) - - // TODO: doesn't actually record in cache, should we? cached := sc.makeCached(e, sc.entityUpdated) sc.mu.Lock() sc.excerpts[result.Id] = sc.makeExcerpt(cached) + // might as well keep them in memory + sc.cached[result.Id] = cached sc.mu.Unlock() } } diff --git a/cache/with_snapshot.go b/cache/with_snapshot.go new file mode 100644 index 0000000000000000000000000000000000000000..674b6923b0f78ea3c3c23c28966fcea8d554423e --- /dev/null +++ b/cache/with_snapshot.go @@ -0,0 +1,56 @@ +package cache + +import ( + "sync" + + "github.com/MichaelMure/git-bug/entity/dag" + "github.com/MichaelMure/git-bug/repository" +) + +var _ dag.Interface[dag.Snapshot, dag.OperationWithApply[dag.Snapshot]] = &withSnapshot[dag.Snapshot, dag.OperationWithApply[dag.Snapshot]]{} + +// withSnapshot encapsulate an entity and maintain a snapshot efficiently. +type withSnapshot[SnapT dag.Snapshot, OpT dag.OperationWithApply[SnapT]] struct { + dag.Interface[SnapT, OpT] + mu sync.Mutex + snap *SnapT +} + +func (ws *withSnapshot[SnapT, OpT]) Compile() SnapT { + ws.mu.Lock() + defer ws.mu.Unlock() + if ws.snap == nil { + snap := ws.Interface.Compile() + ws.snap = &snap + } + return *ws.snap +} + +// Append intercept Bug.Append() to update the snapshot efficiently +func (ws *withSnapshot[SnapT, OpT]) Append(op OpT) { + ws.mu.Lock() + defer ws.mu.Unlock() + + ws.Interface.Append(op) + + if ws.snap == nil { + return + } + + op.Apply(*ws.snap) + (*ws.snap).AppendOperation(op) +} + +// Commit intercept Bug.Commit() to update the snapshot efficiently +func (ws *withSnapshot[SnapT, OpT]) Commit(repo repository.ClockedRepo) error { + ws.mu.Lock() + defer ws.mu.Unlock() + + err := ws.Interface.Commit(repo) + if err != nil { + ws.snap = nil + return err + } + + return nil +} diff --git a/commands/execenv/env.go b/commands/execenv/env.go index 4b353279024bf7a0a15d64e290bb0a7287085b15..0813ab7e320911792f18cdf672eea150529d29e0 100644 --- a/commands/execenv/env.go +++ b/commands/execenv/env.go @@ -135,20 +135,20 @@ func LoadBackend(env *Env) func(*cobra.Command, []string) error { } if events != nil { - _, _ = fmt.Fprintln(os.Stderr, "Building cache... ") + env.Err.Println("Building cache... ") } for event := range events { if event.Err != nil { - _, _ = fmt.Fprintf(os.Stderr, "Cache building error [%s]: %v\n", event.Typename, event.Err) + env.Err.Printf("Cache building error [%s]: %v\n", event.Typename, event.Err) continue } switch event.Event { case cache.BuildEventStarted: - _, _ = fmt.Fprintf(os.Stderr, "[%s] started\n", event.Typename) + env.Err.Printf("[%s] started\n", event.Typename) case cache.BuildEventFinished: - _, _ = fmt.Fprintf(os.Stderr, "[%s] done\n", event.Typename) + env.Err.Printf("[%s] done\n", event.Typename) } } diff --git a/commands/webui.go b/commands/webui.go index e1e0fc2bc4aa05ef2725b95759d0d6e27484f8a3..4f7a02b5f8aa0436cfa78b6e6bb347eb4ab8ea54 100644 --- a/commands/webui.go +++ b/commands/webui.go @@ -105,11 +105,25 @@ func runWebUI(env *execenv.Env, opts webUIOptions) error { } mrc := cache.NewMultiRepoCache() - _, _, err := mrc.RegisterDefaultRepository(env.Repo) + _, events, err := mrc.RegisterDefaultRepository(env.Repo) if err != nil { return err } + for event := range events { + if event.Err != nil { + env.Err.Printf("Cache building error [%s]: %v\n", event.Typename, event.Err) + continue + } + + switch event.Event { + case cache.BuildEventStarted: + env.Err.Printf("[%s] started\n", event.Typename) + case cache.BuildEventFinished: + env.Err.Printf("[%s] done\n", event.Typename) + } + } + var errOut io.Writer if opts.logErrors { errOut = env.Err diff --git a/entities/bug/snapshot.go b/entities/bug/snapshot.go index 333fe20769d2e29b01b90dd939546eec12e50f61..5c260d85f87099447810ae30e41cb379d86e3937 100644 --- a/entities/bug/snapshot.go +++ b/entities/bug/snapshot.go @@ -43,6 +43,10 @@ func (snap *Snapshot) AllOperations() []dag.Operation { return snap.Operations } +func (snap *Snapshot) AppendOperation(op dag.Operation) { + snap.Operations = append(snap.Operations, op) +} + // EditTime returns the last time a bug was modified func (snap *Snapshot) EditTime() time.Time { if len(snap.Operations) == 0 { diff --git a/entities/bug/with_snapshot.go b/entities/bug/with_snapshot.go deleted file mode 100644 index 0474cac728765a4adb990608387f5a456ed2d90a..0000000000000000000000000000000000000000 --- a/entities/bug/with_snapshot.go +++ /dev/null @@ -1,53 +0,0 @@ -package bug - -import ( - "github.com/MichaelMure/git-bug/repository" -) - -var _ Interface = &WithSnapshot{} - -// WithSnapshot encapsulate a Bug and maintain the corresponding Snapshot efficiently -type WithSnapshot struct { - *Bug - snap *Snapshot -} - -func (b *WithSnapshot) Compile() *Snapshot { - if b.snap == nil { - snap := b.Bug.Compile() - b.snap = snap - } - return b.snap -} - -// Append intercept Bug.Append() to update the snapshot efficiently -func (b *WithSnapshot) Append(op Operation) { - b.Bug.Append(op) - - if b.snap == nil { - return - } - - op.Apply(b.snap) - b.snap.Operations = append(b.snap.Operations, op) -} - -// Commit intercept Bug.Commit() to update the snapshot efficiently -func (b *WithSnapshot) Commit(repo repository.ClockedRepo) error { - err := b.Bug.Commit(repo) - - if err != nil { - b.snap = nil - return err - } - - // Commit() shouldn't change anything of the bug state apart from the - // initial ID set - - if b.snap == nil { - return nil - } - - b.snap.id = b.Bug.Id() - return nil -} diff --git a/entity/dag/op_set_metadata_test.go b/entity/dag/op_set_metadata_test.go index f4f20e8e8521481d25b0a1c081017b9dc2beb88d..07ece013fd07460e9c816dc8275dfe0b795861fd 100644 --- a/entity/dag/op_set_metadata_test.go +++ b/entity/dag/op_set_metadata_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/require" ) +var _ Snapshot = &snapshotMock{} + type snapshotMock struct { ops []Operation } @@ -20,6 +22,10 @@ func (s *snapshotMock) AllOperations() []Operation { return s.ops } +func (s *snapshotMock) AppendOperation(op Operation) { + s.ops = append(s.ops, op) +} + func TestSetMetadata(t *testing.T) { snap := &snapshotMock{} diff --git a/entity/dag/operation.go b/entity/dag/operation.go index 1b891aeb38d5fe6ceac111a2b4255e3e5579a339..f50d91b65d43dfc197b56104bd6ba6f298b58699 100644 --- a/entity/dag/operation.go +++ b/entity/dag/operation.go @@ -90,6 +90,8 @@ type OperationDoesntChangeSnapshot interface { type Snapshot interface { // AllOperations returns all the operations that have been applied to that snapshot, in order AllOperations() []Operation + // AppendOperation add an operation in the list + AppendOperation(op Operation) } // OpBase implement the common feature that every Operation should support.