-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[experiment-only] comparison testing #15317
base: main
Are you sure you want to change the base?
Conversation
⏱️ 23m total CI duration on this PR
|
// }); | ||
|
||
for (v, txn_index, state) in data_state_c.iter() { | ||
let res = Self::execute_one_txn_with_result_alternative(*v, state, txn_index, &cache_copy_v1); |
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.
txn is executed here.
if v2_flag { | ||
features.enable(FeatureFlag::VM_BINARY_FORMAT_V7); | ||
features.enable(FeatureFlag::VM_BINARY_FORMAT_V7); | ||
// features.enable(FeatureFlag::ENABLE_LOADER_V2); |
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.
new loader is enabled here
// } | ||
if let Some(debugger) = debugger_opt { | ||
let data_view = DataStateView::new(debugger, version, state); | ||
block_executor.execute_block(&DefaultTxnProvider::new(verified_txns), &data_view, onchain_config, transaction_slice_metadata) |
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.
block executor is called.
let txns = vec![txn.clone()]; | ||
let verified_txns = into_signature_verified_block(txns); | ||
let onchain_config: BlockExecutorConfigFromOnchain = BlockExecutorConfigFromOnchain::on_but_large_for_test(); | ||
let transaction_slice_metadata = TransactionSliceMetadata::Chunk { begin: version, end: version }; |
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.
You need end to be version + 1? Sorry if it's not clear from the type, I have a PR out which adds more documentation and assert for that
// the initializations, but ignore its internal state, i.e., FakeDataStore. | ||
let executor = FakeExecutor::no_genesis(); | ||
let block_executor = AptosVMBlockExecutor::new(); | ||
let txns = vec![txn.clone()]; |
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.
Why 1 txn? Can we compare-test a chunk, and if it fails (most likely not?) we can decide what to do? For example, re-execute from failed version plus one, or one by one? Like replay verify does
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.
Currently the data state we store is for each txn because they are not consecutive and the write set is not applied after each execution.
5d875b5
to
408fb29
Compare
408fb29
to
67f6be6
Compare
40d1cd9
to
b5e7b38
Compare
Description
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist