split_off => trim

aptos-labs · Oct 9, 2024 · 6046016 · 6046016
1 parent 5d1ee9d
commit 6046016
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 94 deletions.
diff --git 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
@@ -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 <code>other</code> vector into the <code>self<
 
 </details>
 
-<a id="0x1_vector_split_off"></a>
+<a id="0x1_vector_trim"></a>
 
-## 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 <code>split_off</code>.
 
 
-<pre><code><b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_split_off">split_off</a>&lt;Element&gt;(self: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;, at: u64): <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;
+<pre><code><b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_trim">trim</a>&lt;Element&gt;(self: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;, new_len: u64): <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;
 </code></pre>
 
 
@@ -550,15 +550,15 @@ with its previous capacity unchanged.
 <summary>Implementation</summary>
 
 
-<pre><code><b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_split_off">split_off</a>&lt;Element&gt;(self: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;, at: u64): <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt; {
+<pre><code><b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_trim">trim</a>&lt;Element&gt;(self: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;, new_len: u64): <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt; {
     <b>let</b> len = <a href="vector.md#0x1_vector_length">length</a>(self);
-    <b>assert</b>!(at &lt;= len, <a href="vector.md#0x1_vector_EINDEX_OUT_OF_BOUNDS">EINDEX_OUT_OF_BOUNDS</a>);
+    <b>assert</b>!(new_len &lt;= len, <a href="vector.md#0x1_vector_EINDEX_OUT_OF_BOUNDS">EINDEX_OUT_OF_BOUNDS</a>);
 
     <b>let</b> other = <a href="vector.md#0x1_vector_empty">empty</a>();
     <b>if</b> (<a href="vector.md#0x1_vector_USE_MOVE_RANGE">USE_MOVE_RANGE</a>) {
-        <a href="vector.md#0x1_vector_range_move">range_move</a>(self, at, len - at, &<b>mut</b> other, 0);
+        <a href="vector.md#0x1_vector_range_move">range_move</a>(self, new_len, len - new_len, &<b>mut</b> other, 0);
     } <b>else</b> {
-        <b>while</b> (len &gt; at) {
+        <b>while</b> (len &gt; new_len) {
             <a href="vector.md#0x1_vector_push_back">push_back</a>(&<b>mut</b> other, <a href="vector.md#0x1_vector_pop_back">pop_back</a>(self));
             len = len - 1;
         };
@@ -571,33 +571,6 @@ with its previous capacity unchanged.
 
 
 
-</details>
-
-<a id="0x1_vector_trim"></a>
-
-## Function `trim`
-
-Trim a vector to a smaller size, returning the evicted elements in order
-
-
-<pre><code><b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_trim">trim</a>&lt;Element&gt;(self: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;, new_len: u64): <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;
-</code></pre>
-
-
-
-<details>
-<summary>Implementation</summary>
-
-
-<pre><code><b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_trim">trim</a>&lt;Element&gt;(self: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt;, new_len: u64): <a href="vector.md#0x1_vector">vector</a>&lt;Element&gt; {
-    <b>let</b> res = <a href="vector.md#0x1_vector_trim_reverse">trim_reverse</a>(self, new_len);
-    <a href="vector.md#0x1_vector_reverse">reverse</a>(&<b>mut</b> res);
-    res
-}
-</code></pre>
-
-
-
 </details>
 
 <a id="0x1_vector_trim_reverse"></a>

diff --git 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<Element>(self: &mut vector<Element>, at: u64): vector<Element> {
+    /// In many languages this is also called `split_off`.
+    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;
             };
@@ -160,13 +161,6 @@ module std::vector {
 
         other
     }
-
-    /// 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> {
-        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
@@ -105,14 +105,32 @@ 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() {
         let v = vector[1];
         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
@@ -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
@@ -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
@@ -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
@@ -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),