Skip to content

refactor(lru): switch LRU implementation to golang-lru #3674

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

Open
brbrr wants to merge 6 commits into
base: main
Choose a base branch
from
Open

refactor(lru): switch LRU implementation to golang-lru #3674

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ecb8eea
refactor(lru): switch to golang-lru
brbrr May 22, 2026
f68d61e
refactor(lru): wrap lru constructors so we could panic on error.
brbrr May 22, 2026
bfeda6c
add tests and fix lint errors
brbrr May 22, 2026
3ad6be2
wrap golang-lru in our package YAGNI style
brbrr May 25, 2026
4354dd1
add missing tests
brbrr May 27, 2026
1d8d578
revert AggregatedBloomFilterCache changes
brbrr May 27, 2026
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
9 changes: 5 additions & 4 deletions blockchain/aggregated_bloom_filter_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
"fmt"

"github.com/NethermindEth/juno/core"
"github.com/NethermindEth/juno/utils"
"github.com/bits-and-blooms/bitset"
"github.com/ethereum/go-ethereum/common/lru"
lru "github.com/hashicorp/golang-lru/v2"
)

// NOTE(Ege): consider making it configurable
Expand All @@ -27,15 +28,15 @@
// for block ranges, supporting fallback loading and bulk insertion.
// It is safe for concurrent use.
type AggregatedBloomFilterCache struct {
cache lru.Cache[EventFiltersCacheKey, *core.AggregatedBloomFilter]
cache *lru.Cache[EventFiltersCacheKey, *core.AggregatedBloomFilter]
fallbackFunc func(EventFiltersCacheKey) (core.AggregatedBloomFilter, error)
}

