-
Notifications
You must be signed in to change notification settings - Fork 118
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.
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
tapgarden: use lru.Cache for various new group caches #543
Conversation
69615bb
to
10021e0
Compare
@@ -1381,12 +1400,14 @@ func GenRawGroupAnchorVerifier(ctx context.Context) func(*asset.Genesis, | |||
return ErrGenesisNotGroupAnchor | |||
} | |||
|
|||
groupAnchors[assetGroupKey] = gen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here groupAnchor
wasn't re assigned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah returns nil anyway, so np.
// cache. This should be used when the cache just needs to worry aobut the | ||
// total number of elements, and not also the size (in bytes) of the elements. | ||
type singleCacheValue[T any] struct { | ||
val T |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't embed type param values, so had to use this here.
|
||
// emptyCacheVal is a type def for an empty cache value. In this case the cache | ||
// is used more as a set. | ||
type emptyCacheVal = singleCacheValue[emptyVal] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type alias used here to it inherits all the methods.
This also fixes a bug in `GenRawGroupAnchorVerifier` where the `groupAnchor` value wasn't being properly re assigned.
10021e0
to
111042f
Compare
var verifierLock sync.Mutex | ||
groupAnchors := lru.NewCache[ | ||
asset.SerializedKey, singleCacheValue[*asset.Genesis]]( | ||
assetGroupCacheSize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC with a 10k element cache this would be a few MB max?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Will be useful pattern for safely adding caching to some other callbacks as well.
This also fixes a bug in
GenRawGroupAnchorVerifier
where thegroupAnchor
value wasn't being properly re assigned.