From c5ac4e69d0d793903129be36614ccf4b30be05cd Mon Sep 17 00:00:00 2001 From: Igor Date: Fri, 6 Dec 2024 09:32:42 -0800 Subject: [PATCH 1/2] Optimizing vector::replace with mem::replace --- .../framework/aptos-stdlib/doc/big_vector.md | 2 +- aptos-move/framework/move-stdlib/doc/vector.md | 18 +++++++++--------- .../framework/move-stdlib/sources/mem.move | 2 +- .../framework/move-stdlib/sources/vector.move | 17 ++++++++--------- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/aptos-move/framework/aptos-stdlib/doc/big_vector.md b/aptos-move/framework/aptos-stdlib/doc/big_vector.md index 7d785a5b730a1..da978bc07ff9e 100644 --- a/aptos-move/framework/aptos-stdlib/doc/big_vector.md +++ b/aptos-move/framework/aptos-stdlib/doc/big_vector.md @@ -502,7 +502,7 @@ Aborts if i is out of bounds. if (self.end_index == i) { return last_val }; - // because the lack of mem::swap, here we swap remove the requested value from the bucket + // because the lack of mem::swap, here we swap remove the requested value from the bucket // and append the last_val to the bucket then swap the last bucket val back let bucket = table_with_length::borrow_mut(&mut self.buckets, i / self.bucket_size); let bucket_len = vector::length(bucket); diff --git a/aptos-move/framework/move-stdlib/doc/vector.md b/aptos-move/framework/move-stdlib/doc/vector.md index 9e837c9c1cfae..18512424c38e8 100644 --- a/aptos-move/framework/move-stdlib/doc/vector.md +++ b/aptos-move/framework/move-stdlib/doc/vector.md @@ -88,7 +88,8 @@ the return on investment didn't seem worth it for these simple functions. - [Function `rotate_slice`](#@Specification_1_rotate_slice) -
+
use 0x1::mem;
+
@@ -930,14 +931,13 @@ Aborts if i is out of bounds. 
public fun replace<Element>(self: &mut vector<Element>, i: u64, val: Element): Element {
     let last_idx = length(self);
     assert!(i < last_idx, EINDEX_OUT_OF_BOUNDS);
-    // TODO: Enable after tests are fixed.
-    // if (USE_MOVE_RANGE) {
-    //     mem::replace(borrow_mut(self, i), val)
-    // } else {
-    push_back(self, val);
-    swap(self, i, last_idx);
-    pop_back(self)
-    // }
+    if (USE_MOVE_RANGE) {
+        mem::replace(borrow_mut(self, i), val)
+    } else {
+        push_back(self, val);
+        swap(self, i, last_idx);
+        pop_back(self)
+    }
 }
diff --git a/aptos-move/framework/move-stdlib/sources/mem.move b/aptos-move/framework/move-stdlib/sources/mem.move index cf2eae276f9a5..e96db063db6c0 100644 --- a/aptos-move/framework/move-stdlib/sources/mem.move +++ b/aptos-move/framework/move-stdlib/sources/mem.move @@ -2,7 +2,7 @@ module std::mem { // TODO - functions here are `public(friend)` here for one release, // and to be changed to `public` one release later. - // friend std::vector; + friend std::vector; #[test_only] friend std::mem_tests; diff --git a/aptos-move/framework/move-stdlib/sources/vector.move b/aptos-move/framework/move-stdlib/sources/vector.move index b7add1eb2112a..e4d018e4f0b21 100644 --- a/aptos-move/framework/move-stdlib/sources/vector.move +++ b/aptos-move/framework/move-stdlib/sources/vector.move @@ -9,7 +9,7 @@ /// Move functions here because many have loops, requiring loop invariants to prove, and /// the return on investment didn't seem worth it for these simple functions. module std::vector { - // use std::mem; + use std::mem; /// The index into the vector is out of bounds const EINDEX_OUT_OF_BOUNDS: u64 = 0x20000; @@ -357,14 +357,13 @@ module std::vector { public fun replace(self: &mut vector, i: u64, val: Element): Element { let last_idx = length(self); assert!(i < last_idx, EINDEX_OUT_OF_BOUNDS); - // TODO: Enable after tests are fixed. - // if (USE_MOVE_RANGE) { - // mem::replace(borrow_mut(self, i), val) - // } else { - push_back(self, val); - swap(self, i, last_idx); - pop_back(self) - // } + if (USE_MOVE_RANGE) { + mem::replace(borrow_mut(self, i), val) + } else { + push_back(self, val); + swap(self, i, last_idx); + pop_back(self) + } } /// Apply the function to each element in the vector, consuming it. From 4bc4babe560c2ffda89e14f983e8e7a03d40f282 Mon Sep 17 00:00:00 2001 From: Igor Date: Mon, 9 Dec 2024 10:37:09 -0800 Subject: [PATCH 2/2] [compiler-v2] Also include all dependencies of vector for compiler v2 from #15528 --- third_party/move/move-model/src/lib.rs | 29 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/third_party/move/move-model/src/lib.rs b/third_party/move/move-model/src/lib.rs index 737a5a847021d..d16720ecf4642 100644 --- a/third_party/move/move-model/src/lib.rs +++ b/third_party/move/move-model/src/lib.rs @@ -305,13 +305,25 @@ pub fn run_model_builder_with_options_and_compilation_flags< }, }; - // Extract the module/script closure + // Extract the module/script dependency closure let mut visited_modules = BTreeSet::new(); + // Extract the module dependency closure for the vector module + let mut vector_and_its_dependencies = BTreeSet::new(); + let mut seen_vector = false; for (_, mident, mdef) in &expansion_ast.modules { let src_file_hash = mdef.loc.file_hash(); if !dep_files.contains(&src_file_hash) { collect_related_modules_recursive(mident, &expansion_ast.modules, &mut visited_modules); } + if !seen_vector && is_vector(*mident) { + seen_vector = true; + // Collect the vector module and its dependencies. + collect_related_modules_recursive( + mident, + &expansion_ast.modules, + &mut vector_and_its_dependencies, + ); + } } for sdef in expansion_ast.scripts.values() { let src_file_hash = sdef.loc.file_hash(); @@ -330,14 +342,13 @@ pub fn run_model_builder_with_options_and_compilation_flags< let mut expansion_ast = { let E::Program { modules, scripts } = expansion_ast; let modules = modules.filter_map(|mident, mut mdef| { - // We need to always include the `vector` module (only for compiler v2), + // For compiler v2, we need to always include the `vector` module and any of its dependencies, // to handle cases of implicit usage. // E.g., index operation on a vector results in a call to `vector::borrow`. // TODO(#15483): consider refactoring code to avoid this special case. - let is_vector = mident.value.address.into_addr_bytes().into_inner() - == AccountAddress::ONE - && mident.value.module.0.value.as_str() == "vector"; - (is_vector && compile_via_model || visited_modules.contains(&mident.value)).then(|| { + ((compile_via_model && vector_and_its_dependencies.contains(&mident.value)) + || visited_modules.contains(&mident.value)) + .then(|| { mdef.is_source_module = true; mdef }) @@ -416,6 +427,12 @@ pub fn run_model_builder_with_options_and_compilation_flags< } } +/// Is `module_ident` the `vector` module? +fn is_vector(module_ident: ModuleIdent_) -> bool { + module_ident.address.into_addr_bytes().into_inner() == AccountAddress::ONE + && module_ident.module.0.value.as_str() == "vector" +} + fn run_move_checker(env: &mut GlobalEnv, program: E::Program) { let mut builder = ModelBuilder::new(env); for (module_count, (module_id, module_def)) in program