-
Notifications
You must be signed in to change notification settings - Fork 143
Delete stale values from the shared database LRU caches in persist #3550
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
base: master
Are you sure you want to change the base?
Conversation
In general, the first 3 million blocks tend to be not representative for lru sizing since the database is still small in general and everything just fits - ie at block height 3m, the whole database is just 1.3gb total. Also, in the sample we can see another effect: the change is efficient in some block ranges but not all - in particular, it may be inefficient past the gas repricing following the shanghai dos attack, as can be seen in the last timing bracket .. for this kind of tests, it's usually better to run them at some more recent block range. running on a recent block range comes with its own difficulties - in particular, block time is no longer a good measure because it's too easily influenced by OS caching of the database - instead, it's better to look at "number of database lookups" - ie the hit / miss rate of the cache, more or less, and see which one is better - the test has to be long enough to get past the "warm-up" period. |
In that case, I'll run a larger test over a more recent block range and use metrics to count the cache hit / miss rates. |
see also |
I ran another longer test over approx 4 million blocks starting from just after the merge and here are the results:
In summary no improvement in run time overall. I also collected the cache hits/misses stats of the delete-from-caches run but lost this for the master run because my computer restarted before I could save it:
|
This result makes me think that we should have a "refresh" operation in minilru that updates an existing value without updating its position in the lru - this is a reasonable middle ground actually where reads still determine the eviction order but we don't lose the item due to a premature and potentially unnecessary |
Ok, I'll try that. Will run another test using this refresh operation instead. |
I've completed another test run using the LRUCache refresh operation and here are the results:
Cache hit stats for the run using refresh:
|
hmm .. what's going on here? this is oddly consistent between the versions also, would it make sense to rerun master to see that it's not a fluke? ie master vs master run should be near-identical if the benchmarking setup is good. |
Not sure about that. Sure, I'll run master again. |
After creating a new baseline from the latest master branch here are the results:
Here are the cache hit stats from the latest baseline run:
|
This change appears to improve performance by a few percent.
I ran block imports over the first 3 million blocks and here are the results: