mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/memoize: delete Bind(cleanup) hook 

Now that the workspace directory uses Snapshot.Destroy
to clean up (see https://go-review.googlesource.com/c/tools/+/414499)
there is no need for this feature.

Change-Id: Id5782273ce5030b4fb8f3b66a8d16a45a831ed91
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414500
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
This commit is contained in:
Alan Donovan 2022-07-01 10:35:27 -04:00 committed by Gopher Robot
parent bec0cf16be
commit f042799df4
7 changed files with 27 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/lsp/cache/analysis.go vendored
View File

@ -147,7 +147,7 @@ func (s *snapshot) actionHandle(ctx context.Context, id PackageID, a *analysis.A
}
}
return runAnalysis(ctx, snapshot, a, pkg, results)
}, nil)
})
act.handle = h
act = s.addActionHandle(act)

6
internal/lsp/cache/mod.go vendored
View File

@ -88,7 +88,7 @@ func (s *snapshot) ParseMod(ctx context.Context, modFH source.FileHandle) (*sour
},
err: parseErr,
}
}, nil)
})
pmh := &parseModHandle{handle: h}
s.mu.Lock()
@ -162,7 +162,7 @@ func (s *snapshot) ParseWork(ctx context.Context, modFH source.FileHandle) (*sou
},
err: parseErr,
}
}, nil)
})
pwh := &parseWorkHandle{handle: h}
s.mu.Lock()
@ -288,7 +288,7 @@ func (s *snapshot) ModWhy(ctx context.Context, fh source.FileHandle) (map[string
why[req.Mod.Path] = whyList[i]
}
return &modWhyData{why: why}
}, nil)
})
mwh := &modWhyHandle{handle: h}
s.mu.Lock()

2
internal/lsp/cache/mod_tidy.go vendored
View File

@ -138,7 +138,7 @@ func (s *snapshot) ModTidy(ctx context.Context, pm *source.ParsedModule) (*sourc
TidiedContent: tempContents,
},
}
}, nil)
})
mth := &modTidyHandle{handle: h}
s.mu.Lock()

2
internal/lsp/cache/parse.go vendored
View File

@ -130,7 +130,7 @@ func (s *snapshot) astCacheData(ctx context.Context, spkg source.Package, pos to
}
astHandle := s.generation.Bind(astCacheKey{pkgHandle.key, pgf.URI}, func(ctx context.Context, arg memoize.Arg) interface{} {
return buildASTCache(pgf)
}, nil)
})
d, err := astHandle.Get(ctx, s.generation, s)
if err != nil {

2
internal/lsp/cache/symbols.go vendored
View File

@ -41,7 +41,7 @@ func (s *snapshot) buildSymbolHandle(ctx context.Context, fh source.FileHandle)
snapshot := arg.(*snapshot)
symbols, err := symbolize(snapshot, fh)
return &symbolData{symbols, err}
}, nil)
})
sh := &symbolHandle{
handle: handle,

67
internal/memoize/memoize.go
View File

@ -34,6 +34,7 @@ type Store struct {
handlesMu sync.Mutex // lock ordering: Store.handlesMu before Handle.mu
handles map[interface{}]*Handle
// handles which are bound to generations for GC purposes.
// (It is the subset of values of 'handles' with trackGenerations enabled.)
boundHandles map[*Handle]struct{}
}
@ -78,7 +79,11 @@ func (g *Generation) Destroy(destroyedBy string) {
if _, ok := h.generations[g]; ok {
delete(h.generations, g) // delete even if it's dead, in case of dangling references to the entry.
if len(h.generations) == 0 {
h.destroy(g.store)
h.state = stateDestroyed
delete(g.store.handles, h.key)
if h.trackGenerations {
delete(g.store.boundHandles, h)
}
}
}
h.mu.Unlock()
@ -155,14 +160,6 @@ type Handle struct {
function Function
// value is set in completed state.
value interface{}
// cleanup, if non-nil, is used to perform any necessary clean-up on values
// produced by function.
//
// cleanup is never set for reference counted handles.
//
// TODO(rfindley): remove this field once workspace folders no longer need to
// be tracked.
cleanup func(interface{})
// If trackGenerations is set, this handle tracks generations in which it
// is valid, via the generations field. Otherwise, it is explicitly reference
@ -171,7 +168,7 @@ type Handle struct {
refCounter int32
}
// Bind returns a handle for the given key and function.
// Bind returns a "generational" handle for the given key and function.
//
// Each call to bind will return the same handle if it is already bound. Bind
// will always return a valid handle, creating one if needed. Each key can
@ -179,22 +176,18 @@ type Handle struct {
// until the associated generation is destroyed. Bind does not cause the value
// to be generated.
//
// If cleanup is non-nil, it will be called on any non-nil values produced by
// function when they are no longer referenced.
//
// It is responsibility of the caller to call Inherit on the handler whenever
// it should still be accessible by a next generation.
func (g *Generation) Bind(key interface{}, function Function, cleanup func(interface{})) *Handle {
return g.getHandle(key, function, cleanup, true)
func (g *Generation) Bind(key interface{}, function Function) *Handle {
return g.getHandle(key, function, true)
}
// GetHandle returns a handle for the given key and function with similar
// properties and behavior as Bind.
//
// As in opposite to Bind it returns a release callback which has to be called
// once this reference to handle is not needed anymore.
// GetHandle returns a "reference-counted" handle for the given key
// and function with similar properties and behavior as Bind. Unlike
// Bind, it returns a release callback which must be called once the
// handle is no longer needed.
func (g *Generation) GetHandle(key interface{}, function Function) (*Handle, func()) {
h := g.getHandle(key, function, nil, false)
h := g.getHandle(key, function, false)
store := g.store
release := func() {
// Acquire store.handlesMu before mutating refCounter
@ -206,7 +199,7 @@ func (g *Generation) GetHandle(key interface{}, function Function) (*Handle, fun
h.refCounter--
if h.refCounter == 0 {
// Don't call h.destroy: for reference counted handles we can't know when
// Don't mark destroyed: for reference counted handles we can't know when
// they are no longer reachable from runnable goroutines. For example,
// gopls could have a current operation that is using a packageHandle.
// Destroying the handle here would cause that operation to hang.
@ -216,7 +209,7 @@ func (g *Generation) GetHandle(key interface{}, function Function) (*Handle, fun
return h, release
}
func (g *Generation) getHandle(key interface{}, function Function, cleanup func(interface{}), trackGenerations bool) *Handle {
func (g *Generation) getHandle(key interface{}, function Function, trackGenerations bool) *Handle {
// panic early if the function is nil
// it would panic later anyway, but in a way that was much harder to debug
if function == nil {
@ -232,7 +225,6 @@ func (g *Generation) getHandle(key interface{}, function Function, cleanup func(
h = &Handle{
key: key,
function: function,
cleanup: cleanup,
trackGenerations: trackGenerations,
}
if trackGenerations {
@ -298,18 +290,6 @@ func (g *Generation) Inherit(h *Handle) {
h.incrementRef(g)
}
// destroy marks h as destroyed. h.mu and store.handlesMu must be held.
func (h *Handle) destroy(store *Store) {
h.state = stateDestroyed
if h.cleanup != nil && h.value != nil {
h.cleanup(h.value)
}
delete(store.handles, h.key)
if h.trackGenerations {
delete(store.boundHandles, h)
}
}
func (h *Handle) incrementRef(g *Generation) {
h.mu.Lock()
defer h.mu.Unlock()
@ -412,11 +392,6 @@ func (h *Handle) run(ctx context.Context, g *Generation, arg Arg) (interface{},
}
v := function(childCtx, arg)
if childCtx.Err() != nil {
// It's possible that v was computed despite the context cancellation. In
// this case we should ensure that it is cleaned up.
if h.cleanup != nil && v != nil {
h.cleanup(v)
}
return
}
@ -427,19 +402,9 @@ func (h *Handle) run(ctx context.Context, g *Generation, arg Arg) (interface{},
// checked childCtx above. Even so, that should be harmless, since each
// run should produce the same results.
if h.state != stateRunning {
// v will never be used, so ensure that it is cleaned up.
if h.cleanup != nil && v != nil {
h.cleanup(v)
}
return
}
if h.cleanup != nil && h.value != nil {
// Clean up before overwriting an existing value.
h.cleanup(h.value)
}
// At this point v will be cleaned up whenever h is destroyed.
h.value = v
h.function = nil
h.state = stateCompleted

45
internal/memoize/memoize_test.go
View File

@ -23,7 +23,7 @@ func TestGet(t *testing.T) {
h := g.Bind("key", func(context.Context, memoize.Arg) interface{} {
evaled++
return "res"
}, nil)
})
expectGet(t, h, g, "res")
expectGet(t, h, g, "res")
if evaled != 1 {
@ -50,7 +50,7 @@ func TestGenerations(t *testing.T) {
s := &memoize.Store{}
// Evaluate key in g1.
g1 := s.Generation("g1")
h1 := g1.Bind("key", func(context.Context, memoize.Arg) interface{} { return "res" }, nil)
h1 := g1.Bind("key", func(context.Context, memoize.Arg) interface{} { return "res" })
expectGet(t, h1, g1, "res")
// Get key in g2. It should inherit the value from g1.
@ -58,7 +58,7 @@ func TestGenerations(t *testing.T) {
h2 := g2.Bind("key", func(context.Context, memoize.Arg) interface{} {
t.Fatal("h2 should not need evaluation")
return "error"
}, nil)
})
expectGet(t, h2, g2, "res")
// With g1 destroyed, g2 should still work.
@ -68,47 +68,10 @@ func TestGenerations(t *testing.T) {
// With all generations destroyed, key should be re-evaluated.
g2.Destroy("TestGenerations")
g3 := s.Generation("g3")
h3 := g3.Bind("key", func(context.Context, memoize.Arg) interface{} { return "new res" }, nil)
h3 := g3.Bind("key", func(context.Context, memoize.Arg) interface{} { return "new res" })
expectGet(t, h3, g3, "new res")
}
func TestCleanup(t *testing.T) {
s := &memoize.Store{}
g1 := s.Generation("g1")
v1 := false
v2 := false
cleanup := func(v interface{}) {
*(v.(*bool)) = true
}
h1 := g1.Bind("key1", func(context.Context, memoize.Arg) interface{} {
return &v1
}, nil)
h2 := g1.Bind("key2", func(context.Context, memoize.Arg) interface{} {
return &v2
}, cleanup)
expectGet(t, h1, g1, &v1)
expectGet(t, h2, g1, &v2)
g2 := s.Generation("g2")
g2.Inherit(h1)
g2.Inherit(h2)
g1.Destroy("TestCleanup")
expectGet(t, h1, g2, &v1)
expectGet(t, h2, g2, &v2)
for k, v := range map[string]*bool{"key1": &v1, "key2": &v2} {
if got, want := *v, false; got != want {
t.Errorf("after destroying g1, bound value %q is cleaned up", k)
}
}
g2.Destroy("TestCleanup")
if got, want := v1, false; got != want {
t.Error("after destroying g2, v1 is cleaned up")
}
if got, want := v2, true; got != want {
t.Error("after destroying g2, v2 is not cleaned up")
}
}
func TestHandleRefCounting(t *testing.T) {
s := &memoize.Store{}
g1 := s.Generation("g1")