Improve Benchmarking#2798
Conversation
JohnReedV
added
the
skip-cargo-audit
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE Baseline-to-medium scrutiny: established OpenTensor-associated contributor with substantial subtensor history; cached repo permission is read; no Gittensor association found; final commit includes non-PR-author activity by UnArbosSix; branch apply-benchmark-to-only-bad-benches -> devnet-ready. The PR does not modify Findings
Prior-comment reconciliation
ConclusionThe PR appears legitimate, but 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👎 Gittensor UNKNOWN; author has repo write permission, @opentensor profile context, and substantial recent subtensor history, so calibrated as established-contributor domain review. PR body is substantive and matches the benchmark/tooling scope, so I would leave it unchanged. Validation performed: Findings
Prior-comment reconciliation
ConclusionBlocking on the |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
JohnReedV
changed the title
…tensor/subtensor into apply-benchmark-to-only-bad-benches
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: VULNERABLE |
UnArbosSix
force-pushed
the
apply-benchmark-to-only-bad-benches
branch
from
ea42319 to
b1ab08e
Compare
| Subtensor::<T>::init_new_network(netuid, 1); | ||
| Subtensor::<T>::set_max_allowed_uids(netuid, 4096); | ||
| Subtensor::<T>::set_network_registration_allowed(netuid, true); | ||
| SubtokenEnabled::<T>::insert(netuid, true); | ||
| Burn::<T>::insert(netuid, benchmark_registration_burn()); | ||
| seed_swap_reserves::<T>(netuid); | ||
| fund_for_registration::<T>(netuid, &coldkey); | ||
|
|
||
| #[extrinsic_call] | ||
| _(RawOrigin::Signed(coldkey.clone()), netuid, hotkey, u64::MAX); |
There was a problem hiding this comment.
[HIGH] Benchmark the full-subnet register_limit path
This benchmark sets MaxAllowedUids to 4096 but leaves SubnetworkN at 0, so the measured dispatch takes the cheap append branch in register_neuron. In production, register_limit delegates to do_register, and when current_n >= max_n it calls get_neuron_to_prune before registration and then prunes/replaces an existing neuron. The generated register_limit weight has no component for that full-subnet scan and replacement cleanup, so registrations against full subnets can be undercharged. Seed the subnet to MaxAllowedUids without warming the prune path, then measure the register_limit dispatch on that state.
|
🔄 AI review updated — Skeptic: VULNERABLE |
2 participants
Description
This PR
Modifies the benchmark flow so that only extrinsics that have drifted passed the threshold or have incorrect read/writes are updated rather than replacing the entire file and creating conflicts everywhere.
Adds a lint to check for extrinsics with missing benchmarks. This can be hidden with
#[allow(unknown_lints, benchmarked_weight_not_plugged)]Adds 48 missing benchmarks
Fixes a bug where benchmarks could fail on noisy internal component drift even when the actual benchmark-level weight was still within the configured threshold.
Adds
block_stepto the benchmark flow so theon_initializeweight is generated instead of hardcoded. The benchmark simulates the current worst case with 2 epochs in a block.Updates the drift check to include proof size and to compare the actual benchmark-level weight instead of failing on noisy internal component drift.