Contract: Optimize TaskConfig storage footprint#744
Open
wheval wants to merge 1 commit into
Open
Conversation
interval: u64 -> u32. Seconds-between-executions never needs more than ~136 years of range, unlike last_run (a ledger timestamp, kept u64 to match env.ledger().timestamp()) or gas_balance (a token amount, kept i128). blocked_by/yield_strategy stay u64 for consistency with every other ID in the contract (task IDs, portfolio IDs, etc. are all u64). Benchmark: added test_task_config_storage_footprint, which measures a TaskConfig's actual XDR-serialized size (the thing that determines ledger storage/rent, as opposed to the CPU/memory the existing test_gas.rs benchmarks measure) and asserts it as a regression lock. Measured 412 bytes before this change, 408 after - a 4-byte reduction per stored TaskConfig, matching the XDR ScVal encoding difference between U64 (4-byte type tag + 8-byte value) and U32 (4-byte type tag + 4-byte value). Also fixed track_gas() in test_gas.rs, which computed cpu/memory cost but discarded it without printing (stale 'println not available in wasm tests' comment - these are native tests, not wasm) - it now prints via std::eprintln! so cargo test -- --nocapture actually benchmarks something. Closes SoroLabs#674
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.
Summary
TaskConfig.interval: u64 -> u32. It's seconds between executions, never needs more than ~136 years of range - unlikelast_run(a ledger timestamp, keptu64to matchenv.ledger().timestamp()) orgas_balance(a token amount, kepti128). Leftblocked_by/yield_strategyasu64since every other ID in the contract (task IDs, portfolio IDs, etc.) isu64- shrinking just one would be an inconsistent, riskier change for a smaller win.test_task_config_storage_footprint, a benchmark that measures aTaskConfig's actual XDR-serialized size (the thing that determines ledger storage/rent, as opposed to the CPU/memorytest_gas.rs's existing benchmarks measure) and locks it in as a regression assertion.TaskConfig, matching the XDRScValencoding difference betweenU64(4-byte type tag + 8-byte value) andU32(4-byte type tag + 4-byte value).track_gas()intest_gas.rsalong the way: it computedcpu_instruction_cost()/memory_bytes_cost()but discarded them without printing, behind a stale "println not available in wasm tests" comment (these are nativecargo testtests, not wasm -eprintln!works fine). It now actually prints viastd::eprintln!, socargo test -- --nocapturebenchmarks something instead of nothing.Closes #674
Test plan
cargo test --lib- 136 tests passcargo test test_task_config_storage_footprint -- --nocapturemanually re-run againstinterval: u64(temporarily reverted) to get the "before" number, then restored - see commit message for both measurements