core/state: replace fastcache code cache with gc-friendly structure #26092
Conversation
|
Should this (or something else) be used for the cleans cache too? That's also not currently being freed: Line 73 in bff84a9 |
There was a problem hiding this comment.
I agree this direction is better.
This is a reminder that fastcache is also used in trie.Database (cleans) with the same memory leak.
It's also used in snapshot's disklayer.cache - this should probably be cleaned up as well though I'm not certain if a memleak can be created from snapshot on normal operation.
| // Get looks up a key's value from the cache. | ||
| func (c *SizeConstrainedLRU) Get(key common.Hash) []byte { | ||
| c.lock.RLock() | ||
| defer c.lock.RUnlock() |
There was a problem hiding this comment.
nitpick, please add a blank line below, make eyes feel better.
common/lru/blob_lru.go
Outdated
| c.lock.RLock() | ||
| defer c.lock.RUnlock() | ||
| if v, ok := c.lru.Get(key); ok { | ||
| return []byte(v.(string)) |
There was a problem hiding this comment.
Why not we always accept []byte type value instead of converting it all the time? Code is []byte, we can set/get it into/from cache directly without any conversion.
If you think the passed byte slice might be "mutable", we can just add a comment that the passed value must not be changed.
|
@tsahee It's another story, we are aware of it! Snapshot and TrieDB are all should be singleton in Geth and only released when node is shutdown. But we might abuse it somewhere, will check it. |
|
@rjl493456442 Hi! |
|
@tsahee thanks for the reminder, regard |
…thereum#26092) This PR replaces fastcache with a pretty simple LRU which does not require explicit closing. (cherry picked from commit 5fded04)
…thereum#26092) This PR replaces fastcache with a pretty simple LRU which does not require explicit closing. (cherry picked from commit 5fded04)
…thereum#26092) This PR replaces fastcache with a pretty simple LRU which does not require explicit closing.
Initial poc, one way to replace
fastcachewith a pretty simple LRU which does not require explicit closing.This is an alternative to #26071