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

cache+neutrino: update existing Cache interface to use type parameters #261

Merged
merged 9 commits into from
Feb 11, 2023

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Nov 9, 2022

This change gives all LRU cache declaration a bit more context, and also added compile-time type safety. Users no longer need to cast items before inserting or retrieving from the cache. Along the way, we copied the list.List implementation from Go and updated that itself to be generic.

PR is best reviewed in order with each commit.

@Roasbeef
Copy link
Member Author

Roasbeef commented Nov 9, 2022

cc @halseth

@Roasbeef
Copy link
Member Author

Roasbeef commented Nov 9, 2022

Alternatively:

diff --git a/cache/lru/lru.go b/cache/lru/lru.go
index c08c825..bc3cb66 100644
--- a/cache/lru/lru.go
+++ b/cache/lru/lru.go
@@ -9,7 +9,7 @@ import (
 
 // elementMap is an alias for a map from a generic interface to a list.Element.
 type elementMap[K comparable, V any] struct {
-	m map[K]*Element[V]
+	m map[K]V
 }
 
 // Del deletes an item from the cache.
@@ -18,13 +18,13 @@ func (e *elementMap[K, V]) Del(key K) {
 }
 
 // Lookup attempts to lookup a value in the cache.
-func (e *elementMap[K, V]) Lookup(key K) (*Element[V], bool) {
+func (e *elementMap[K, V]) Lookup(key K) (V, bool) {
 	el, ok := e.m[key]
 	return el, ok
 }
 
 // Store attempts to store an item in the cache.
-func (e *elementMap[K, V]) Store(key K, val *Element[V]) {
+func (e *elementMap[K, V]) Store(key K, val V) {
 	e.m[key] = val
 }
 
@@ -51,7 +51,7 @@ type Cache[K comparable, V cache.Value] struct {
 
 	// cache is a generic cache which allows us to find an elements position
 	// in the ll list from a given key.
-	cache elementMap[K, entry[K, V]]
+	cache elementMap[K, *Element[entry[K, V]]]
 
 	// mtx is used to make sure the Cache is thread-safe.
 	mtx sync.RWMutex
@@ -63,7 +63,7 @@ func NewCache[K comparable, V cache.Value](capacity uint64) *Cache[K, V] {
 	return &Cache[K, V]{
 		capacity: capacity,
 		ll:       NewList[entry[K, V]](),
-		cache: elementMap[K, entry[K, V]]{
+		cache: elementMap[K, *Element[entry[K, V]]]{
 			m: make(map[K]*Element[entry[K, V]]),
 		},
 	}

Then elementMap can go back to being a typedef.

@Roasbeef Roasbeef changed the title cache+neutrino: update existing Cache interface to use type parmaters cache+neutrino: update existing Cache interface to use type parameters Nov 9, 2022
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Love the generic map as well, we'll likely use that in quite a few places. LGTM 🎉

The linter is failing but it looks like that's a general problem with the version we use (I can create a fix in another PR).

cache/lru/lru.go Outdated Show resolved Hide resolved
@Roasbeef
Copy link
Member Author

The linter is failing but it looks like that's a general problem with the version we use (I can create a fix in another PR)

Seems like it can't handle the new generics syntax?

Copy link
Contributor

@halseth halseth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! 👯

Make a PR to go standard lib? 🤓

cache/lru/lru.go Outdated Show resolved Hide resolved
This'll let us use generics in the package.
We still need to retain the *list.List for now, as Go hasn't updated the
library to be aware of the new type params.

For the `Get` method, things are slightly different as we'll now return
the "default" value instead of nil when we encounter an error.
We'll be updating this in a follow up commit to use generics.
@Roasbeef Roasbeef merged commit 9e714d5 into lightninglabs:master Feb 11, 2023
buck54321 added a commit to buck54321/btcwallet that referenced this pull request Apr 21, 2024
lightninglabs/neutrino#261

Author: Olaoluwa Osuntokun <laolu32@gmail.com>
Date:   Fri Feb 10 19:13:50 2023 -0800
buck54321 added a commit to buck54321/btcwallet that referenced this pull request Apr 21, 2024
lightninglabs/neutrino#261

Author: Olaoluwa Osuntokun <laolu32@gmail.com>
Date:   Fri Feb 10 19:13:50 2023 -0800
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.

3 participants