-
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
Merged
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
2852f7a
Fixed benchmarks to follow new memory rule
xgreenx 57a9619
WIP: refactor commit_changes_with_height_update
Dentosal 2a503bd
Make it actually work
Dentosal 3e24828
Merge branch 'master' into dento/regenesis-migrate-FuelBlockIdsToHeights
Dentosal 46d6c8a
Revert changes to commit
Dentosal ba36f01
Metadata using RwLock
Dentosal 562bca3
Add genesis flag to db handler
Dentosal 336378a
More docs
Dentosal 9c0fd52
Add changelog entry
Dentosal 1c91490
Merge branch 'master' into dento/regenesis-migrate-FuelBlockIdsToHeights
Dentosal d50083e
Cleanup
Dentosal 8c861e0
Simplify locking
Dentosal ee3c832
Merge branch 'master' into feature/benchmarks-0.25.2
xgreenx 8e91767
Fixed csiz benchmark and updated VM initialization benchmark
xgreenx d8fd3dd
Fixed something
xgreenx 7fcd31d
Temporary fixed benchmarks
xgreenx df13a4a
Use UncheckedDatabase type for genesis imports
Dentosal 451f8be
Restore TransactableStorage::commit height parameter
Dentosal 28de830
Make Database use UncheckedDatabase internally
Dentosal 8ba1fc2
Restore height fetch on db creation
Dentosal 97d681c
Used stages to reuse most of the functionality
xgreenx 2136866
Merge branch 'dento/regenesis-migrate-FuelBlockIdsToHeights' into fea…
xgreenx 2bf802f
Fixed benchmarks with a proper fix from https://github.com/FuelLabs/f…
xgreenx 56859ad
Merge branch 'master' into feature/benchmarks-0.25.2
xgreenx f7946bc
Reset some unnecessary changes
xgreenx 9978427
Merge branch 'master' into feature/benchmarks-0.25.2
xgreenx 59fc1a1
Make CI happy
xgreenx d9c2d8f
Merge branch 'master' into feature/benchmarks-0.25.2
xgreenx 7738ffa
Merge branch 'master' into feature/benchmarks-0.25.2
xgreenx 298685d
Merge branch 'master' into feature/benchmarks-0.25.2
xgreenx 43756c4
Applied `master`
xgreenx ba11d13
Update CHANGELOG.md
xgreenx b9eb596
Improved redeability of the vm initialization
xgreenx 6397e25
Merge remote-tracking branch 'origin/feature/benchmarks-0.25.2' into …
xgreenx 9891916
Merge branch 'master' into feature/benchmarks-0.25.2
xgreenx 31fdbb0
Hope
xgreenx 7be5108
Merge remote-tracking branch 'origin/feature/benchmarks-0.25.2' into …
xgreenx File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Binary file modified
BIN
+3.44 KB
(100%)
bin/fuel-core/chainspec/dev-testnet/state_transition_bytecode.wasm
Binary file not shown.
Binary file modified
BIN
+3.44 KB
(100%)
bin/fuel-core/chainspec/testnet/state_transition_bytecode.wasm
Binary file not shown.
This file contains 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
This file contains 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
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> | ||
|
This file contains 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
Oops, something went wrong.
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.
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.
I'm not sure I understand the significance of adding
cfe
here or why it uses these specific registers. Can you elaborate on why we need this change? Is this just to make sure our benchmarks account for the worst case scenario?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.
Because of the new VM rules: the memory that we are accessing should be initialized. The
cfe
opcode in the benchmarks expands the stack to allocate the memory that will later be used by the instruction that we are benchmarking.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.
I think I understand better. In this example, we have
retd(RegId::ONE, 0x10)
, where we return the memory in the range from*ONE
to*0x10
. So we need to extend the call stack by the value pointed by0x10
.