// NewAggregatedBloomCache creates a new LRU cache for aggregated bloom filters
// with the specified maximum size (number of ranges to cache).
func NewAggregatedBloomCache(size int) AggregatedBloomFilterCache {
func NewAggregatedBloomCache() AggregatedBloomFilterCache {
return AggregatedBloomFilterCache{
cache: *lru.NewCache[EventFiltersCacheKey, *core.AggregatedBloomFilter](size),
cache: utils.NewLRU[EventFiltersCacheKey, *core.AggregatedBloomFilter](AggregatedBloomFilterCacheSize),

Check failure on line 39 in blockchain/aggregated_bloom_filter_cache.go

View workflow job for this annotation

GitHub Actions / lint 

The line is 105 characters long, which exceeds the maximum of 100 characters. (lll)
}
Comment thread
brbrr marked this conversation as resolved.
}

Expand Down
8 changes: 4 additions & 4 deletions blockchain/aggregated_bloom_filter_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func populateAggregatedBloomDeterministic(

func TestMatchBlockIterator_InsertAndQueryRandomEvents(t *testing.T) {
numEvents := 64
numAggregatedBloomFilters := uint64(16)
var numAggregatedBloomFilters uint64 = blockchain.AggregatedBloomFilterCacheSize
Comment thread
brbrr marked this conversation as resolved.
Outdated
blocksPerFilter := core.NumBlocksPerFilter
chainHeight := numAggregatedBloomFilters*blocksPerFilter - 1

Expand All @@ -159,7 +159,7 @@ func TestMatchBlockIterator_InsertAndQueryRandomEvents(t *testing.T) {

testDB := memory.New()
// Create cache and insert filters
cache := blockchain.NewAggregatedBloomCache(int(numAggregatedBloomFilters))
cache := blockchain.NewAggregatedBloomCache()
cache.SetMany(filters)
runningFilterStart := numAggregatedBloomFilters * blocksPerFilter
innerFilter := core.NewAggregatedFilter(runningFilterStart)
Expand Down Expand Up @@ -188,15 +188,15 @@ func TestMatchBlockIterator_InsertAndQueryRandomEvents(t *testing.T) {
}

func TestMatchedBlockIterator_BasicCases(t *testing.T) {
var numAggregatedBloomFilters uint64 = 16
var numAggregatedBloomFilters uint64 = blockchain.AggregatedBloomFilterCacheSize
chainHeight := numAggregatedBloomFilters*core.NumBlocksPerFilter - 1

events := generateRandomEvents(t, 1, 3, 1)
test := events[0]
emmitedEvery := 4
filters := populateAggregatedBloomDeterministic(t, numAggregatedBloomFilters, test, core.NumBlocksPerFilter, uint64(emmitedEvery))

cache := blockchain.NewAggregatedBloomCache(int(numAggregatedBloomFilters))
cache := blockchain.NewAggregatedBloomCache()
cache.SetMany(filters)

testDB := memory.New()
Expand Down
2 changes: 1 addition & 1 deletion blockchain/blockchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func New(database db.KeyValueStore, network *networks.Network, opts ...Option) *
opt(&o)
}

cachedFilters := NewAggregatedBloomCache(AggregatedBloomFilterCacheSize)
cachedFilters := NewAggregatedBloomCache()
Copy link
Copy Markdown
Contributor

@rodrodros rodrodros May 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Why remove the input param?

Copy link
Copy Markdown
Contributor Author

@brbrr brbrr May 27, 2026

Choose a reason for hiding this comment

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

This constructor is used only with a comptime const:

// NOTE(Ege): consider making it configurable
const AggregatedBloomFilterCacheSize = 16

I'm not sure what the rationale was to make the cache size runtime configurable, but currently it's not being used anywhere. If we'd leave it as is, we'd make the constructor to panic on invalid size values (<= 0), or we'd have to do a size check, and return and propagate error across the stack i.e. changing return signature of blockchain.New across 40-ish files.

So I've decided to remove the configurationability until it is needed.

Copy link
Copy Markdown
Contributor

@rodrodros rodrodros May 27, 2026

Choose a reason for hiding this comment

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

Add the configuration, panic on invalid size. There is no fear of panicking here, since it means it is a programmer error.

Copy link
Copy Markdown
Contributor Author

@brbrr brbrr May 27, 2026

Choose a reason for hiding this comment

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

reverted in 1d8d578

fallback := func(key EventFiltersCacheKey) (core.AggregatedBloomFilter, error) {
return core.GetAggregatedBloomFilter(database, key.fromBlock, key.toBlock)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ require (
github.com/ethereum/go-ethereum v1.17.3
github.com/fxamacker/cbor/v2 v2.9.2
github.com/go-playground/validator/v10 v10.30.2
github.com/hashicorp/golang-lru/v2 v2.0.7
github.com/libp2p/go-libp2p v0.48.0
github.com/libp2p/go-libp2p-kad-dht v0.39.1
github.com/libp2p/go-libp2p-pubsub v0.16.0
Expand Down Expand Up @@ -84,7 +85,6 @@ require (
github.com/google/uuid v1.6.0 // indirect
github.com/gorilla/websocket v1.5.3 // indirect
github.com/hashicorp/golang-lru v1.0.2 // indirect
github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
github.com/holiman/uint256 v1.3.2 // indirect
github.com/huin/goupnp v1.3.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
Expand Down
9 changes: 4 additions & 5 deletions rpc/v10/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
"github.com/NethermindEth/juno/rpc/rpccore"
"github.com/NethermindEth/juno/starknet/compiler"
"github.com/NethermindEth/juno/sync"
"github.com/NethermindEth/juno/utils"
"github.com/NethermindEth/juno/utils/log"
"github.com/NethermindEth/juno/vm"
"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common/lru"
lru "github.com/hashicorp/golang-lru/v2"
"github.com/sourcegraph/conc"
)

Expand Down Expand Up @@ -78,6 +79,7 @@
if err != nil {
logger.Fatalf("Failed to parse ABI: %v", err)
}

return &Handler{
bcReader: bcReader,
syncReader: syncReader,
Expand All @@ -95,10 +97,7 @@
l1Heads: feed.New[*core.L1Head](),
preLatestFeed: feed.New[*pending.PreLatest](),

blockTraceCache: lru.NewCache[
rpccore.TraceCacheKey,
TraceBlockTransactionsResponse,
](rpccore.TraceCacheSize),
blockTraceCache: utils.NewLRU[rpccore.TraceCacheKey, TraceBlockTransactionsResponse](rpccore.TraceCacheSize),

Check failure on line 100 in rpc/v10/handlers.go

View workflow job for this annotation

GitHub Actions / lint 

The line is 111 characters long, which exceeds the maximum of 100 characters. (lll)
filterLimit: math.MaxUint,
coreContractABI: contractABI,
}
Expand Down
5 changes: 3 additions & 2 deletions rpc/v8/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
"github.com/NethermindEth/juno/rpc/rpccore"
"github.com/NethermindEth/juno/starknet/compiler"
"github.com/NethermindEth/juno/sync"
"github.com/NethermindEth/juno/utils"
"github.com/NethermindEth/juno/utils/log"
"github.com/NethermindEth/juno/vm"
"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common/lru"
lru "github.com/hashicorp/golang-lru/v2"
"github.com/sourcegraph/conc"
)

Expand Down Expand Up @@ -89,7 +90,7 @@
preConfirmedFeed: feed.New[*pendingpkg.PreConfirmed](),
l1Heads: feed.New[*core.L1Head](),

blockTraceCache: lru.NewCache[rpccore.TraceCacheKey, []TracedBlockTransaction](rpccore.TraceCacheSize),
blockTraceCache: utils.NewLRU[rpccore.TraceCacheKey, []TracedBlockTransaction](rpccore.TraceCacheSize),

Check failure on line 93 in rpc/v8/handlers.go

View workflow job for this annotation

GitHub Actions / lint 

The line is 105 characters long, which exceeds the maximum of 100 characters. (lll)
filterLimit: math.MaxUint,
coreContractABI: contractABI,
}
Expand Down
8 changes: 3 additions & 5 deletions rpc/v9/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@
"github.com/NethermindEth/juno/rpc/rpccore"
"github.com/NethermindEth/juno/starknet/compiler"
"github.com/NethermindEth/juno/sync"
"github.com/NethermindEth/juno/utils"
"github.com/NethermindEth/juno/utils/log"
"github.com/NethermindEth/juno/vm"
"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common/lru"
lru "github.com/hashicorp/golang-lru/v2"
"github.com/sourcegraph/conc"
)

Expand Down Expand Up @@ -94,10 +95,7 @@
l1Heads: feed.New[*core.L1Head](),
preLatestFeed: feed.New[*pending.PreLatest](),

blockTraceCache: lru.NewCache[
rpccore.TraceCacheKey,
[]TracedBlockTransaction,
](rpccore.TraceCacheSize),
blockTraceCache: utils.NewLRU[rpccore.TraceCacheKey, []TracedBlockTransaction](rpccore.TraceCacheSize),

Check failure on line 98 in rpc/v9/handlers.go

View workflow job for this annotation

GitHub Actions / lint 

The line is 105 characters long, which exceeds the maximum of 100 characters. (lll)
filterLimit: math.MaxUint,
coreContractABI: contractABI,
}
Expand Down
9 changes: 5 additions & 4 deletions sync/pending_polling.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import (
"github.com/NethermindEth/juno/core/felt"
"github.com/NethermindEth/juno/core/pending"
"github.com/NethermindEth/juno/db"
"github.com/ethereum/go-ethereum/common/lru"
"github.com/NethermindEth/juno/utils"
"github.com/hashicorp/golang-lru/v2/simplelru"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -130,7 +131,7 @@ func (s *Synchronizer) storeEmptyPreConfirmed(
func (s *Synchronizer) handleTickerPreLatest(
ctx context.Context,
currentHead *core.Block,
seenByParent *lru.BasicLRU[felt.Felt, *pending.PreLatest],
seenByParent *simplelru.LRU[felt.Felt, *pending.PreLatest],
out chan<- *pending.PreLatest,
) bool {
preLatest, err := s.dataSource.BlockPreLatest(ctx)
Expand Down Expand Up @@ -168,7 +169,7 @@ func (s *Synchronizer) pollPreLatest(ctx context.Context, out chan<- *pending.Pr

// Cache of pre-latest blocks keyed by the hash of their parent.
// When we receive the head with this parent hash, we emit the cached pre-latest.
seenByParent := lru.NewBasicLRU[felt.Felt, *pending.PreLatest](preLatestCacheSize)
seenByParent := utils.NewSimpleLRU[felt.Felt, *pending.PreLatest](preLatestCacheSize)

ticker := time.NewTicker(s.preLatestPollInterval)
defer ticker.Stop()
Expand Down Expand Up @@ -219,7 +220,7 @@ func (s *Synchronizer) pollPreLatest(ctx context.Context, out chan<- *pending.Pr
deliveredForHead = s.handleTickerPreLatest(
ctx,
currentHead,
&seenByParent,
seenByParent,
out,
)
}
Expand Down
30 changes: 30 additions & 0 deletions utils/lru.go
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@rodrodros rodrodros May 25, 2026

Choose a reason for hiding this comment

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

I am thinking it is better to create a package inside utils/lru/lru.go so that imports are called lru by default. We would wrap whatever implementation we used from there

Copy link
Copy Markdown
Contributor

@rodrodros rodrodros May 27, 2026

Choose a reason for hiding this comment

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

I see this comment was marked as resolved without being resolved, why?

Copy link
Copy Markdown
Contributor Author

@brbrr brbrr May 27, 2026

Choose a reason for hiding this comment

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

I think it was updated in 3ad6be2. Let me know if that's not enough, or it's not what you meant.

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package utils

import (
"fmt"

lru "github.com/hashicorp/golang-lru/v2"
"github.com/hashicorp/golang-lru/v2/simplelru"
)

// NewLRU returns a new thread-safe LRU cache or panics if size <= 0.
// Use for caches sized by constants or validated config, where a zero size
// indicates a programmer error rather than a runtime condition.
func NewLRU[K comparable, V any](size int) *lru.Cache[K, V] {
c, err := lru.New[K, V](size)
if err != nil {
panic(fmt.Errorf("lru: %w (size=%d)", err, size))

Check warning on line 16 in utils/lru.go

View check run for this annotation

Codecov / codecov/patch

utils/lru.go#L16 

Added line #L16 was not covered by tests
}
return c
}

// NewSimpleLRU returns a new non-thread-safe LRU cache or panics if size <= 0.
// Use the same way as NewLRU when external synchronization is provided
// (e.g. single-goroutine ownership).
func NewSimpleLRU[K comparable, V any](size int) *simplelru.LRU[K, V] {
c, err := simplelru.NewLRU[K, V](size, nil)
if err != nil {
panic(fmt.Errorf("simplelru: %w (size=%d)", err, size))

Check warning on line 27 in utils/lru.go

View check run for this annotation

Codecov / codecov/patch

utils/lru.go#L27 

Added line #L27 was not covered by tests
}
return c
Comment thread
brbrr marked this conversation as resolved.
Outdated
}
Loading