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.

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

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Oct 3, 2023

This also fixes a bug in GenRawGroupAnchorVerifier where the groupAnchor value wasn't being properly re assigned.

@@ -1381,12 +1400,14 @@ func GenRawGroupAnchorVerifier(ctx context.Context) func(*asset.Genesis,
return ErrGenesisNotGroupAnchor
}

groupAnchors[assetGroupKey] = gen
Copy link
Member Author

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.

Copy link
Member Author

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
Copy link
Member Author

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]
Copy link
Member Author

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.
var verifierLock sync.Mutex
groupAnchors := lru.NewCache[
asset.SerializedKey, singleCacheValue[*asset.Genesis]](
assetGroupCacheSize,
Copy link
Contributor

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?

@jharveyb jharveyb self-requested a review October 3, 2023 21:33
Copy link
Contributor

@jharveyb jharveyb left a 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.

@Roasbeef Roasbeef merged commit 55847fa into lightninglabs:main Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants