En 6455 lrucache maxsize #1819
Merged
En 6455 lrucache maxsize #1819
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
iulianpascalau
commented
May 28, 2020
iulianpascalau
commented
- implemented a LRU cache with size limit
- changed cacher interface: added sizeInBytes parameter in Put and HasOrAdd functions
- changed shardedData interface: added sizeInBytes parameter in Add function
- adapted mocks and code to the new interfaces signatures
…sOrAdd functions - changed shardedData interface: added sizeInBytes parameter in Add function - adapted mocks and code to the new interfaces signatures
# Conflicts: # dataRetriever/txpool/shardedTxPool_test.go # process/block/preprocess/adapterGenericCacheToSortedTransactionsProvider_test.go # process/throttle/antiflood/blackList/p2pBlackListProcessor.go # process/throttle/antiflood/floodPreventers/quotaFloodPreventer.go # process/throttle/antiflood/floodPreventers/quotaFloodPreventer_test.go # storage/txcache/txCache.go # storage/txcache/txCache_test.go
- added new cache type, adapted implementations to interfaces
- added unit tests, fixed lru cache with size implementation
sasurobert
requested changes
Jun 1, 2020
bogdan-rosianu
requested changes
Jun 1, 2020
config/config.go
Outdated
| @@ -5,7 +5,7 @@ type CacheConfig struct { | |||
| Type string `json:"type"` | |||
| Size uint32 `json:"size"` | |||
| SizePerSender uint32 `json:"sizePerSender"` | |||
| SizeInBytes uint32 `json:"sizeInBytes"` | |||
| SizeInBytes uint64 `json:"sizeInBytes"` | |||
There was a problem hiding this comment.
I don't think we serialize these configs anywhere so json tags are not neccesarry. Please check before removing (or at least leave a TODO)
consensus/mock/validatorMock.go
Outdated
| @@ -30,6 +30,11 @@ func (v *validator) Index() uint32 { | |||
| return v.index | |||
| } | |||
|
|
|||
| // Size - | |||
| func (v *validator) Size() int { | |||
| return len(v.pubKey) + 8 | |||
There was a problem hiding this comment.
2 uint32 (4+4 bytes, without associated pointers), moved to a const
| @@ -8,10 +8,11 @@ import ( | |||
|
|
|||
| const defaultMemDBSize = 10000 | |||
| const defaultNumShards = 1 | |||
| const noSizeInBytes = 0 | |||
There was a problem hiding this comment.
zeroSize shall be
| @@ -243,7 +243,7 @@ func TestValidatorInfoProcessor_ProcesStartOfEpochWithMissinPeerMiniblocksShould | |||
| } | |||
| return nil, false | |||
| }, | |||
| PutCalled: func(key []byte, value interface{}) (evicted bool) { | |||
| PutCalled: func(key []byte, value interface{}, sizeInBytes int) (evicted bool) { | |||
There was a problem hiding this comment.
not used, rename to _
| @@ -14,6 +14,7 @@ import ( | |||
| ) | |||
|
|
|||
| const maxNumPidsPerPk = 3 | |||
| const shardIdSize = 4 | |||
There was a problem hiding this comment.
rename to uint32Size
storage/interface.go
Outdated
| } | ||
|
|
||
| //SizeLRUCacheHandler is the interface for size capable LRU cache. | ||
| type SizeLRUCacheHandler interface { |
There was a problem hiding this comment.
might rename to SizedLRUCacheHandler
| log.Error("size LRU cache add error", | ||
| "key", fmt.Sprintf("%v", key), | ||
| "value", fmt.Sprintf("%v", value), | ||
| "error", "size in bytes is negative", |
There was a problem hiding this comment.
define an error for this
There was a problem hiding this comment.
it's a print. Added.
| } | ||
|
|
||
| func (c *CapacityLRU) addNew(key interface{}, value interface{}, sizeInBytes int64) { | ||
| ent := &entry{key, value, sizeInBytes} |
There was a problem hiding this comment.
please name the fields. (key: key for example)
| // ContainsOrAddSized checks if a key is in the cache without updating the | ||
| // recent-ness or deleting it for being stale, and if not, adds the value. | ||
| // Returns whether found and whether an eviction occurred. | ||
| func (c *CapacityLRU) ContainsOrAddSized(key, value interface{}, sizeInBytes int64) (bool, bool) { |
There was a problem hiding this comment.
rename to AddSizedIfMissing
# Conflicts: # dataRetriever/mock/shardedDataStub.go # dataRetriever/txpool/shardedTxPool.go # dataRetriever/txpool/shardedTxPool_test.go # epochStart/mock/stardedDataStub.go # integrationTests/mock/shardedDataStub.go # node/mock/shardedDataStub.go # process/mock/shardedDataStub.go # storage/errors.go # storage/interface.go # storage/txcache/disabledCache.go
…etwork/elrond-go into EN-6455-lrucache-maxsize
sasurobert
approved these changes
Jun 1, 2020
bogdan-rosianu
approved these changes
Jun 1, 2020
LucianMincu
approved these changes
Jun 1, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.