-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fixed benchmarks for the 0.25.3
#1870
Changes from 32 commits
2852f7a
57a9619
2a503bd
3e24828
46d6c8a
ba36f01
562bca3
336378a
9c0fd52
1c91490
d50083e
8c861e0
ee3c832
8e91767
d8fd3dd
7fcd31d
df13a4a
451f8be
28de830
8ba1fc2
97d681c
2136866
2bf802f
56859ad
f7946bc
9978427
59fc1a1
d9c2d8f
7738ffa
298685d
43756c4
ba11d13
b9eb596
6397e25
9891916
31fdbb0
7be5108
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,8 @@ use fuel_core_types::{ | |
Checked, | ||
IntoChecked, | ||
}, | ||
constraints::reg_key::Reg, | ||
consts::VM_MAX_RAM, | ||
interpreter::NotSupportedEcal, | ||
Interpreter, | ||
}, | ||
|
@@ -85,6 +87,9 @@ pub fn vm_initialization(c: &mut Criterion) { | |
if size as u64 > consensus_params.script_params().max_script_data_length() { | ||
break | ||
} | ||
if 2 * size as u64 > consensus_params.tx_params().max_size() { | ||
break | ||
} | ||
let script = vec![op::ret(1); size / Instruction::SIZE] | ||
.into_iter() | ||
.collect(); | ||
|
@@ -99,8 +104,36 @@ pub fn vm_initialization(c: &mut Criterion) { | |
Interpreter::<_, Script, NotSupportedEcal>::with_memory_storage(), | ||
); | ||
let ready_tx = tx.clone().test_into_ready(); | ||
black_box(vm.init_script(ready_tx)) | ||
.expect("Should be able to execute transaction"); | ||
|
||
// Initialize the VM and require the allocation of the whole memory | ||
// to charge for the worst possible case. | ||
black_box({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could say a the same for a lot of this. Can we move everthing in these curlies into some helper? |
||
vm.init_script(ready_tx) | ||
.expect("Should be able to execute transaction"); | ||
const VM_MEM_HALF: u64 = VM_MAX_RAM / 2; | ||
let mut i = 0; | ||
loop { | ||
let stack = 1 << i; | ||
|
||
if stack > VM_MEM_HALF { | ||
vm.memory_mut() | ||
.grow_heap(Reg::new(&0), 0) | ||
.expect("Should be able to grow heap"); | ||
break | ||
} | ||
|
||
let heap = VM_MAX_RAM - stack; | ||
vm.memory_mut() | ||
.grow_stack(stack) | ||
.expect("Should be able to grow stack"); | ||
vm.memory_mut() | ||
.grow_heap(Reg::new(&0), heap) | ||
.expect("Should be able to grow heap"); | ||
|
||
i += 1; | ||
} | ||
i | ||
}); | ||
}) | ||
}); | ||
i += 1; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,7 +70,7 @@ pub fn run(c: &mut Criterion) { | |
format!("{i}"), | ||
VmBench::contract(rng, op::retd(RegId::ONE, 0x10)) | ||
.unwrap() | ||
.with_post_call(vec![op::movi(0x10, *i)]) | ||
.with_post_call(vec![op::movi(0x10, *i), op::cfe(0x10)]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand the significance of adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of the new VM rules: the memory that we are accessing should be initialized. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I understand better. In this example, we have |
||
.with_call_receipts(receipts_ctx.clone()), | ||
); | ||
} | ||
|
@@ -84,7 +84,7 @@ pub fn run(c: &mut Criterion) { | |
format!("{i}"), | ||
VmBench::contract(rng, op::retd(RegId::ONE, 0x10)) | ||
.unwrap() | ||
.with_post_call(vec![op::movi(0x10, *i)]) | ||
.with_post_call(vec![op::movi(0x10, *i), op::cfe(0x10)]) | ||
.with_call_receipts(receipts_ctx.clone()), | ||
); | ||
} | ||
|
@@ -118,7 +118,7 @@ pub fn run(c: &mut Criterion) { | |
&mut logd, | ||
format!("{i}"), | ||
VmBench::new(op::logd(0x10, 0x11, RegId::ZERO, 0x13)) | ||
.with_prepare_script(vec![op::movi(0x13, *i)]) | ||
.with_prepare_script(vec![op::movi(0x13, *i), op::cfe(0x13)]) | ||
.with_call_receipts(receipts_ctx.clone()), | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -227,20 +227,54 @@ impl<Storage> Modifiable for InMemoryTransaction<Storage> { | |
} | ||
} | ||
|
||
impl<Column, S> InMemoryTransaction<S> | ||
where | ||
Column: StorageColumn, | ||
S: KeyValueInspect<Column = Column>, | ||
{ | ||
fn get_from_changes(&self, key: &[u8], column: Column) -> Option<&WriteOperation> { | ||
let k = key.to_vec(); | ||
self.changes | ||
.get(&column.id()) | ||
.and_then(|btree| btree.get(&k)) | ||
} | ||
} | ||
|
||
impl<Column, S> KeyValueInspect for InMemoryTransaction<S> | ||
where | ||
Column: StorageColumn, | ||
S: KeyValueInspect<Column = Column>, | ||
{ | ||
type Column = Column; | ||
|
||
fn exists(&self, key: &[u8], column: Self::Column) -> StorageResult<bool> { | ||
if let Some(operation) = self.get_from_changes(key, column) { | ||
match operation { | ||
WriteOperation::Insert(_) => Ok(true), | ||
WriteOperation::Remove => Ok(false), | ||
} | ||
} else { | ||
self.storage.exists(key, column) | ||
} | ||
} | ||
|
||
fn size_of_value( | ||
&self, | ||
key: &[u8], | ||
column: Self::Column, | ||
) -> StorageResult<Option<usize>> { | ||
if let Some(operation) = self.get_from_changes(key, column) { | ||
match operation { | ||
WriteOperation::Insert(value) => Ok(Some(value.len())), | ||
WriteOperation::Remove => Ok(None), | ||
} | ||
} else { | ||
self.storage.size_of_value(key, column) | ||
} | ||
} | ||
|
||
fn get(&self, key: &[u8], column: Self::Column) -> StorageResult<Option<Value>> { | ||
let k = key.to_vec(); | ||
if let Some(operation) = self | ||
.changes | ||
.get(&column.id()) | ||
.and_then(|btree| btree.get(&k)) | ||
{ | ||
if let Some(operation) = self.get_from_changes(key, column) { | ||
match operation { | ||
WriteOperation::Insert(value) => Ok(Some(value.clone())), | ||
WriteOperation::Remove => Ok(None), | ||
|
@@ -249,6 +283,31 @@ where | |
self.storage.get(key, column) | ||
} | ||
} | ||
|
||
fn read( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused. Did we not have implementations for these methods before on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
&self, | ||
key: &[u8], | ||
column: Self::Column, | ||
buf: &mut [u8], | ||
) -> StorageResult<Option<usize>> { | ||
if let Some(operation) = self.get_from_changes(key, column) { | ||
match operation { | ||
WriteOperation::Insert(value) => { | ||
let read = value.len(); | ||
if read != buf.len() { | ||
return Err(crate::Error::Other(anyhow::anyhow!( | ||
"Buffer size is not equal to the value size" | ||
))); | ||
} | ||
buf.copy_from_slice(value.as_ref()); | ||
Ok(Some(read)) | ||
} | ||
WriteOperation::Remove => Ok(None), | ||
} | ||
} else { | ||
self.storage.read(key, column, buf) | ||
} | ||
} | ||
} | ||
|
||
impl<Column, S> KeyValueMutate for InMemoryTransaction<S> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this into a helper with the name that explains why this should
break
? Same with the abovebreak
.It's not clear to me what the significance of
size
(size of what?) being half as big as the max tx size.