aptos-core
aptos-core copied to clipboard
aptos-labs
Add size_of implementation, decoupled from #4067
Addresses @davidiw 's request to decouple commits
Adds size_of() function for calculating number of bytes in a type, tests with vector size dynamism.
This change is 
To @wrwg and @zekun000's point in the previous PR, you've implemented size_of_val and not size_of. See https://github.com/aptos-labs/aptos-core/pull/4067#discussion_r968903116
I think the intention is very much size_of_val, based upon the previous PR. I do not even know how or why you would want size_of.
@davidiw Agreed that size_of_value is probably more appropriate naming in this context, and the intent was size_of, which would indeed would be more appropriate as a native function. The quasi-null paradigm would not be required in such an implementation, because the type argument could be simply operated on instead.
@wrwg you commented that a struct can occupy 100+ bytes in memory while only requiring a few when serialized. So if a struct has a single u8 field, does this mean that, for instance, it may require much more than just 1 byte for per-byte storage gas costs? I'd appreciate if you or @vgao1996 could point me to anywhere in the Aptos documentation where this kind of memory consideration is laid out cleanly, as dApp developers are a bit in the dark right now about these kinds of low-level concerns which have a large impact on gas optimizations.
If this is for accessing size in memory, the function is unfeasible. For example, a u128 with value 0 maybe just a byte serialized but much larger in Memory. A struct can be serialized a few bytes but expands to 100+ bytes in memory.
I think @alnoki , your goal was storage optimization... and per @wrwg point, that we should distinguish between memory costs and storage costs. For example, the length of a vector is stored as a uleb128 of up to 32-bits, so it can be as small as 8-bits while stored, but will always take at least 32-bits in memory.
I think @alnoki , your goal was storage optimization... and per @wrwg point, that we should distinguish between memory costs and storage costs. For example, the length of a vector is stored as a uleb128 of up to 32-bits, so it can be as small as 8-bits while stored, but will always take at least 32-bits in memory.
Yes, functionally this was intended for storage optimization, and thank you for the additional clarity. I have since been pointed to the Diem documentation on BCS by @chen-robert which touches on some of these points, but alas has nothing to say about memory versus storage in the Aptos-specific implementation.
@movekevin 19a029 compresses signature to a single line.
@davidiw 19a029 renames to size_of_val.
@wrwg 19a029 eliminates documentation references to null.
What's the progress here?
Per the above message, the last update was that I incorporated changes as requested by the three tagged reviewers.
Forge is running suite land_blocking on 19a029f2884a54c0370d76882caa4c7cf1fc5659
- Grafana dashboard (auto-refresh)
- Humio Logs
- Test runner output
- Test run is land-blocking
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
:white_check_mark: Forge suite land_blocking success on 19a029f2884a54c0370d76882caa4c7cf1fc5659
performance benchmark with full nodes : 7250 TPS, 5479 ms latency, 8100 ms p99 latency,(!) expired 280 out of 3096240 txns
Test Ok
- Grafana dashboard
- Humio Logs
- Test runner output
- Test run is land-blocking
@alnoki Could you re-push the commit?
@lightmark per your request 15583e45ab5e9c0bcacbb36db599ce8cae2b1bb5 has been pushed.
Forge is running suite compat on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> de887fa7a17a1fab65586a8f9caeb603284bd2b1
- Grafana dashboard (auto-refresh)
- Humio Logs
- Test runner output
- Test run is land-blocking
Forge is running suite land_blocking on de887fa7a17a1fab65586a8f9caeb603284bd2b1
- Grafana dashboard (auto-refresh)
- Humio Logs
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> de887fa7a17a1fab65586a8f9caeb603284bd2b1
Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> de887fa7a17a1fab65586a8f9caeb603284bd2b1 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7329 TPS, 5281 ms latency, 6900 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: de887fa7a17a1fab65586a8f9caeb603284bd2b1
compatibility::simple-validator-upgrade::single-validator-upgrade : 4573 TPS, 9163 ms latency, 12300 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: de887fa7a17a1fab65586a8f9caeb603284bd2b1
compatibility::simple-validator-upgrade::half-validator-upgrade : 4404 TPS, 9325 ms latency, 11400 ms p99 latency,no expired txns
4. upgrading second batch to new version: de887fa7a17a1fab65586a8f9caeb603284bd2b1
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6780 TPS, 5773 ms latency, 9800 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> de887fa7a17a1fab65586a8f9caeb603284bd2b1 passed
Test Ok
- Grafana dashboard
- Humio Logs
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite land_blocking success on de887fa7a17a1fab65586a8f9caeb603284bd2b1
performance benchmark with full nodes : 6921 TPS, 5738 ms latency, 13800 ms p99 latency,no expired txns
Test Ok
- Grafana dashboard
- Humio Logs
- Test runner output
- Test run is land-blocking
Forge is running suite compat on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 895d4b9524fb645ab791f4d409a350b76507f493
- Grafana dashboard (auto-refresh)
- Humio Logs
- Test runner output
- Test run is land-blocking
Forge is running suite land_blocking on 895d4b9524fb645ab791f4d409a350b76507f493
- Grafana dashboard (auto-refresh)
- Humio Logs
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite land_blocking success on 895d4b9524fb645ab791f4d409a350b76507f493
performance benchmark with full nodes : 6872 TPS, 5775 ms latency, 20700 ms p99 latency,no expired txns
Test Ok
- Grafana dashboard
- Humio Logs
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 895d4b9524fb645ab791f4d409a350b76507f493
Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 895d4b9524fb645ab791f4d409a350b76507f493 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7506 TPS, 5161 ms latency, 6600 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 895d4b9524fb645ab791f4d409a350b76507f493
compatibility::simple-validator-upgrade::single-validator-upgrade : 4557 TPS, 9182 ms latency, 13300 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 895d4b9524fb645ab791f4d409a350b76507f493
compatibility::simple-validator-upgrade::half-validator-upgrade : 4709 TPS, 8698 ms latency, 12200 ms p99 latency,no expired txns
4. upgrading second batch to new version: 895d4b9524fb645ab791f4d409a350b76507f493
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6113 TPS, 6228 ms latency, 9800 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 895d4b9524fb645ab791f4d409a350b76507f493 passed
Test Ok
- Grafana dashboard
- Humio Logs
- Test runner output
- Test run is land-blocking
Forge is running suite land_blocking on af76d6cdc20531e4eb49de554e600c0ec9c4e51c
- Grafana dashboard (auto-refresh)
- Humio Logs
- Test runner output
- Test run is land-blocking
Forge is running suite compat on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> af76d6cdc20531e4eb49de554e600c0ec9c4e51c
- Grafana dashboard (auto-refresh)
- Humio Logs
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite land_blocking success on af76d6cdc20531e4eb49de554e600c0ec9c4e51c
performance benchmark with full nodes : 6927 TPS, 5738 ms latency, 14800 ms p99 latency,no expired txns
Test Ok
- Grafana dashboard
- Humio Logs
- Test runner output
- Test run is land-blocking
:white_check_mark: Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> af76d6cdc20531e4eb49de554e600c0ec9c4e51c
Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> af76d6cdc20531e4eb49de554e600c0ec9c4e51c (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7748 TPS, 5029 ms latency, 6600 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: af76d6cdc20531e4eb49de554e600c0ec9c4e51c
compatibility::simple-validator-upgrade::single-validator-upgrade : 4434 TPS, 9118 ms latency, 13000 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: af76d6cdc20531e4eb49de554e600c0ec9c4e51c
compatibility::simple-validator-upgrade::half-validator-upgrade : 4849 TPS, 8524 ms latency, 10600 ms p99 latency,no expired txns
4. upgrading second batch to new version: af76d6cdc20531e4eb49de554e600c0ec9c4e51c
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6815 TPS, 5666 ms latency, 10400 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> af76d6cdc20531e4eb49de554e600c0ec9c4e51c passed
Test Ok
- Grafana dashboard
- Humio Logs
- Test runner output
- Test run is land-blocking