-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: stmgr: speed up calculation of genesis circ supply #10553
Conversation
chain/stmgr/supply.go
Outdated
err := sm.setupGenesisVestingSchedule(ctx) | ||
if err != nil { | ||
return vf, xerrors.Errorf("failed to setup pre-ignition vesting schedule: %w", err) | ||
} | ||
sm.genesisMsigLk.Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each of these unlocks don't happen if we err out on the line above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite right, I'll move it up above the err return.
2e78d84
to
4d60f6e
Compare
chain/stmgr/supply.go
Outdated
// TODO: combine all this? | ||
if sm.preIgnitionVesting == nil || sm.genesisPledge.IsZero() || sm.genesisMarketFunds.IsZero() { | ||
if sm.preIgnitionVesting == nil || sm.genesisPledge.IsZero() { | ||
sm.genesisMsigLk.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might make more sense to move this lock into each of the vesting functions as they are also owned by the state manager and we are trying to make this lock more granular
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'm happy to do that. Note that we should only be calling each of these methods once, but I think that makes sense.
4d60f6e
to
bc87017
Compare
Related Issues
@jennijuju's goroutines reported that when trying to send many messages (or just create a lot of FVMs), this lock becomes contended for.
Proposed Changes
genesisMarketFunds
, which is zero on mainnet (confirmed)genesisMarketFunds
was equal to 0Additional Info
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 signatures
PR 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