From a8423c92343ab643d5ebe9965cf1a7987f00bf11 Mon Sep 17 00:00:00 2001 From: Chris Masone Date: Wed, 13 Apr 2016 12:49:31 -0700 Subject: [PATCH] Special-case Package chunks when caching reachables during ReadValue For performance reasons, Package objects for generated Noms Types are side-loaded when reading Values. This means that the opportunistically-populated chunk->Type map used by DataStore when validating writes won't see these chunks in a number of cases. This can lead to false negatives and erroneous validation failures. This patch special-cases RefOfPackage when caching the Chunks reachable from a newly-read Value, manually fetching them from the types.PackageRegistry and crawling their reachable Chunks. Fixes #1229. --- datas/caching_value_store.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/datas/caching_value_store.go b/datas/caching_value_store.go index a04745c133..1589f20875 100644 --- a/datas/caching_value_store.go +++ b/datas/caching_value_store.go @@ -53,12 +53,27 @@ func (cvs *cachingValueStore) ReadValue(r ref.Ref) types.Value { var entry chunkCacheEntry = absentChunk{} if v != nil { entry = presentChunk(v.Type()) - for _, reachable := range v.Chunks() { - cvs.checkAndSet(reachable.TargetRef(), hintedChunk{getTargetType(reachable), r}) + cvs.cacheChunks(v, r) + } + if cur := cvs.check(r); cur == nil || cur.Hint().IsEmpty() { + cvs.set(r, entry) + } + return v +} + +func (cvs *cachingValueStore) cacheChunks(v types.Value, r ref.Ref) { + for _, reachable := range v.Chunks() { + hash := reachable.TargetRef() + if cur := cvs.check(hash); cur == nil || cur.Hint().IsEmpty() { + cvs.set(hash, hintedChunk{getTargetType(reachable), r}) + // Code-genned Packages are side-loaded when reading Values for performance reasons. This means that they won't pass through the ReadValue() codepath above, which means that they won't have their Chunks added to the cache. So, if reachable is a RefOfPackage, go look the package up in the types.PackageRegistry and recursively add its Chunks to the cache. + if _, ok := reachable.(types.RefOfPackage); ok { + if p := types.LookupPackage(hash); p != nil { + cvs.cacheChunks(p, hash) + } + } } } - cvs.checkAndSet(r, entry) - return v } func (cvs *cachingValueStore) isPresent(r ref.Ref) (present bool) {