Benchmark register_network against a full subnet map - #3022
Closed
philipmaymin wants to merge 3 commits into
Closed
Conversation
register_network's benchmark registers into a chain with no subnets on it, so neither of the two costs that scale with subnet count lands in the measurement: the unconditional NetworksAdded::iter() count, and the get_network_to_prune scan that runs once the count reaches SubnetLimit. Fill the map before measuring, and add register_network_pruning for the branch that was never measured at all. The two outcomes are mutually exclusive and neither dominates: creation costs more ref_time, pruning more proof_size. Both dispatches charge the componentwise max. register_network measures 293 reads and 14.39 ms of charged weight against the 41 reads and 5.98 ms it currently declares.
fill_subnets created stable subnets with empty pools, so both price scans over NetworksAdded exited early on every entry. get_moving_alpha_price returns on SubnetMechanism == 0 without reading SubnetMovingPrice, and swap current_price returns before reading the TAO reserve or the balancer unless the alpha reserve is nonzero. The map was walked while nothing on it was priced. Every synthetic subnet now gets a dynamic mechanism and a funded pool. Reserves and moving prices are staggered per subnet so the median's BTreeMap holds distinct keys instead of collapsing to a single entry. register_network_with_identity and register_network_pruning now carry a maximum 6,656 byte identity. Creation writes it to storage; queueing encodes it into the queue entry and again into the event. Passing None measured neither, which is why main charges register_network_with_identity less than register_network on every dimension despite it doing strictly more work. register_network 293 -> 671 reads, register_network_pruning 408 -> 916, register_network_with_identity 292 -> 670. do_register_network has three successful outcomes, not two. The third, wait_to_cleanup, reaches the same queueing block without running get_network_to_prune or do_dissolve_network, so at equal queue depth it is dominated by the pruning measurement. Comments corrected to say so.
The weight constants in pallets/subtensor live in the runtime wasm, so the corrected register_network measurement only reaches a chain through a runtime release. Mainnet is on 440.
|
@philipmaymin is attempting to deploy a commit to the RaoFoundation Team on Vercel. A member of the Team first needs to authorize it. |
This was referenced Jul 31, 2026
Collaborator
|
this will be covered here #2997 |
This file contains hidden or 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
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.
What
register_network's benchmark registers into a chain with no subnets on it. Everything expensive the extrinsic does scales with how many subnets already exist, so none of it lands in the measurement.Three costs scale with the subnet count and the benchmark misses all three.
do_register_networkcounts the subnet map on every call:That runs unconditionally. On a chain at the 128-subnet limit the iterator reads all 129 entries, root included, before the filter drops it. The weight this replaces annotates
NetworksAdded (r:3 w:1).If the count has reached
SubnetLimit,get_network_to_prunewalks the map a second time, readingNetworkRegisteredAtand the moving price for every entry, anddo_dissolve_networkruns on whichever subnet loses.get_median_subnet_alpha_pricethen walks it a third time, asking the swap pallet for each subnet's price. That one runs on both outcomes, on every call that clears the lock-cost check.Measured against a full chain instead of an empty one,
register_networkis 671 reads and 25.18 ms of charged weight, against the 41 reads and 5.98 ms it charges today. Proof size goes from 9,947 to 327,961.Why now
The benchmark measures a branch mainnet does not take. At block 8,739,071,
NetworksAddedholds 128 live non-root entries against aSubnetLimitof 128. The chain sits exactly at the cap, so every registration on finney right now takes the at-limit path: the full count, the median scan, the prune scan, then the queue. That path measures at 916 reads and charges 41.The gap also gets worse soon. The v440 notes say the price of entry should fall toward the cost of the registration transaction, which leaves the weight as most of what a registration costs.
The fix
A helper that fills
NetworksAddedbefore the measurement, and a second benchmark for the branch that was never measured at all.Filling the map isn't enough on its own. If you're checking the fixture, check this: both price scans exit early on a subnet that is stable or has an empty pool.
get_moving_alpha_pricereturns onSubnetMechanism == 0without readingSubnetMovingPrice.init_new_networkdoesn't set a mechanism, so a naively built subnet is stable.current_pricereturns on a non-dynamic mechanism, and on a dynamic one still returns before reading the TAO reserve or the balancer unless the alpha reserve is nonzero.A fixture of stable, empty-pool subnets therefore walks 128 entries while pricing none, which is the same mistake in a subtler form. So every synthetic subnet gets a mechanism of 1 and a funded pool, and the prices are staggered rather than identical, because the median builds a
BTreeMapkeyed on price and one repeated value collapses it to a single entry.Where an identity is in play it is measured at its maximum of 6,656 bytes, the largest
is_valid_subnet_identityaccepts.register_networktakes no identity argument, so that row measuresNone;register_network_with_identityandregister_network_pruningcarry a full one. It matters most on the queueing path, which encodes the identity into the queue entry and again into the event.The three outcomes
do_register_networkhas three successful ends, not two:The third joins the second at the same queueing block but gets there without running
get_network_to_pruneordo_dissolve_network, so at equal queue depth it is strictly cheaper and needs no separate measurement.The first two are mutually exclusive:
So
register_networkis benchmarked one below the limit, where it still pays the count and the median scan but takes the creation path, and a newregister_network_pruningcovers the other one.register_network_pruningis not an extrinsic. It exists only so the dispatches that can reach that branch have a worst case to charge.Both dispatches charge the componentwise
.max()of the pair. Pruning happens to dominate both dimensions underRocksDbWeight, but that isn't structural: creation does 39 more writes and 245 fewer reads, so the ordering depends on the read/write ratio inDbWeight..max()is correct whichever way that falls.What this still doesn't measure
Two pieces of the state these weights depend on have no bound, so no constant can cover them:
sudo_set_subnet_limitonly checksensure_root. The subnet count is root-settable with no ceiling.DissolveCleanupQueueandNetworkRegistrationQueueare plainVecrather thanBoundedVec, and both get decoded on every call before any branch is chosen. Reaching the queueing branch appends to the second one.I measured the second one on
register_network, so it gets a number instead of a caveat. Every figure above is taken with both queues empty. ANetworkRegistrationInfoentry encodes to 6,775 bytes at a maximum identity, so filling the queue to one entry per subnet slot adds 860,805 bytes of proof, 3.6x the table. The annotation set does not change: no new key is read, the keys already being read just hold more.register_network_pruningmoves the same way for the same reason. I am not quoting a figure for it, because the only measurement I have of that path came off a branch carrying unrelated changes.So this correction is partial. Empty-queue is not a defensible worst case, and neither is any particular depth while the type is unbounded. Fixing it properly means bounding the storage or adding a weight component, which needs its own migration and review. I kept that out of a measurement fix, but I'll write it if you want both together.
The same empty-chain problem exists in a sibling I have not touched here.
register_leased_network(pallets/subtensor/src/benchmarks/benchmarks.rs:1797on this branch) does not fill the subnet map either, so a lease that succeeds below the cap on a populated chain walks all ofNetworksAddedand all ofSubnetOwner, then refunds post-dispatch to a constant measured against an empty one. For a two-contributor lease that is a refund from roughly 1.30 MB down to about 15.4 KB. It is the same bug this PR describes, it predates this PR, and it is a two-line fixture change on top of the helper added here. I can include it if you want.One consequence
LinearWeightToFeeis degree 1 withcoeff_fracof 500,000 parts per billion, so the weight component of the fee tracks the declared weight directly. Correcting the weight raises that component from about 0.00299 TAO to about 0.0140 TAO.pallet_transaction_paymentcharges a base and a length fee on top, so those figures are not the whole of what a registration pays, only the part this PR moves. Both are small next to the lock, but the change is real, so it belongs in the description.Because that is a runtime behavior change,
spec_versiongoes 440 to 441. The weight constants live in the runtime wasm, so a corrected measurement only reaches a chain through a release. Mainnet is on 440. No migration: nothing moves storage or call index, sotransaction_versionstays 1.Testing
Existing tests unchanged and passing. No new tests: this changes a benchmark fixture and the weights generated from it, and the benchmark is the measurement. Weights measured locally with the repo's own flags from
scripts/benchmark_action.sh.run-benchmarks.ymlskips forks, so the bot will need to re-run these.Related Issue(s)
None that this fixes. #3024 depends on it, and #3023 is stacked on this branch: both cite these numbers, so a change here moves them.
Type of Change
Breaking Change
No API or storage change. The declared weight of
register_networkandregister_network_with_identityrises, which raises the weight component of the fee they pay from about 0.00299 TAO (0.00292 for the identity variant) to about 0.0140 TAO. That is the intended correction rather than a side effect, and it is whyspec_versiongoes to 441.Checklist
./scripts/fix_rust.shto fix formatting and clippy issuesAdditional Notes
run-benchmarks.ymlgates the self-hosted runner onhead.repo.fork == false, so the benchmark bot will not run against this branch. Every number here was measured on my hardware with the repo's own flags fromscripts/benchmark_action.sh. Treat them as a claim about the shape of the gap rather than as constants to ship, and re-run them on yours before merging. If it helps, add therun-benchmarkslabel or push the branch into the repo and I will rebase onto whatever it produces.This is deliberately a partial correction, for the reasons in "What this still doesn't measure" above. Landing the measurement that is already wrong beat bundling a redesign into it. Tell me if you want both at once.