Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: remove cache types to avoid inconsistencies #2606

Closed
wants to merge 2 commits into from
Closed

fix: remove cache types to avoid inconsistencies #2606

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d529ba0
fix: remove cache types to avoid inconsistencies
ajnavarro Jul 19, 2024
ee7873b
Remove usage of SetCacheType
ajnavarro Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 4 additions & 41 deletions gnovm/pkg/gnolang/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
alloc *Allocator // for accounting for cached items
pkgGetter PackageGetter // non-realm packages
cacheObjects map[ObjectID]Object
cacheTypes map[TypeID]Type
cacheNodes map[Location]BlockNode
cacheNativeTypes map[reflect.Type]Type // go spec: reflect.Type are comparable
baseStore store.Store // for objects, types, nodes
Expand All @@ -101,7 +100,6 @@
alloc: alloc,
pkgGetter: nil,
cacheObjects: make(map[ObjectID]Object),
cacheTypes: make(map[TypeID]Type),
cacheNodes: make(map[Location]BlockNode),
cacheNativeTypes: make(map[reflect.Type]Type),
baseStore: baseStore,
Expand All @@ -118,7 +116,6 @@
func CopyCachesFromStore(dst, src Store) {
ds, ss := dst.(*defaultStore), src.(*defaultStore)
ds.cacheObjects = maps.Clone(ss.cacheObjects)
ds.cacheTypes = maps.Clone(ss.cacheTypes)
ds.cacheNodes = maps.Clone(ss.cacheNodes)
}

Expand Down Expand Up @@ -219,7 +216,7 @@
// (for other types, .T == nil even after definition).
} else if tv.T.Kind() == TypeKind {
t := tv.GetType()
ds.SetCacheType(t)
ds.SetType(t)
}
}
return pv
Expand Down Expand Up @@ -406,10 +403,6 @@
}

func (ds *defaultStore) GetTypeSafe(tid TypeID) Type {
// check cache.
if tt, exists := ds.cacheTypes[tid]; exists {
return tt
}
// check backend.
if ds.baseStore != nil {
key := backendTypeKey(tid)
Expand All @@ -423,9 +416,7 @@
tid, tt.TypeID()))
}
}
// set in cache.
ds.cacheTypes[tid] = tt
// after setting in cache, fill tt.

fillType(ds, tt)
return tt
}
Expand All @@ -434,38 +425,18 @@
}

func (ds *defaultStore) SetCacheType(tt Type) {
tid := tt.TypeID()
if tt2, exists := ds.cacheTypes[tid]; exists {
if tt != tt2 {
// NOTE: not sure why this would happen.
panic("should not happen")
} else {
// already set.
}
} else {
ds.cacheTypes[tid] = tt
}

Check failure on line 428 in gnovm/pkg/gnolang/store.go

View workflow job for this annotation

GitHub Actions / Run Main / Go Linter / lint 

File is not `gofumpt`-ed (gofumpt)
}

func (ds *defaultStore) SetType(tt Type) {
tid := tt.TypeID()
// return if tid already known.
if tt2, exists := ds.cacheTypes[tid]; exists {
if tt != tt2 {
// this can happen for a variety of reasons.
// TODO classify them and optimize.
return
}
}
// save type to backend.
if ds.baseStore != nil {
key := backendTypeKey(tid)
tcopy := copyTypeWithRefs(tt)
bz := amino.MustMarshalAny(tcopy)
ds.baseStore.Set([]byte(key), bz)
}
// save type to cache.
ds.cacheTypes[tid] = tt
}

func (ds *defaultStore) GetBlockNode(loc Location) BlockNode {
Expand Down Expand Up @@ -645,7 +616,6 @@

// Re-initialize caches. Some are cloned for speed.
cacheObjects: make(map[ObjectID]Object),
cacheTypes: maps.Clone(ds.cacheTypes),
// XXX: This is bad to say the least (ds.cacheNodes is shared with a
// child Store); however, cacheNodes is _not_ a cache, but a proper
// data store instead. SetBlockNode does not write anything to
Expand Down Expand Up @@ -775,7 +745,6 @@

func (ds *defaultStore) ClearCache() {
ds.cacheObjects = make(map[ObjectID]Object)
ds.cacheTypes = make(map[TypeID]Type)
ds.cacheNodes = make(map[Location]BlockNode)
ds.cacheNativeTypes = make(map[reflect.Type]Type)
// restore builtin types to cache.
Expand All @@ -791,12 +760,6 @@
fmt.Println(colors.Green("defaultStore:iavlStore..."))
utils.Print(ds.iavlStore)
fmt.Println(colors.Yellow("//----------------------------------------"))
fmt.Println(colors.Green("defaultStore:cacheTypes..."))
for tid, typ := range ds.cacheTypes {
fmt.Printf("- %v: %v\n", tid,
stringz.TrimN(fmt.Sprintf("%v", typ), 50))
}
fmt.Println(colors.Yellow("//----------------------------------------"))
fmt.Println(colors.Green("defaultStore:cacheNodes..."))
for loc, bn := range ds.cacheNodes {
fmt.Printf("- %v: %v\n", loc,
Expand Down Expand Up @@ -854,7 +817,7 @@
gErrorType, // from uverse.go
}
for _, tt := range types {
store.SetCacheType(tt)
store.SetType(tt)
}
store.SetCachePackage(Uverse())
}
Loading