perf: mempool: lower priority optimizations #10693
Conversation
…hether curTsLks are needed as comments
…t we only need to make a single allocation
…ything except an ID address is robust) here, and return if so. That will: be faster reduce the size of this cache by half, because we can drop mp.keyCache.Add(ka, ka) on line 491.
| return mp.resolveToKeyFromID(ctx, addr) | ||
| } | ||
|
|
||
| func (mp *MessagePool) resolveToKeyFromID(ctx context.Context, addr address.Address) (address.Address, error) { |
There was a problem hiding this comment.
I don't think we need a new method here, just add the "is not ID" check to resolveToKey and leave it as before?
There was a problem hiding this comment.
I prefer the way I have it. I think it's easier to read! Definitely happy to defer to your suggestion if you have a preference
| mset, ok, err := mp.getPendingMset(ctx, from) | ||
| if err != nil { | ||
| log.Warnf("errored while getting pending mset: %w", err) | ||
| return nil, err |
There was a problem hiding this comment.
Can we please chain these errors (return nil, xerrors.Errorf("....%w", err)?
There was a problem hiding this comment.
(it makes debugging a lot easier when things go wrong)
There was a problem hiding this comment.
YES, that code is a lot cleaner! Appreciate the tip!!
| return mp.resolveToKeyFromID(ctx, addr) | ||
| } | ||
|
|
||
| func (mp *MessagePool) resolveToKeyFromID(ctx context.Context, addr address.Address) (address.Address, error) { |
| return mp.resolveToKeyFromID(ctx, addr) | ||
| } | ||
|
|
||
| func (mp *MessagePool) resolveToKeyFromID(ctx context.Context, addr address.Address) (address.Address, error) { |
There was a problem hiding this comment.
@Stebalien In what cases can an actor have both an f4 and an f2 address? Is that just EVM actors (not including EthAccounts)?
I wanna make sure you can never have an actor have both an f4 and an f2 address AND serving as a top-level sender, because that would make things funky here.
(Not really related to changes in this PR)
There was a problem hiding this comment.
Just EVM actors, at least for now. Account abstraction may someday change this, but that's a whole can of worms.
Related Issues
#10597
Proposed Changes
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps