Skip to content
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

[Gas Metering] More accurate object size calculation #1249

Merged
merged 2 commits into from
Apr 9, 2022

Conversation

lxfind
Copy link
Contributor

@lxfind lxfind commented Apr 5, 2022

We need object size to properly calculate both of the computation cost and storage cost.
We could try to serialize the object to bytes and look at the serialized size, but that's too costly and we don't need to be that accurate. We should focus on what brings most of the size and that should be sufficient.
This PR looks at the size of all metadata and vectors and strings in the objects.

@oxade
Copy link
Contributor

oxade commented Apr 5, 2022

@lxfind good stuff. Few questions:

Have you compared the serialized size with the size yielded by this approximation for different types? Curious to see how much delta.

++ we're considering enforcing object sizes. Loosely 1MB for now. Do you reckon this approximation is good enough for use when enforcing size limits?

@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #1249 (d943c40) into main (2e10bbc) will increase coverage by 0.10%.
The diff coverage is 85.81%.

@@            Coverage Diff             @@
##             main    #1249      +/-   ##
==========================================
+ Coverage   81.65%   81.75%   +0.10%     
==========================================
  Files          99       99              
  Lines       20522    20560      +38     
==========================================
+ Hits        16757    16809      +52     
+ Misses       3765     3751      -14     
Impacted Files Coverage Δ
sui/src/sui-move.rs 3.12% <0.00%> (ø)
sui_core/src/execution_engine.rs 100.00% <ø> (ø)
sui_types/src/base_types.rs 77.74% <ø> (ø)
sui_programmability/adapter/src/adapter.rs 92.29% <69.64%> (+3.58%) ⬆️
sui/src/sui_commands.rs 76.63% <100.00%> (+0.49%) ⬆️
sui_core/src/authority.rs 94.95% <100.00%> (ø)
sui_core/src/authority/temporary_store.rs 81.49% <100.00%> (ø)
sui_core/src/unit_tests/gas_tests.rs 97.49% <100.00%> (+0.10%) ⬆️
sui_programmability/framework/src/lib.rs 85.54% <100.00%> (-1.50%) ⬇️
sui_types/src/object.rs 80.98% <100.00%> (-0.18%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bffd601...d943c40. Read the comment docs.

@lxfind lxfind force-pushed the use-accurate-object-size-for-gas-metering branch from c026d57 to cca6e43 Compare April 6, 2022 18:41
@lxfind
Copy link
Contributor Author

lxfind commented Apr 6, 2022

@lxfind good stuff. Few questions:

Have you compared the serialized size with the size yielded by this approximation for different types? Curious to see how much delta.

++ we're considering enforcing object sizes. Loosely 1MB for now. Do you reckon this approximation is good enough for use when enforcing size limits?

The difference should be relatively constant (i.e. mostly serialization type metadata stuff). So I would say the difference should be relatively small and we should be able to use this for enforcing size limits too.

Added tests.

@lxfind lxfind merged commit 640dcbe into main Apr 9, 2022
@lxfind lxfind deleted the use-accurate-object-size-for-gas-metering branch April 9, 2022 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants