From 6046016826e7d97495b034cbe8b7476d9a9407e2 Mon Sep 17 00:00:00 2001 From: Igor Date: Tue, 8 Oct 2024 11:07:25 -0700 Subject: [PATCH] split_off => trim --- aptos-move/e2e-benchmark/src/main.rs | 10 ++-- .../framework/move-stdlib/doc/vector.md | 49 +++++-------------- .../framework/move-stdlib/sources/vector.move | 22 +++------ .../move-stdlib/tests/vector_tests.move | 46 +++++++++-------- crates/transaction-generator-lib/src/args.rs | 6 +-- .../src/publishing/module_simple.rs | 14 +++--- .../sources/vector_example.move | 4 +- testsuite/single_node_performance.py | 2 +- 8 files changed, 59 insertions(+), 94 deletions(-) diff --git a/aptos-move/e2e-benchmark/src/main.rs b/aptos-move/e2e-benchmark/src/main.rs index a83537675e65d..1893fc9788fbd 100644 --- a/aptos-move/e2e-benchmark/src/main.rs +++ b/aptos-move/e2e-benchmark/src/main.rs @@ -154,20 +154,20 @@ fn main() { }), (6871, EntryPoints::EmitEvents { count: 1000 }), // long vectors with small elements - (15890, EntryPoints::VectorSplitOffAppend { + (15890, EntryPoints::VectorTrimAppend { // baseline, only vector creation vec_len: 3000, element_len: 1, index: 0, repeats: 0, }), - (38047, EntryPoints::VectorSplitOffAppend { + (38047, EntryPoints::VectorTrimAppend { vec_len: 3000, element_len: 1, index: 100, repeats: 1000, }), - (25923, EntryPoints::VectorSplitOffAppend { + (25923, EntryPoints::VectorTrimAppend { vec_len: 3000, element_len: 1, index: 2990, @@ -193,14 +193,14 @@ fn main() { repeats: 1000, }), // vectors with large elements - (654, EntryPoints::VectorSplitOffAppend { + (654, EntryPoints::VectorTrimAppend { // baseline, only vector creation vec_len: 100, element_len: 100, index: 0, repeats: 0, }), - (11147, EntryPoints::VectorSplitOffAppend { + (11147, EntryPoints::VectorTrimAppend { vec_len: 100, element_len: 100, index: 10, diff --git a/aptos-move/framework/move-stdlib/doc/vector.md b/aptos-move/framework/move-stdlib/doc/vector.md index 620d0b5965fa9..b5b6ed525c382 100644 --- a/aptos-move/framework/move-stdlib/doc/vector.md +++ b/aptos-move/framework/move-stdlib/doc/vector.md @@ -30,7 +30,6 @@ the return on investment didn't seem worth it for these simple functions. - [Function `reverse_slice`](#0x1_vector_reverse_slice) - [Function `append`](#0x1_vector_append) - [Function `reverse_append`](#0x1_vector_reverse_append) -- [Function `split_off`](#0x1_vector_split_off) - [Function `trim`](#0x1_vector_trim) - [Function `trim_reverse`](#0x1_vector_trim_reverse) - [Function `is_empty`](#0x1_vector_is_empty) @@ -531,17 +530,18 @@ Pushes all of the elements of the other vector into the self< - + -## Function `split_off` +## Function `trim` -Splits the collection into two at the given index. -Returns a newly allocated vector containing the elements in the range [at, len). -After the call, the original vector will be left containing the elements [0, at) +Splits (trims) the collection into two at the given index. +Returns a newly allocated vector containing the elements in the range [new_len, len). +After the call, the original vector will be left containing the elements [0, new_len) with its previous capacity unchanged. +In many languages this is also called split_off. -
public fun split_off<Element>(self: &mut vector<Element>, at: u64): vector<Element>
+
public fun trim<Element>(self: &mut vector<Element>, new_len: u64): vector<Element>
 

 
 
@@ -550,15 +550,15 @@ with its previous capacity unchanged.
 Implementation
 
 
-
public fun split_off<Element>(self: &mut vector<Element>, at: u64): vector<Element> {
+
public fun trim<Element>(self: &mut vector<Element>, new_len: u64): vector<Element> {
     let len = length(self);
-    assert!(at <= len, EINDEX_OUT_OF_BOUNDS);
+    assert!(new_len <= len, EINDEX_OUT_OF_BOUNDS);
 
     let other = empty();
     if (USE_MOVE_RANGE) {
-        range_move(self, at, len - at, &mut other, 0);
+        range_move(self, new_len, len - new_len, &mut other, 0);
     } else {
-        while (len > at) {
+        while (len > new_len) {
             push_back(&mut other, pop_back(self));
             len = len - 1;
         };
@@ -571,33 +571,6 @@ with its previous capacity unchanged.
 
 
 
-
-
-
-
-## Function `trim`
-
-Trim a vector to a smaller size, returning the evicted elements in order
-
-
-
public fun trim<Element>(self: &mut vector<Element>, new_len: u64): vector<Element>
-

-
-
-
-

-Implementation
-
-
-
public fun trim<Element>(self: &mut vector<Element>, new_len: u64): vector<Element> {
-    let res = trim_reverse(self, new_len);
-    reverse(&mut res);
-    res
-}
-

-
-
-
 

 
 
diff --git a/aptos-move/framework/move-stdlib/sources/vector.move b/aptos-move/framework/move-stdlib/sources/vector.move
index a4a63446a4110..ab7b9a699778a 100644
--- a/aptos-move/framework/move-stdlib/sources/vector.move
+++ b/aptos-move/framework/move-stdlib/sources/vector.move
@@ -139,19 +139,20 @@ module std::vector {
         pragma intrinsic = true;
     }
 
-    /// Splits the collection into two at the given index.
-    /// Returns a newly allocated vector containing the elements in the range [at, len).
-    /// After the call, the original vector will be left containing the elements [0, at)
+    /// Splits (trims) the collection into two at the given index.
+    /// Returns a newly allocated vector containing the elements in the range [new_len, len).
+    /// After the call, the original vector will be left containing the elements [0, new_len)
     /// with its previous capacity unchanged.
-    public fun split_off(self: &mut vector, at: u64): vector {
+    /// In many languages this is also called `split_off`.
+    public fun trim(self: &mut vector, new_len: u64): vector {
         let len = length(self);
-        assert!(at <= len, EINDEX_OUT_OF_BOUNDS);
+        assert!(new_len <= len, EINDEX_OUT_OF_BOUNDS);
 
         let other = empty();
         if (USE_MOVE_RANGE) {
-            range_move(self, at, len - at, &mut other, 0);
+            range_move(self, new_len, len - new_len, &mut other, 0);
         } else {
-            while (len > at) {
+            while (len > new_len) {
                 push_back(&mut other, pop_back(self));
                 len = len - 1;
             };
@@ -160,13 +161,6 @@ module std::vector {
 
         other
     }
-
-    /// Trim a vector to a smaller size, returning the evicted elements in order
-    public fun trim(self: &mut vector, new_len: u64): vector {
-        let res = trim_reverse(self, new_len);
-        reverse(&mut res);
-        res
-    }
     spec trim {
         pragma intrinsic = true;
     }
diff --git a/aptos-move/framework/move-stdlib/tests/vector_tests.move b/aptos-move/framework/move-stdlib/tests/vector_tests.move
index f1c0884d021de..2c17620d02b96 100644
--- a/aptos-move/framework/move-stdlib/tests/vector_tests.move
+++ b/aptos-move/framework/move-stdlib/tests/vector_tests.move
@@ -105,7 +105,18 @@ module std::vector_tests {
             let v = vector[1, 2];
             assert!(&V::trim(&mut v, 0) == &vector[1, 2], 3);
         };
+        {
+            let v = vector[1, 2, 3, 4, 5, 6];
+            let other = V::trim(&mut v, 4);
+            assert!(v == vector[1, 2, 3, 4], 4);
+            assert!(other == vector[5, 6], 5);
+
+            let other_empty = V::trim(&mut v, 4);
+            assert!(v == vector[1, 2, 3, 4], 6);
+            assert!(other_empty == vector[], 7);
+        };
     }
+
     #[test]
     #[expected_failure(abort_code = V::EINDEX_OUT_OF_BOUNDS)]
     fun test_trim_fail() {
@@ -113,6 +124,13 @@ module std::vector_tests {
         V::trim(&mut v, 2);
     }
 
+    #[test]
+    #[expected_failure(abort_code = V::EINDEX_OUT_OF_BOUNDS)]
+    fun test_trim_fail_2() {
+        let v = vector[1, 2, 3];
+        V::trim(&mut v, 4);
+    }
+
     #[test]
     #[expected_failure(vector_error, minor_status = 1, location = Self)]
     fun borrow_out_of_range() {
@@ -952,7 +970,7 @@ module std::vector_tests {
     #[test]
     fun test_destroy() {
         let v = vector[MoveOnly {}];
-        vector::destroy(v, |m| { let MoveOnly {} = m; })
+        V::destroy(v, |m| { let MoveOnly {} = m; })
     }
 
     #[test]
@@ -969,34 +987,14 @@ module std::vector_tests {
     #[expected_failure(abort_code = V::EINDEX_OUT_OF_BOUNDS)]
     fun test_replace_empty_abort() {
         let v = vector[];
-        let MoveOnly {} = vector::replace(&mut v, 0, MoveOnly {});
-        vector::destroy_empty(v);
+        let MoveOnly {} = V::replace(&mut v, 0, MoveOnly {});
+        V::destroy_empty(v);
     }
 
     #[test]
     fun test_replace() {
         let v = vector[1, 2, 3, 4];
-        vector::replace(&mut v, 1, 17);
+        V::replace(&mut v, 1, 17);
         assert!(v == vector[1, 17, 3, 4], 0);
     }
-
-    #[test]
-    fun test_split_off() {
-        let v = vector[1, 2, 3, 4, 5, 6];
-        let other = vector::split_off(&mut v, 4);
-        assert!(v == vector[1, 2, 3, 4], 0);
-        assert!(other == vector[5, 6], 1);
-
-        let other_empty = vector::split_off(&mut v, 4);
-        assert!(v == vector[1, 2, 3, 4], 2);
-        assert!(other_empty == vector[], 3);
-
-    }
-
-    #[test]
-    #[expected_failure(abort_code = V::EINDEX_OUT_OF_BOUNDS)]
-    fun test_split_off_abort() {
-        let v = vector[1, 2, 3];
-        vector::split_off(&mut v, 4);
-    }
 }
diff --git a/crates/transaction-generator-lib/src/args.rs b/crates/transaction-generator-lib/src/args.rs
index 9c8443f842089..fe116e7124ccd 100644
--- a/crates/transaction-generator-lib/src/args.rs
+++ b/crates/transaction-generator-lib/src/args.rs
@@ -44,7 +44,7 @@ pub enum TransactionTypeArg {
     CreateObjects100,
     CreateObjects100WithPayload10k,
     CreateObjectsConflict100WithPayload10k,
-    VectorSplitOffAppendLen3000Size1,
+    VectorTrimAppendLen3000Size1,
     VectorRemoveInsertLen3000Size1,
     ResourceGroupsGlobalWriteTag1KB,
     ResourceGroupsGlobalWriteAndReadTag1KB,
@@ -217,8 +217,8 @@ impl TransactionTypeArg {
                     object_payload_size: 10 * 1024,
                 })
             },
-            TransactionTypeArg::VectorSplitOffAppendLen3000Size1 => {
-                call_custom_module(EntryPoints::VectorSplitOffAppend {
+            TransactionTypeArg::VectorTrimAppendLen3000Size1 => {
+                call_custom_module(EntryPoints::VectorTrimAppend {
                     vec_len: 3000,
                     element_len: 1,
                     index: 100,
diff --git a/crates/transaction-generator-lib/src/publishing/module_simple.rs b/crates/transaction-generator-lib/src/publishing/module_simple.rs
index 073029c357224..50979abe2878a 100644
--- a/crates/transaction-generator-lib/src/publishing/module_simple.rs
+++ b/crates/transaction-generator-lib/src/publishing/module_simple.rs
@@ -226,7 +226,7 @@ pub enum EntryPoints {
         num_objects: u64,
         object_payload_size: u64,
     },
-    VectorSplitOffAppend {
+    VectorTrimAppend {
         vec_len: u64,
         element_len: u64,
         index: u64,
@@ -316,7 +316,7 @@ impl EntryPoints {
             | EntryPoints::ModifyGlobalBoundedAggV2 { .. }
             | EntryPoints::CreateObjects { .. }
             | EntryPoints::CreateObjectsConflict { .. }
-            | EntryPoints::VectorSplitOffAppend { .. }
+            | EntryPoints::VectorTrimAppend { .. }
             | EntryPoints::VectorRemoveInsert { .. }
             | EntryPoints::VectorRangeMove { .. }
             | EntryPoints::TokenV1InitializeCollection
@@ -376,7 +376,7 @@ impl EntryPoints {
             EntryPoints::CreateObjects { .. } | EntryPoints::CreateObjectsConflict { .. } => {
                 "objects"
             },
-            EntryPoints::VectorSplitOffAppend { .. }
+            EntryPoints::VectorTrimAppend { .. }
             | EntryPoints::VectorRemoveInsert { .. }
             | EntryPoints::VectorRangeMove { .. } => "vector_example",
             EntryPoints::TokenV1InitializeCollection
@@ -558,7 +558,7 @@ impl EntryPoints {
                     bcs::to_bytes(other.expect("Must provide other")).unwrap(),
                 ],
             ),
-            EntryPoints::VectorSplitOffAppend {
+            EntryPoints::VectorTrimAppend {
                 vec_len,
                 element_len,
                 index,
@@ -572,8 +572,8 @@ impl EntryPoints {
             } => get_payload(
                 module_id,
                 ident_str!(
-                    if let EntryPoints::VectorSplitOffAppend { .. } = self {
-                        "test_split_off_append"
+                    if let EntryPoints::VectorTrimAppend { .. } = self {
+                        "test_trim_append"
                     } else {
                         "test_remove_insert"
                     }
@@ -869,7 +869,7 @@ impl EntryPoints {
             EntryPoints::CreateObjects { .. } | EntryPoints::CreateObjectsConflict { .. } => {
                 AutomaticArgs::Signer
             },
-            EntryPoints::VectorSplitOffAppend { .. }
+            EntryPoints::VectorTrimAppend { .. }
             | EntryPoints::VectorRemoveInsert { .. }
             | EntryPoints::VectorRangeMove { .. } => AutomaticArgs::None,
             EntryPoints::TokenV1InitializeCollection
diff --git a/testsuite/module-publish/src/packages/framework_usecases/sources/vector_example.move b/testsuite/module-publish/src/packages/framework_usecases/sources/vector_example.move
index 1308eddc1f5e9..82502274302a7 100644
--- a/testsuite/module-publish/src/packages/framework_usecases/sources/vector_example.move
+++ b/testsuite/module-publish/src/packages/framework_usecases/sources/vector_example.move
@@ -17,11 +17,11 @@ module 0xABCD::vector_example {
         vec
     }
 
-    public entry fun test_split_off_append(vec_len: u64, element_len: u64, index: u64, repeats: u64) {
+    public entry fun test_trim_append(vec_len: u64, element_len: u64, index: u64, repeats: u64) {
         let vec = generate_vec(vec_len, element_len);
 
         for (i in 0..repeats) {
-            let part = vector::split_off(&mut vec, index);
+            let part = vector::trim(&mut vec, index);
             vector::append(&mut vec, part);
         };
     }
diff --git a/testsuite/single_node_performance.py b/testsuite/single_node_performance.py
index 42d89c1e74211..69bd755474c1d 100755
--- a/testsuite/single_node_performance.py
+++ b/testsuite/single_node_performance.py
@@ -272,7 +272,7 @@ class RunGroupConfig:
     RunGroupConfig(key=RunGroupKey("no-op-fee-payer"), included_in=LAND_BLOCKING_AND_C),
     RunGroupConfig(key=RunGroupKey("no-op-fee-payer", module_working_set_size=50), included_in=Flow.CONTINUOUS),
 
-    RunGroupConfig(expected_tps=394, key=RunGroupKey("vector-split-off-append-len3000-size1"), included_in=Flow.CONTINUOUS, waived=True),
+    RunGroupConfig(expected_tps=394, key=RunGroupKey("vector-trim-append-len3000-size1"), included_in=Flow.CONTINUOUS, waived=True),
     RunGroupConfig(expected_tps=398, key=RunGroupKey("vector-remove-insert-len3000-size1"), included_in=Flow.CONTINUOUS, waived=True),
 
     RunGroupConfig(expected_tps=50000, key=RunGroupKey("coin_transfer_connected_components", executor_type="sharded"), key_extra=RunGroupKeyExtra(sharding_traffic_flags="--connected-tx-grps 5000", transaction_type_override=""), included_in=Flow.REPRESENTATIVE, waived=True),