Init Cache Manager #223
Init Cache Manager #223
Conversation
rachel-bousfield
force-pushed
the
init-cache
branch
from
65ebf1f
to
f057a89
Compare
| } | ||
|
|
||
| // Caches all programs with the given codehash. Caller must be a cache manager or chain owner. | ||
| func (con ArbWasmCache) CacheCodehash(c ctx, evm mech, codehash hash) error { |
There was a problem hiding this comment.
There is no explicit connection between caching and eviction, and not limit here on number of total contracts hashed.
I understand why - but it makes the existence of multiple cache managers more risky than helpful IMHO. This level should only have one cacheManager (+possibly chain owner). If multiple contracts are needed - let them all request caching from the single cachemanager. Then the single cachemanager could e.g. give different sub-managers allowance or something..
There was a problem hiding this comment.
I like having multiple only in that it keeps our storage format the same in future ArbOS upgrades should we extend the affordances of these actors. Back when we were initially designing the chain owner system in Nitro and before that 3rd party aggregators (throwback) we had a similar convo.
| initGas uint24 | ||
| asmEstimateKb uint24 // Predicted size of the asm | ||
| initGas uint16 | ||
| cachedInitGas uint16 |
There was a problem hiding this comment.
do we really want initGas + cachedInitGas per program?
it both complicates code and makes some changes harder.
I think we only want an initGas per contract, and a general param(s?) for discount. That'll give owner an easy handle that changes how much discount is given for being in cache (which it'll make sense to change while changing cache policy - you can give a larger discount if you cache 16 contracts vs if you cache 1024 of them)
There was a problem hiding this comment.
The reason for having both initGas and cachedInitGas is that programs vary in their initialization timings. I ran some benchmarks and found that it's not as simple as "cached is 8x cheaper" or "uncached pays an extra Y gas". So basically the variability of the savings motivates recording both during program activation.
arbos/programs/cache.go
Outdated
|
|
||
| import "github.com/ethereum/go-ethereum/common" | ||
|
|
||
| type RecentPrograms struct { |
There was a problem hiding this comment.
worth documenting somewhere that this is only inside transaction.
There was a problem hiding this comment.
Moved and documented in geth!
arbos/programs/cache.go
Outdated
| p.queue = append(p.queue, item) | ||
| p.items[item] = struct{}{} | ||
|
|
||
| if len(p.queue) > int(params.TxCacheSize) { |
There was a problem hiding this comment.
If access pattern is (A,B,A,C,A,D,A,E....) first to be evicted will be A.
- since this is inside a transaction - can't we have a reasonable upper-limit that means we don't really need to worry about eviction at all? (+possibly put some hardcoded upper limit that can't be reached)
- since this is inside memory - if we do care about eviction we can probably have something a little more complicated that will be more LRU
There was a problem hiding this comment.
Sure we can do the sequence number idea in memory. I'm about to push a commit to make the scoping per-block too, and then have Rust maintain a super-set :)
There was a problem hiding this comment.
Switched to an LRU cache in Geth :)
This PR introduces the concept of an "init cache manager" contract, which can be a Stylus program or EVM smart contract. Init cache managers can cache programs and slots, and are configurable via
ArbOwner. This allows the design of expresive auction mechanisms for Stylus-enabled chains.Additionally, this PR
Resolves STY-2