From 91ee13bd952dc66426336d438afe1259bd839849 Mon Sep 17 00:00:00 2001 From: Teng Zhang Date: Wed, 22 May 2024 22:28:58 -0400 Subject: [PATCH] add package filter --- Cargo.lock | 2 ++ .../aptos-e2e-comparison-testing/Cargo.toml | 2 ++ .../src/execution.rs | 34 +++++++++++++++++-- .../aptos-e2e-comparison-testing/src/lib.rs | 3 ++ .../aptos-e2e-comparison-testing/src/main.rs | 29 +++++++++++++--- .../src/online_execution.rs | 27 +++++++++++++-- .../move/move-command-line-common/src/env.rs | 2 ++ .../move/move-compiler-v2/src/experiments.rs | 2 +- third_party/move/move-compiler-v2/src/lib.rs | 2 +- .../move/move-compiler-v2/src/options.rs | 21 ++++++++++-- 10 files changed, 110 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 246fc9a6af45c8..0e7bfbf5475d71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -772,7 +772,9 @@ dependencies = [ "clap 4.4.14", "futures", "itertools 0.12.1", + "move-command-line-common", "move-compiler", + "move-compiler-v2", "move-core-types", "move-model", "move-package", diff --git a/aptos-move/aptos-e2e-comparison-testing/Cargo.toml b/aptos-move/aptos-e2e-comparison-testing/Cargo.toml index e28cf89d1a5d4d..f8edee26f2e3dc 100644 --- a/aptos-move/aptos-e2e-comparison-testing/Cargo.toml +++ b/aptos-move/aptos-e2e-comparison-testing/Cargo.toml @@ -22,7 +22,9 @@ bcs = { workspace = true } clap = { workspace = true } futures = { workspace = true } itertools = { workspace = true } +move-command-line-common = { workspace = true } move-compiler = { workspace = true } +move-compiler-v2 = { workspace = true } move-core-types = { workspace = true } move-model = { workspace = true } move-package = { workspace = true } diff --git a/aptos-move/aptos-e2e-comparison-testing/src/execution.rs b/aptos-move/aptos-e2e-comparison-testing/src/execution.rs index b7a7daf8a58671..6a690da485c4cd 100644 --- a/aptos-move/aptos-e2e-comparison-testing/src/execution.rs +++ b/aptos-move/aptos-e2e-comparison-testing/src/execution.rs @@ -4,7 +4,8 @@ use crate::{ check_aptos_packages_availability, compile_aptos_packages, compile_package, data_state_view::DataStateView, generate_compiled_blob, is_aptos_package, CompilationCache, - DataManager, IndexReader, PackageInfo, TxnIndex, APTOS_COMMONS, + DataManager, IndexReader, PackageInfo, TxnIndex, APTOS_COMMONS, DISABLE_REF_CHECK, + DISABLE_SPEC_CHECK, ENABLE_REF_CHECK, }; use anyhow::Result; use aptos_framework::APTOS_PACKAGES; @@ -22,7 +23,7 @@ use clap::ValueEnum; use itertools::Itertools; use move_core_types::{account_address::AccountAddress, language_storage::ModuleId}; use move_model::metadata::CompilerVersion; -use std::{cmp, collections::HashMap, path::PathBuf, sync::Arc}; +use std::{cmp, collections::HashMap, env, path::PathBuf, sync::Arc}; fn load_packages_to_executor( executor: &mut FakeExecutor, @@ -91,18 +92,34 @@ pub struct Execution { input_path: PathBuf, pub execution_mode: ExecutionMode, pub bytecode_version: u32, + pub skip_ref_packages: Option, } impl Execution { + pub fn check_package_skip(&self, package_name: &str) -> bool { + println!("package name:{}", package_name); + if let Some(p) = &self.skip_ref_packages { + let packages = p.split(',').collect_vec(); + packages.contains(&package_name) + } else { + false + } + } + pub fn output_result_str(&self, msg: String) { eprintln!("{}", msg); } - pub fn new(input_path: PathBuf, execution_mode: ExecutionMode) -> Self { + pub fn new( + input_path: PathBuf, + execution_mode: ExecutionMode, + skip_ref_packages: Option, + ) -> Self { Self { input_path, execution_mode, bytecode_version: 6, + skip_ref_packages, } } @@ -221,6 +238,17 @@ impl Execution { if compiled_cache.failed_packages_v2.contains(&package_info) { v2_failed = true; } else { + if self.check_package_skip(&package_info.package_name) { + env::set_var( + "MOVE_COMPILER_EXP", + format!("{},{}", DISABLE_SPEC_CHECK, DISABLE_REF_CHECK), + ); + } else { + env::set_var( + "MOVE_COMPILER_EXP", + format!("{},{}", DISABLE_SPEC_CHECK, ENABLE_REF_CHECK), + ); + } let compiled_res_v2 = compile_package(package_dir, &package_info, Some(CompilerVersion::V2_0)); if let Ok(compiled_res) = compiled_res_v2 { diff --git a/aptos-move/aptos-e2e-comparison-testing/src/lib.rs b/aptos-move/aptos-e2e-comparison-testing/src/lib.rs index 1c929670f56a26..a4a0ec927db2b9 100644 --- a/aptos-move/aptos-e2e-comparison-testing/src/lib.rs +++ b/aptos-move/aptos-e2e-comparison-testing/src/lib.rs @@ -57,6 +57,9 @@ const ERR_LOG: &str = "err_log.txt"; const ROCKS_INDEX_DB: &str = "rocks_txn_idx_db"; pub const APTOS_COMMONS: &str = "aptos-commons"; const MAX_TO_FLUSH: usize = 50000; +pub const DISABLE_SPEC_CHECK: &str = "spec-check=off"; +pub const DISABLE_REF_CHECK: &str = "reference-safety=off"; +pub const ENABLE_REF_CHECK: &str = "reference-safety=on"; struct IndexWriter { index_writer: BufWriter, diff --git a/aptos-move/aptos-e2e-comparison-testing/src/main.rs b/aptos-move/aptos-e2e-comparison-testing/src/main.rs index 0d50666fe345f1..0e8ae68cbbe73a 100644 --- a/aptos-move/aptos-e2e-comparison-testing/src/main.rs +++ b/aptos-move/aptos-e2e-comparison-testing/src/main.rs @@ -3,12 +3,15 @@ use anyhow::Result; use aptos_comparison_testing::{ - prepare_aptos_packages, DataCollection, Execution, ExecutionMode, OnlineExecutor, APTOS_COMMONS, + prepare_aptos_packages, DataCollection, Execution, ExecutionMode, OnlineExecutor, + APTOS_COMMONS, DISABLE_SPEC_CHECK, }; use aptos_rest_client::Client; use clap::{Parser, Subcommand}; +use move_command_line_common::env::OVERRIDE_EXP_CACHE; +use move_compiler_v2::Experiment; use move_core_types::account_address::AccountAddress; -use std::path::PathBuf; +use std::{env, path::PathBuf}; use url::Url; const BATCH_SIZE: u64 = 500; @@ -58,6 +61,9 @@ pub enum Cmd { /// Used when execution_only is true #[clap(long)] execution_mode: Option, + /// Packages to be skipped for reference safety check + #[clap(long)] + skip_ref_packages: Option, }, /// Execution of txns Execute { @@ -66,6 +72,9 @@ pub enum Cmd { /// Whether to execute against V1, V2 alone or both compilers for comparison #[clap(long)] execution_mode: Option, + /// Packages to be skipped for reference safety check + #[clap(long)] + skip_ref_packages: Option, }, } @@ -86,7 +95,15 @@ pub struct Argument { #[tokio::main] async fn main() -> Result<()> { let args = Argument::parse(); - + env::set_var( + OVERRIDE_EXP_CACHE, + format!( + "{},{}", + Experiment::SPEC_CHECK, + Experiment::REFERENCE_SAFETY + ), + ); + env::set_var("MOVE_COMPILER_EXP", DISABLE_SPEC_CHECK); match args.cmd { Cmd::Dump { endpoint, @@ -129,6 +146,7 @@ async fn main() -> Result<()> { skip_failed_txns, skip_publish_txns, execution_mode, + skip_ref_packages, } => { let batch_size = BATCH_SIZE; let output = if let Some(path) = output_path { @@ -148,12 +166,14 @@ async fn main() -> Result<()> { skip_publish_txns, execution_mode.unwrap_or_default(), endpoint, + skip_ref_packages, )?; online.execute(args.begin_version, args.limit).await?; }, Cmd::Execute { input_path, execution_mode, + skip_ref_packages, } => { let input = if let Some(path) = input_path { path @@ -161,7 +181,8 @@ async fn main() -> Result<()> { PathBuf::from(".") }; prepare_aptos_packages(input.join(APTOS_COMMONS)).await; - let executor = Execution::new(input, execution_mode.unwrap_or_default()); + let executor = + Execution::new(input, execution_mode.unwrap_or_default(), skip_ref_packages); executor .execute_txns(args.begin_version, args.limit) .await?; diff --git a/aptos-move/aptos-e2e-comparison-testing/src/online_execution.rs b/aptos-move/aptos-e2e-comparison-testing/src/online_execution.rs index 166f2ff1e808fb..8a9162a754c417 100644 --- a/aptos-move/aptos-e2e-comparison-testing/src/online_execution.rs +++ b/aptos-move/aptos-e2e-comparison-testing/src/online_execution.rs @@ -4,6 +4,7 @@ use crate::{ compile_aptos_packages, dump_and_compile_from_package_metadata, is_aptos_package, CompilationCache, ExecutionMode, IndexWriter, PackageInfo, TxnIndex, APTOS_COMMONS, + DISABLE_REF_CHECK, DISABLE_SPEC_CHECK, ENABLE_REF_CHECK, }; use anyhow::Result; use aptos_framework::natives::code::PackageMetadata; @@ -14,6 +15,7 @@ use aptos_validator_interface::{AptosValidatorInterface, FilterCondition, RestDe use move_core_types::account_address::AccountAddress; use std::{ collections::HashMap, + env, path::PathBuf, sync::{Arc, Mutex}, }; @@ -26,6 +28,7 @@ pub struct OnlineExecutor { filter_condition: FilterCondition, execution_mode: ExecutionMode, endpoint: String, + skip_ref_packages: Option, } impl OnlineExecutor { @@ -37,6 +40,7 @@ impl OnlineExecutor { skip_publish_txns: bool, execution_mode: ExecutionMode, endpoint: String, + skip_ref_packages: Option, ) -> Self { Self { debugger, @@ -50,6 +54,7 @@ impl OnlineExecutor { }, execution_mode, endpoint, + skip_ref_packages, } } @@ -61,6 +66,7 @@ impl OnlineExecutor { skip_publish_txns: bool, execution_mode: ExecutionMode, endpoint: String, + skip_ref_packages: Option, ) -> Result { Ok(Self::new( Arc::new(RestDebuggerInterface::new(rest_client)), @@ -70,6 +76,7 @@ impl OnlineExecutor { skip_publish_txns, execution_mode, endpoint, + skip_ref_packages, )) } @@ -176,10 +183,15 @@ impl OnlineExecutor { let current_dir = self.current_dir.clone(); let execution_mode = self.execution_mode; let endpoint = self.endpoint.clone(); + let skip_ref_packages = self.skip_ref_packages.clone(); let txn_execution_thread = tokio::task::spawn_blocking(move || { - let executor = crate::Execution::new(current_dir.clone(), execution_mode); - + println!("skip packages:{:?}", skip_ref_packages); + let executor = crate::Execution::new( + current_dir.clone(), + execution_mode, + skip_ref_packages, + ); let mut version_idx = TxnIndex { version, txn: txn.clone(), @@ -188,6 +200,17 @@ impl OnlineExecutor { // handle source code if let Some((address, package_name, map)) = source_code_data { + if executor.check_package_skip(&package_name) { + env::set_var( + "MOVE_COMPILER_EXP", + format!("{},{}", DISABLE_SPEC_CHECK, DISABLE_REF_CHECK), + ); + } else { + env::set_var( + "MOVE_COMPILER_EXP", + format!("{},{}", DISABLE_SPEC_CHECK, ENABLE_REF_CHECK), + ); + } let execution_mode_opt = Some(execution_mode); let package_info_opt = Self::dump_and_check_src( version, diff --git a/third_party/move/move-command-line-common/src/env.rs b/third_party/move/move-command-line-common/src/env.rs index a2c7df29918497..2b97da76a86322 100644 --- a/third_party/move/move-command-line-common/src/env.rs +++ b/third_party/move/move-command-line-common/src/env.rs @@ -50,6 +50,8 @@ pub fn get_move_compiler_block_v1_from_env() -> bool { read_bool_env_var(MOVE_COMPILER_BLOCK_V1_ENV_VAR) || read_bool_env_var(MVC_BLOCK_V1_ENV_VAR) } +pub const OVERRIDE_EXP_CACHE: &str = "OVERRIDE_EXP_CACHE"; + pub fn read_env_var(v: &str) -> String { std::env::var(v).unwrap_or_else(|_| String::new()) } diff --git a/third_party/move/move-compiler-v2/src/experiments.rs b/third_party/move/move-compiler-v2/src/experiments.rs index 6d073942441074..6a75cebb01639c 100644 --- a/third_party/move/move-compiler-v2/src/experiments.rs +++ b/third_party/move/move-compiler-v2/src/experiments.rs @@ -98,7 +98,7 @@ pub static EXPERIMENTS: Lazy> = Lazy::new(|| { Experiment { name: Experiment::SPEC_CHECK.to_string(), description: "Turns on or off specification checks".to_string(), - default: Given(false), + default: Inherited(Experiment::CHECKS.to_string()), }, Experiment { name: Experiment::SPEC_REWRITE.to_string(), diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index c87b7446b71345..98d81f0d4af549 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -7,7 +7,7 @@ pub mod ast_simplifier; mod bytecode_generator; pub mod cyclic_instantiation_checker; pub mod env_pipeline; -mod experiments; +pub mod experiments; mod file_format_generator; pub mod flow_insensitive_checkers; pub mod function_checker; diff --git a/third_party/move/move-compiler-v2/src/options.rs b/third_party/move/move-compiler-v2/src/options.rs index bb8933ad163c9a..dc659227395ddc 100644 --- a/third_party/move/move-compiler-v2/src/options.rs +++ b/third_party/move/move-compiler-v2/src/options.rs @@ -6,7 +6,7 @@ use crate::experiments::{DefaultValue, EXPERIMENTS}; use clap::Parser; use codespan_reporting::diagnostic::Severity; use itertools::Itertools; -use move_command_line_common::env::read_env_var; +use move_command_line_common::env::{read_env_var, OVERRIDE_EXP_CACHE}; use move_compiler::command_line as cli; use move_model::metadata::LanguageVersion; use once_cell::sync::Lazy; @@ -129,8 +129,15 @@ impl Options { visited.iter().clone().join(",") ) } - if let Some(on) = self.experiment_cache.borrow().get(name).cloned() { - return on; + let experiments_to_override = read_env_var(OVERRIDE_EXP_CACHE); + let experiments = experiments_to_override + .split(',') + .map(|s| s.to_string()) + .collect_vec(); + if !experiments.contains(&name.to_string()) { + if let Some(on) = self.experiment_cache.borrow().get(name).cloned() { + return on; + } } if let Some(exp) = EXPERIMENTS.get(&name.to_string()) { // First we look at experiments provided via the command line, second @@ -219,6 +226,14 @@ fn compiler_exp_var() -> Vec { } vec![] }); + if !read_env_var(OVERRIDE_EXP_CACHE).is_empty() { + for s in ["MVC_EXP", "MOVE_COMPILER_EXP"] { + let s = read_env_var(s); + if !s.is_empty() { + return s.split(',').map(|s| s.to_string()).collect(); + } + } + } (*EXP_VAR).clone() }