Skip to content

Commit

Permalink
Io Gas adjustments
Browse files Browse the repository at this point in the history
1. read bytes charged in 4k steps
2. remove per write op free quota, while make per byte write cheaper

tune write gas numbers
  • Loading branch information
msmouse committed Dec 12, 2023
1 parent 1f0ba32 commit db2aecc
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 27 deletions.
34 changes: 23 additions & 11 deletions aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,38 +79,50 @@ crate::gas_schedule::macros::define_gas_parameters!(
[
storage_io_per_state_slot_read: InternalGasPerArg,
{ 0..=9 => "load_data.base", 10.. => "storage_io_per_state_slot_read"},
300_000,
// At the current mainnet scale, we should assume most levels of the (hexary) JMT nodes
// in cache, hence target charging 1-2 4k-sized pages for each read. Notice the cost
// of seeking for the leaf node is covered by the first page of the "value size fee"
// (storage_io_per_state_byte_read) defined below.
800_000,

Check warning on line 86 in aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L82-L86

Added lines #L82 - L86 were not covered by tests
],
[
storage_io_per_state_byte_read: InternalGasPerByte,
{ 0..=9 => "load_data.per_byte", 10.. => "storage_io_per_state_byte_read"},
300,
// Notice in the latest IoPricing, bytes are charged at 4k intervals (even the smallest
// read will be charged for 4KB) to reflect the assumption that every roughly 4k bytes
// might require a separate random IO upon the FS.
100,

Check warning on line 94 in aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L91-L94

Added lines #L91 - L94 were not covered by tests
],
[load_data_failure: InternalGas, "load_data.failure", 0],
// Gas parameters for writing data to storage.
[
storage_io_per_state_slot_write: InternalGasPerArg,
{ 0..=9 => "write_data.per_op", 10.. => "storage_io_per_state_slot_write"},
300_000,
// The cost of writing down the upper level new JMT nodes are shared between transactions
// because we write down the JMT in batches, however the bottom levels will be specific
// to each transactions assuming they don't touch exactly the same leaves. It's fair to
// target roughly 1-2 full internal JMT nodes (about 1KB in total) worth of writes for
// each write op.
100_000,

Check warning on line 106 in aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L101-L106

Added lines #L101 - L106 were not covered by tests
],
[
write_data_per_new_item: InternalGasPerArg,
"write_data.new_item",
1_280_000
legacy_write_data_per_new_item: InternalGasPerArg,
{0..=9 => "write_data.new_item"},
1_280_000,

Check warning on line 111 in aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L109-L111

Added lines #L109 - L111 were not covered by tests
],
[
storage_io_per_state_byte_write: InternalGasPerByte,
{ 0..=9 => "write_data.per_byte_in_key", 10.. => "storage_io_per_state_byte_write"},
5_000
100,

Check warning on line 116 in aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L116

Added line #L116 was not covered by tests
],
[
write_data_per_byte_in_val: InternalGasPerByte,
"write_data.per_byte_in_val",
legacy_write_data_per_byte_in_val: InternalGasPerByte,
{ 0..=9 => "write_data.per_byte_in_val" },

Check warning on line 120 in aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L119-L120

Added lines #L119 - L120 were not covered by tests
10_000
],
[memory_quota: AbstractValueSize, { 1.. => "memory_quota" }, 10_000_000],
[
free_write_bytes_quota: NumBytes,
legacy_free_write_bytes_quota: NumBytes,

Check warning on line 125 in aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs#L125

Added line #L125 was not covered by tests
{ 5.. => "free_write_bytes_quota" },
1024, // 1KB free per state write
],
Expand Down Expand Up @@ -211,7 +223,7 @@ impl TransactionGasParameters {
pub fn storage_fee_for_bytes(&self, key: &StateKey, op_size: &WriteOpSize) -> Fee {
if let Some(value_size) = op_size.write_len() {
let size = NumBytes::new(key.size() as u64) + NumBytes::new(value_size);
if let Some(excess) = size.checked_sub(self.free_write_bytes_quota) {
if let Some(excess) = size.checked_sub(self.legacy_free_write_bytes_quota) {
return excess * self.storage_fee_per_excess_state_byte;
}
}
Expand Down
54 changes: 45 additions & 9 deletions aptos-move/aptos-vm-types/src/storage/io_pricing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ impl IoPricingV1 {
fn new(gas_params: &AptosGasParameters) -> Self {
Self {
write_data_per_op: gas_params.vm.txn.storage_io_per_state_slot_write,
write_data_per_new_item: gas_params.vm.txn.write_data_per_new_item,
write_data_per_new_item: gas_params.vm.txn.legacy_write_data_per_new_item,

Check warning on line 37 in aptos-move/aptos-vm-types/src/storage/io_pricing.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/io_pricing.rs#L37

Added line #L37 was not covered by tests
write_data_per_byte_in_key: gas_params.vm.txn.storage_io_per_state_byte_write,
write_data_per_byte_in_val: gas_params.vm.txn.write_data_per_byte_in_val,
write_data_per_byte_in_val: gas_params.vm.txn.legacy_write_data_per_byte_in_val,

Check warning on line 39 in aptos-move/aptos-vm-types/src/storage/io_pricing.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/io_pricing.rs#L39

Added line #L39 was not covered by tests
load_data_base: gas_params.vm.txn.storage_io_per_state_slot_read * NumArgs::new(1),
load_data_per_byte: gas_params.vm.txn.storage_io_per_state_byte_read,
load_data_failure: gas_params.vm.txn.load_data_failure,
Expand Down Expand Up @@ -120,7 +120,7 @@ impl IoPricingV2 {
0 => unreachable!("PricingV2 not applicable for feature version 0"),
1..=2 => 0.into(),
3..=4 => 1024.into(),
5.. => gas_params.vm.txn.free_write_bytes_quota,
5.. => gas_params.vm.txn.legacy_free_write_bytes_quota,

Check warning on line 123 in aptos-move/aptos-vm-types/src/storage/io_pricing.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/io_pricing.rs#L123

Added line #L123 was not covered by tests
}
}

Expand Down Expand Up @@ -167,7 +167,7 @@ impl IoPricingV2 {
#[derive(Debug, Clone)]
pub struct IoPricingV3 {
pub feature_version: u64,
pub free_write_bytes_quota: NumBytes,
pub legacy_free_write_bytes_quota: NumBytes,
}

impl IoPricingV3 {
Expand All @@ -183,7 +183,7 @@ impl IoPricingV3 {
let value_size = NumBytes::new(value_size);

(key_size + value_size)
.checked_sub(self.free_write_bytes_quota)
.checked_sub(self.legacy_free_write_bytes_quota)

Check warning on line 186 in aptos-move/aptos-vm-types/src/storage/io_pricing.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/io_pricing.rs#L186

Added line #L186 was not covered by tests
.unwrap_or(NumBytes::zero())
}

Expand All @@ -204,11 +204,44 @@ impl IoPricingV3 {
}
}

#[derive(Debug, Clone)]
pub struct IoPricingV4;

impl IoPricingV4 {
fn calculate_read_gas(
&self,
loaded: NumBytes,
) -> impl GasExpression<VMGasParameters, Unit = InternalGasUnit> {
// round up bytes to whole pages
const PAGE_SIZE: u64 = 4096;

let loaded_u64: u64 = loaded.into();
let r = loaded_u64 % PAGE_SIZE;
let rounded_up = loaded_u64 + if r == 0 { 0 } else { PAGE_SIZE - r };

STORAGE_IO_PER_STATE_SLOT_READ * NumArgs::from(1)
+ STORAGE_IO_PER_STATE_BYTE_READ * NumBytes::new(rounded_up)
}

fn io_gas_per_write(
&self,
key: &StateKey,
op_size: &WriteOpSize,
) -> impl GasExpression<VMGasParameters, Unit = InternalGasUnit> {
let key_size = NumBytes::new(key.size() as u64);
let value_size = NumBytes::new(op_size.write_len().unwrap_or(0));
let size = key_size + value_size;

STORAGE_IO_PER_STATE_SLOT_WRITE * NumArgs::new(1) + STORAGE_IO_PER_STATE_BYTE_WRITE * size
}
}

#[derive(Clone, Debug)]
pub enum IoPricing {
V1(IoPricingV1),
V2(IoPricingV2),
V3(IoPricingV3),
V4(IoPricingV4),
}

impl IoPricing {
Expand All @@ -230,10 +263,11 @@ impl IoPricing {
gas_params,
)),
},
10.. => V3(IoPricingV3 {
10..=11 => V3(IoPricingV3 {
feature_version,
free_write_bytes_quota: gas_params.vm.txn.free_write_bytes_quota,
legacy_free_write_bytes_quota: gas_params.vm.txn.legacy_free_write_bytes_quota,

Check warning on line 268 in aptos-move/aptos-vm-types/src/storage/io_pricing.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/io_pricing.rs#L268

Added line #L268 was not covered by tests
}),
12.. => V4(IoPricingV4),
}
}

Expand All @@ -253,7 +287,8 @@ impl IoPricing {
},
)),
V2(v2) => Either::Left(v2.calculate_read_gas(bytes_loaded)),
V3(v3) => Either::Right(v3.calculate_read_gas(bytes_loaded)),
V3(v3) => Either::Right(Either::Left(v3.calculate_read_gas(bytes_loaded))),

Check warning on line 290 in aptos-move/aptos-vm-types/src/storage/io_pricing.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/io_pricing.rs#L290

Added line #L290 was not covered by tests
V4(v4) => Either::Right(Either::Right(v4.calculate_read_gas(bytes_loaded))),
}
}

Expand All @@ -269,7 +304,8 @@ impl IoPricing {
match self {
V1(v1) => Either::Left(v1.io_gas_per_write(key, op_size)),
V2(v2) => Either::Left(v2.io_gas_per_write(key, op_size)),
V3(v3) => Either::Right(v3.io_gas_per_write(key, op_size)),
V3(v3) => Either::Right(Either::Left(v3.io_gas_per_write(key, op_size))),

Check warning on line 307 in aptos-move/aptos-vm-types/src/storage/io_pricing.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/io_pricing.rs#L307

Added line #L307 was not covered by tests
V4(v4) => Either::Right(Either::Right(v4.io_gas_per_write(key, op_size))),
}
}
}
2 changes: 1 addition & 1 deletion aptos-move/aptos-vm-types/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl StorageGasParameters {
Self {
io_pricing: IoPricing::V3(IoPricingV3 {
feature_version: LATEST_GAS_FEATURE_VERSION,
free_write_bytes_quota,
legacy_free_write_bytes_quota: free_write_bytes_quota,

Check warning on line 41 in aptos-move/aptos-vm-types/src/storage/mod.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/storage/mod.rs#L41

Added line #L41 was not covered by tests
}),
change_set_configs: ChangeSetConfigs::unlimited_at_gas_feature_version(
LATEST_GAS_FEATURE_VERSION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn io_limit_reached_by_new_bytes() {
// Allow 10 value bytes charged at most.
gas_params.vm.txn.max_io_gas = 110_000_000.into();
// Make the key bytes free, only play around value sizes.
gas_params.vm.txn.free_write_bytes_quota = state_key_size();
gas_params.vm.txn.legacy_free_write_bytes_quota = state_key_size();
});

test_create_multiple_items(&mut h, &acc, |status| {
Expand All @@ -112,7 +112,7 @@ fn storage_limit_reached_by_new_bytes() {
// Allow 10 value bytes charged at most.
gas_params.vm.txn.max_storage_fee = 11_000_000.into();
// Make the key bytes free, only play around value sizes.
gas_params.vm.txn.free_write_bytes_quota = state_key_size();
gas_params.vm.txn.legacy_free_write_bytes_quota = state_key_size();
});

test_create_multiple_items(&mut h, &acc, |status| {
Expand All @@ -134,7 +134,7 @@ fn out_of_gas_while_charging_write_gas() {
// Bump max gas allowed
gas_params.vm.txn.maximum_number_of_gas_units = 1_000_000_000.into();
// Make the key bytes free, only play around value sizes.
gas_params.vm.txn.free_write_bytes_quota = state_key_size();
gas_params.vm.txn.legacy_free_write_bytes_quota = state_key_size();
});
// Allow 10 value bytes charged at most. Notice this is in external units.
h.set_max_gas_per_txn(110_000);
Expand All @@ -156,7 +156,7 @@ fn out_of_gas_while_charging_storage_fee() {
// Bump max gas allowed
gas_params.vm.txn.maximum_number_of_gas_units = 1_000_000_000.into();
// Make the key bytes free, only play around value sizes.
gas_params.vm.txn.free_write_bytes_quota = state_key_size();
gas_params.vm.txn.legacy_free_write_bytes_quota = state_key_size();
});
// Allow 10 value bytes charged at most. Notice this is in external units,
// which is 1/100x octas or 1Mx internal units.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn failed_transaction_cleanup_test() {

let gas_params = AptosGasParameters::zeros();
let storage_gas_params =
StorageGasParameters::unlimited(gas_params.vm.txn.free_write_bytes_quota);
StorageGasParameters::unlimited(gas_params.vm.txn.legacy_free_write_bytes_quota);

let change_set_configs = storage_gas_params.change_set_configs.clone();

Expand Down
12 changes: 11 additions & 1 deletion aptos-move/framework/table-natives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,17 @@ fn charge_load_cost(

match loaded {
Some(Some(num_bytes)) => {
context.charge(COMMON_LOAD_BASE_NEW + COMMON_LOAD_PER_BYTE * num_bytes)
if context.gas_feature_version() >= 12 {
// round up bytes to whole pages
const PAGE_SIZE: u64 = 4096;
let loaded_u64: u64 = num_bytes.into();
let r = loaded_u64 % PAGE_SIZE;
let rounded_up = loaded_u64 + if r == 0 { 0 } else { PAGE_SIZE - r };
context
.charge(COMMON_LOAD_BASE_NEW + COMMON_LOAD_PER_BYTE * NumBytes::new(rounded_up))
} else {
context.charge(COMMON_LOAD_BASE_NEW + COMMON_LOAD_PER_BYTE * num_bytes)

Check warning on line 299 in aptos-move/framework/table-natives/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/table-natives/src/lib.rs#L299

Added line #L299 was not covered by tests
}
},
Some(None) => context.charge(COMMON_LOAD_BASE_NEW + COMMON_LOAD_FAILURE),
None => Ok(()),
Expand Down

0 comments on commit db2aecc

Please sign in to comment.