From 8e4141d74e6ea990c4a30f9f3d5a12b01d92fcf4 Mon Sep 17 00:00:00 2001 From: Karlo <88337245+Rqnsom@users.noreply.github.com> Date: Thu, 12 Sep 2024 13:10:42 +0200 Subject: [PATCH] refactor: optimize compile speed (skip fetch deps) Optimization takes use of the `skip-fetch-latest-git-deps` compilation flag. Regardless of this flag, by default - if no deps are found in the `/Users/$USER/.move` directory (a cache folder for Move packages) the missing packages will be downloaded there. But if the cache exists, then the `skip-fetch-latest-git-deps` flag can be used to skip checking whether those cached Move packages ought to be updated. In this PR - for the first compilation step (checking that the original tests are working fine), we disable that flag and ensure we get the latest Move packages. Then, for all subsequent compilations (mutant generation and running tests on those mutants), the tool enables that flag since it doesn't make much sense to check the latest deps repeatedly - checking it only once when the tool is started - it should be good enough. --- .github/workflows/check-and-lint.yml | 4 +-- move-mutation-test/src/cli.rs | 45 ++++++++++++++++++++++++- move-mutation-test/src/lib.rs | 23 +++++++++++-- move-mutation-test/src/mutation_test.rs | 43 +++-------------------- move-spec-test/src/lib.rs | 15 +++++++-- 5 files changed, 83 insertions(+), 47 deletions(-) diff --git a/.github/workflows/check-and-lint.yml b/.github/workflows/check-and-lint.yml index 974e170699..92944d1c53 100644 --- a/.github/workflows/check-and-lint.yml +++ b/.github/workflows/check-and-lint.yml @@ -2,9 +2,9 @@ name: Basic check and lint on: push: - branches: [ main ] + branches: [ main, develop-m1 ] pull_request: - branches: [ main ] + branches: [ main, develop-m1 ] env: CARGO_TERM_COLOR: always diff --git a/move-mutation-test/src/cli.rs b/move-mutation-test/src/cli.rs index 26db9b2efb..ebcd59c0c0 100644 --- a/move-mutation-test/src/cli.rs +++ b/move-mutation-test/src/cli.rs @@ -4,9 +4,13 @@ #![allow(clippy::too_long_first_doc_paragraph)] -use aptos::common::types::MovePackageDir; +use aptos::common::types::{MovePackageDir, OptimizationLevel}; +use aptos_framework::extended_checks; use clap::Parser; +use move_compiler_v2::Experiment; +use move_model::metadata::LanguageVersion; use move_mutator::cli::ModuleFilter; +use move_package::CompilerConfig; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -99,6 +103,45 @@ pub struct TestBuildConfig { //pub compute_coverage: bool, } +impl TestBuildConfig { + pub fn compiler_config(&self) -> CompilerConfig { + let known_attributes = extended_checks::get_all_attribute_names().clone(); + CompilerConfig { + known_attributes, + skip_attribute_checks: self.move_pkg.skip_attribute_checks, + bytecode_version: get_bytecode_version( + self.move_pkg.bytecode_version, + self.move_pkg.language_version, + ), + compiler_version: self.move_pkg.compiler_version, + language_version: self.move_pkg.language_version, + experiments: experiments_from_opt_level(&self.move_pkg.optimize), + } + } +} + +/// Get bytecode version. +fn get_bytecode_version( + bytecode_version_in: Option, + language_version: Option, +) -> Option { + bytecode_version_in.or_else(|| language_version.map(|lv| lv.infer_bytecode_version(None))) +} + +/// Get a list of stringified [`Experiment`] from the optimization level. +fn experiments_from_opt_level(optlevel: &Option) -> Vec { + match optlevel { + None | Some(OptimizationLevel::Default) => { + vec![format!("{}=on", Experiment::OPTIMIZE.to_string())] + }, + Some(OptimizationLevel::None) => vec![format!("{}=off", Experiment::OPTIMIZE.to_string())], + Some(OptimizationLevel::Extra) => vec![ + format!("{}=on", Experiment::OPTIMIZE_EXTRA.to_string()), + format!("{}=on", Experiment::OPTIMIZE.to_string()), + ], + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/move-mutation-test/src/lib.rs b/move-mutation-test/src/lib.rs index 1a9ff8f6a8..be3212039d 100644 --- a/move-mutation-test/src/lib.rs +++ b/move-mutation-test/src/lib.rs @@ -70,7 +70,15 @@ pub fn run_mutation_test( // Run original tests to ensure the original tests are working: - let result = run_tests(test_config, &package_path, &mut error_writer); + // We need to check for the latest git deps only for the first time we run the test. + // All subsequent runs with this tool will then have the latest deps fetched. + let skip_fetch_deps = false; + let result = run_tests( + test_config, + &package_path, + skip_fetch_deps, + &mut error_writer, + ); if let Err(e) = result { let msg = @@ -91,7 +99,14 @@ pub fn run_mutation_test( mutant_path.clone() } else { benchmarks.mutator.start(); - let mutator_config = BuildConfig::default(); + let mutator_config = BuildConfig { + dev_mode: test_config.move_pkg.dev, + additional_named_addresses: test_config.move_pkg.named_addresses(), + // No need to fetch latest deps again. + skip_fetch_latest_git_deps: true, + compiler_config: test_config.compiler_config(), + ..Default::default() + }; let outdir_mutant = run_mutator(options, &mutator_config, &package_path, &outdir)?; benchmarks.mutator.stop(); outdir_mutant @@ -152,7 +167,9 @@ pub fn run_mutation_test( benchmark.start(); - let result = run_tests(test_config, &outdir, &mut error_writer); + // No need to fetch latest deps again. + let skip_fetch_deps = true; + let result = run_tests(test_config, &outdir, skip_fetch_deps, &mut error_writer); benchmark.stop(); if let Err(e) = result { diff --git a/move-mutation-test/src/mutation_test.rs b/move-mutation-test/src/mutation_test.rs index 8fcc0ffc80..3339571798 100644 --- a/move-mutation-test/src/mutation_test.rs +++ b/move-mutation-test/src/mutation_test.rs @@ -4,7 +4,6 @@ use crate::cli::TestBuildConfig; use anyhow::Error; -use aptos::common::types::OptimizationLevel; use aptos_framework::extended_checks; use aptos_gas_schedule::{MiscGasParameters, NativeGasParameters, LATEST_GAS_FEATURE_VERSION}; use aptos_types::on_chain_config::{ @@ -13,9 +12,7 @@ use aptos_types::on_chain_config::{ use aptos_vm::natives; use move_cli::base::test::UnitTestResult; use move_command_line_common::address::NumericalAddress; -use move_compiler_v2::Experiment; -use move_model::metadata::LanguageVersion; -use move_package::{BuildConfig, CompilerConfig}; +use move_package::BuildConfig; use move_unit_test::UnitTestingConfig; use move_vm_runtime::native_functions::NativeFunctionTable; use std::path::Path; @@ -35,27 +32,17 @@ use termcolor::WriteColor; pub(crate) fn run_tests( cfg: &TestBuildConfig, package_path: &Path, + skip_fetch_latest_git_deps: bool, mut error_writer: &mut W, ) -> anyhow::Result<()> { - let known_attributes = extended_checks::get_all_attribute_names(); let config = BuildConfig { dev_mode: cfg.move_pkg.dev, additional_named_addresses: cfg.move_pkg.named_addresses(), test_mode: true, full_model_generation: cfg.move_pkg.check_test_code, install_dir: cfg.move_pkg.output_dir.clone(), - skip_fetch_latest_git_deps: false, - compiler_config: CompilerConfig { - known_attributes: known_attributes.clone(), - skip_attribute_checks: cfg.move_pkg.skip_attribute_checks, - bytecode_version: get_bytecode_version( - cfg.move_pkg.bytecode_version, - cfg.move_pkg.language_version, - ), - compiler_version: cfg.move_pkg.compiler_version, - language_version: cfg.move_pkg.language_version, - experiments: experiments_from_opt_level(&cfg.move_pkg.optimize), - }, + skip_fetch_latest_git_deps, + compiler_config: cfg.compiler_config(), ..Default::default() }; @@ -120,25 +107,3 @@ fn aptos_debug_natives( Features::default(), ) } - -/// Get bytecode version. -fn get_bytecode_version( - bytecode_version_in: Option, - language_version: Option, -) -> Option { - bytecode_version_in.or_else(|| language_version.map(|lv| lv.infer_bytecode_version(None))) -} - -/// Get a list of stringified [`Experiment`] from the optimization level. -fn experiments_from_opt_level(optlevel: &Option) -> Vec { - match optlevel { - None | Some(OptimizationLevel::Default) => { - vec![format!("{}=on", Experiment::OPTIMIZE.to_string())] - }, - Some(OptimizationLevel::None) => vec![format!("{}=off", Experiment::OPTIMIZE.to_string())], - Some(OptimizationLevel::Extra) => vec![ - format!("{}=on", Experiment::OPTIMIZE_EXTRA.to_string()), - format!("{}=on", Experiment::OPTIMIZE.to_string()), - ], - } -} diff --git a/move-spec-test/src/lib.rs b/move-spec-test/src/lib.rs index e04ebfcf67..08952d0f0e 100644 --- a/move-spec-test/src/lib.rs +++ b/move-spec-test/src/lib.rs @@ -82,11 +82,17 @@ pub fn run_spec_test( fs::create_dir_all(&outdir_original)?; + // We can skip fetching the latest deps for generating mutants and proving those mutants + // since the original prover verification already fetched the latest dependencies. + let mut quick_config = config.clone(); + quick_config.skip_fetch_latest_git_deps = true; + let outdir_mutant = if let Some(mutant_path) = &options.use_generated_mutants { mutant_path.clone() } else { benchmarks.mutator.start(); - let outdir_mutant = run_mutator(options, config, &package_path, &outdir)?; + + let outdir_mutant = run_mutator(options, &quick_config, &package_path, &outdir)?; benchmarks.mutator.stop(); outdir_mutant }; @@ -144,7 +150,12 @@ pub fn run_spec_test( move_mutator::compiler::rewrite_manifest_for_mutant(&package_path, &outdir_prove)?; benchmark.start(); - let result = prove(config, &outdir_prove, &prover_conf, &mut error_writer); + let result = prove( + &quick_config, + &outdir_prove, + &prover_conf, + &mut error_writer, + ); benchmark.stop(); if let Err(e) = result {