Skip to content

Commit

Permalink
Add vector::move_range native function
Browse files Browse the repository at this point in the history
  • Loading branch information
igor-aptos committed Oct 9, 2024
1 parent 783b338 commit ae8e817
Show file tree
Hide file tree
Showing 9 changed files with 244 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::{
gas_feature_versions::{RELEASE_V1_18, RELEASE_V1_22},
gas_schedule::NativeGasParameters,
};
use aptos_gas_algebra::{InternalGas, InternalGasPerAbstractValueUnit, InternalGasPerByte};
use aptos_gas_algebra::{
InternalGas, InternalGasPerAbstractValueUnit, InternalGasPerArg, InternalGasPerByte,
};

crate::gas_schedule::macros::define_gas_parameters!(
MoveStdlibGasParameters,
Expand Down Expand Up @@ -40,6 +42,9 @@ crate::gas_schedule::macros::define_gas_parameters!(
[bcs_serialized_size_per_byte_serialized: InternalGasPerByte, { RELEASE_V1_18.. => "bcs.serialized_size.per_byte_serialized" }, 36],
[bcs_serialized_size_failure: InternalGas, { RELEASE_V1_18.. => "bcs.serialized_size.failure" }, 3676],

[vector_range_move_base: InternalGas, { RELEASE_V1_22.. => "vector.range_move.base" }, 4000],
[vector_range_move_per_index_moved: InternalGasPerArg, { RELEASE_V1_22.. => "vector.range_move.per_index_moved" }, 10],

[mem_swap_base: InternalGas, { RELEASE_V1_22.. => "mem.swap.base" }, 367],
[mem_swap_per_abs_val_unit: InternalGasPerAbstractValueUnit, { RELEASE_V1_22.. => "mem.swap.per_abs_val_unit"}, 14],
]
Expand Down
1 change: 0 additions & 1 deletion aptos-move/aptos-vm/src/natives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ pub fn aptos_natives_with_builder(
#[allow(unreachable_code)]
aptos_move_stdlib::natives::all_natives(CORE_CODE_ADDRESS, builder)
.into_iter()
.filter(|(_, name, _, _)| name.as_str() != "vector")
.chain(aptos_framework::natives::all_natives(
CORE_CODE_ADDRESS,
builder,
Expand Down
29 changes: 29 additions & 0 deletions aptos-move/framework/move-stdlib/doc/vector.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ the return on investment didn't seem worth it for these simple functions.
- [Function `pop_back`](#0x1_vector_pop_back)
- [Function `destroy_empty`](#0x1_vector_destroy_empty)
- [Function `swap`](#0x1_vector_swap)
- [Function `range_move`](#0x1_vector_range_move)
- [Function `singleton`](#0x1_vector_singleton)
- [Function `reverse`](#0x1_vector_reverse)
- [Function `reverse_slice`](#0x1_vector_reverse_slice)
Expand Down Expand Up @@ -340,6 +341,34 @@ Aborts if <code>i</code> or <code>j</code> is out of bounds.



</details>

<a id="0x1_vector_range_move"></a>

## Function `range_move`

Moves range of elements <code>[removal_position, removal_position + length)</code> from vector <code>from</code>,
to vector <code><b>to</b></code>, inserting them starting at the <code>insert_position</code>.
In the <code>from</code> vector, elements after the selected range are moved left to fill the hole
(i.e. range is removed, while the order of the rest of the elements is kept)
In the <code><b>to</b></code> vector, elements after the <code>insert_position</code> are moved the the right to make space for new elements
(i.e. range is inserted, while the order of the rest of the elements is kept)


<pre><code><b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_range_move">range_move</a>&lt;T&gt;(from: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;T&gt;, removal_position: u64, length: u64, <b>to</b>: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;T&gt;, insert_position: u64)
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>native</b> <b>public</b> <b>fun</b> <a href="vector.md#0x1_vector_range_move">range_move</a>&lt;T&gt;(from: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;T&gt;, removal_position: u64, length: u64, <b>to</b>: &<b>mut</b> <a href="vector.md#0x1_vector">vector</a>&lt;T&gt;, insert_position: u64);
</code></pre>



</details>

<a id="0x1_vector_singleton"></a>
Expand Down
8 changes: 8 additions & 0 deletions aptos-move/framework/move-stdlib/sources/vector.move
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ module std::vector {
/// Aborts if `i` or `j` is out of bounds.
native public fun swap<Element>(self: &mut vector<Element>, i: u64, j: u64);

/// Moves range of elements `[removal_position, removal_position + length)` from vector `from`,
/// to vector `to`, inserting them starting at the `insert_position`.
/// In the `from` vector, elements after the selected range are moved left to fill the hole
/// (i.e. range is removed, while the order of the rest of the elements is kept)
/// In the `to` vector, elements after the `insert_position` are moved the the right to make space for new elements
/// (i.e. range is inserted, while the order of the rest of the elements is kept)
native public fun range_move<T>(from: &mut vector<T>, removal_position: u64, length: u64, to: &mut vector<T>, insert_position: u64);

/// Return an vector of size one containing element `e`.
public fun singleton<Element>(e: Element): vector<Element> {
let v = empty();
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/framework/move-stdlib/src/natives/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub mod signer;
pub mod string;
#[cfg(feature = "testing")]
pub mod unit_test;
pub mod vector;

use aptos_native_interface::SafeNativeBuilder;
use move_core_types::account_address::AccountAddress;
Expand All @@ -37,6 +38,7 @@ pub fn all_natives(
add_natives!("mem", mem::make_all(builder));
add_natives!("signer", signer::make_all(builder));
add_natives!("string", string::make_all(builder));
add_natives!("vector", vector::make_all(builder));
#[cfg(feature = "testing")]
{
add_natives!("unit_test", unit_test::make_all(builder));
Expand Down
93 changes: 93 additions & 0 deletions aptos-move/framework/move-stdlib/src/natives/vector.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

// Copyright (c) The Diem Core Contributors
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

//! Implementation of native functions for utf8 strings.

use aptos_gas_schedule::gas_params::natives::move_stdlib::{
VECTOR_RANGE_MOVE_BASE, VECTOR_RANGE_MOVE_PER_INDEX_MOVED,
};
use aptos_native_interface::{
safely_pop_arg, RawSafeNative, SafeNativeBuilder, SafeNativeContext, SafeNativeError,
SafeNativeResult,
};
use move_core_types::gas_algebra::NumArgs;
use move_vm_runtime::native_functions::NativeFunction;
use move_vm_types::{
loaded_data::runtime_types::Type,
values::{Value, VectorRef},
};
use smallvec::{smallvec, SmallVec};
use std::collections::VecDeque;

/// The generic type supplied to aggregator snapshots is not supported.
pub const EINDEX_OUT_OF_BOUNDS: u64 = 0x03_0001;

/***************************************************************************************************
* native fun vector_move<T>(from: &mut vector<T>, removal_position: u64, length: u64, to: &mut vector<T>, insert_position: u64)
*
* gas cost: VECTOR_RANGE_MOVE_BASE + EINDEX_OUT_OF_BOUNDS * num_elements_to_move
*
**************************************************************************************************/
fn native_range_move(
context: &mut SafeNativeContext,
ty_args: Vec<Type>,
mut args: VecDeque<Value>,
) -> SafeNativeResult<SmallVec<[Value; 1]>> {
context.charge(VECTOR_RANGE_MOVE_BASE)?;

let map_err = |_| SafeNativeError::Abort {
abort_code: EINDEX_OUT_OF_BOUNDS,
};

Check warning on line 44 in aptos-move/framework/move-stdlib/src/natives/vector.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/move-stdlib/src/natives/vector.rs#L43-L44

Added lines #L43 - L44 were not covered by tests
let insert_position = usize::try_from(safely_pop_arg!(args, u64)).map_err(map_err)?;
let to = safely_pop_arg!(args, VectorRef);
let length = usize::try_from(safely_pop_arg!(args, u64)).map_err(map_err)?;
let removal_position = usize::try_from(safely_pop_arg!(args, u64)).map_err(map_err)?;
let from = safely_pop_arg!(args, VectorRef);

// need to fetch sizes here to charge upfront, and later in move_range, not sure if possible to combine
let to_len = to.len_usize_raw(&ty_args[0])?;
let from_len = from.len_usize_raw(&ty_args[0])?;

if removal_position
.checked_add(length)
.map_or(true, |end| end > from_len)
|| insert_position > to_len
{
return Err(SafeNativeError::Abort {
abort_code: EINDEX_OUT_OF_BOUNDS,
});

Check warning on line 62 in aptos-move/framework/move-stdlib/src/natives/vector.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/move-stdlib/src/natives/vector.rs#L60-L62

Added lines #L60 - L62 were not covered by tests
}

// We are moving all elements in the range, all elements after range, and all elements after insertion point.
// We are counting "length" of moving block twice, as it both gets moved out and moved in.
context.charge(
VECTOR_RANGE_MOVE_PER_INDEX_MOVED
* NumArgs::new(
(from_len - removal_position)
.checked_add(to_len - insert_position)
.and_then(|v| v.checked_add(length))
.ok_or_else(|| SafeNativeError::Abort {
abort_code: EINDEX_OUT_OF_BOUNDS,

Check warning on line 74 in aptos-move/framework/move-stdlib/src/natives/vector.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/move-stdlib/src/natives/vector.rs#L74

Added line #L74 was not covered by tests
})? as u64,
),
)?;

Check warning on line 77 in aptos-move/framework/move-stdlib/src/natives/vector.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/framework/move-stdlib/src/natives/vector.rs#L77

Added line #L77 was not covered by tests

from.move_range(removal_position, length, &to, insert_position, &ty_args[0])?;

Ok(smallvec![])
}

/***************************************************************************************************
* module
**************************************************************************************************/
pub fn make_all(
builder: &SafeNativeBuilder,
) -> impl Iterator<Item = (String, NativeFunction)> + '_ {
let natives = [("range_move", native_range_move as RawSafeNative)];

builder.make_named_natives(natives)
}
10 changes: 10 additions & 0 deletions aptos-move/framework/move-stdlib/tests/vector_tests.move
Original file line number Diff line number Diff line change
Expand Up @@ -954,4 +954,14 @@ module std::vector_tests {
let v = vector[MoveOnly {}];
vector::destroy(v, |m| { let MoveOnly {} = m; })
}

#[test]
fun test_range_move_ints() {
let v = vector[3, 4, 5, 6];
let w = vector[1, 2];

V::range_move(&mut v, 1, 2, &mut w, 1);
assert!(&v == &vector[3, 6], 0);
assert!(&w == &vector[1, 4, 5, 2], 0);
}
}
20 changes: 19 additions & 1 deletion third_party/move/move-vm/runtime/src/native_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use move_vm_types::{
loaded_data::runtime_types::Type, natives::function::NativeResult, values::Value,
};
use std::{
collections::{HashMap, VecDeque},
collections::{HashMap, HashSet, VecDeque},
fmt::Write,
sync::Arc,
};
Expand Down Expand Up @@ -81,8 +81,26 @@ impl NativeFunctions {
where
I: IntoIterator<Item = (AccountAddress, Identifier, Identifier, NativeFunction)>,
{
let vector_bytecode_instruction_methods = HashSet::from([
"empty",
"length",
"borrow",
"borrow_mut",
"push_back",
"pop_back",
"destroy_empty",
"swap",
]);

let mut map = HashMap::new();
for (addr, module_name, func_name, func) in natives.into_iter() {
if module_name.as_str() == "string"
&& vector_bytecode_instruction_methods.contains(func_name.as_str())
{
println!("ERROR: Tried to register as native a vector bytecode_instruction method {}, skipping.", func_name.as_str());
continue;

Check warning on line 101 in third_party/move/move-vm/runtime/src/native_functions.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/runtime/src/native_functions.rs#L100-L101

Added lines #L100 - L101 were not covered by tests
}

let modules = map.entry(addr).or_insert_with(HashMap::new);
let funcs = modules
.entry(module_name.into_string())
Expand Down
77 changes: 77 additions & 0 deletions third_party/move/move-vm/types/src/values/values_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,25 @@ fn check_elem_layout(ty: &Type, v: &Container) -> PartialVMResult<()> {
}

impl VectorRef {
pub fn len_usize_raw(&self, type_param: &Type) -> PartialVMResult<usize> {
let c: &Container = self.0.container();
check_elem_layout(type_param, c)?;

let len = match c {
Container::VecU8(r) => r.borrow().len(),
Container::VecU16(r) => r.borrow().len(),
Container::VecU32(r) => r.borrow().len(),

Check warning on line 2167 in third_party/move/move-vm/types/src/values/values_impl.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/values/values_impl.rs#L2165-L2167

Added lines #L2165 - L2167 were not covered by tests
Container::VecU64(r) => r.borrow().len(),
Container::VecU128(r) => r.borrow().len(),
Container::VecU256(r) => r.borrow().len(),
Container::VecBool(r) => r.borrow().len(),
Container::VecAddress(r) => r.borrow().len(),
Container::Vec(r) => r.borrow().len(),
Container::Locals(_) | Container::Struct(_) => unreachable!(),

Check warning on line 2174 in third_party/move/move-vm/types/src/values/values_impl.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/values/values_impl.rs#L2169-L2174

Added lines #L2169 - L2174 were not covered by tests
};
Ok(len)
}

pub fn len(&self, type_param: &Type) -> PartialVMResult<Value> {
let c: &Container = self.0.container();
check_elem_layout(type_param, c)?;
Expand Down Expand Up @@ -2302,6 +2321,64 @@ impl VectorRef {
self.0.mark_dirty();
Ok(())
}

pub fn move_range(
&self,
removal_position: usize,
length: usize,
to_self: &Self,
insert_position: usize,
type_param: &Type,
) -> PartialVMResult<()> {
let from_c = self.0.container();
let to_c = to_self.0.container();
check_elem_layout(type_param, from_c)?;
check_elem_layout(type_param, to_c)?;

macro_rules! move_range {
($from:expr, $to:expr) => {{
let mut from_v = $from.borrow_mut();
let mut to_v = $to.borrow_mut();

if removal_position.checked_add(length).map_or(true, |end| end > from_v.len())
|| insert_position > to_v.len() {
return Err(PartialVMError::new(StatusCode::VECTOR_OPERATION_ERROR)
.with_sub_status(INDEX_OUT_OF_BOUNDS));
}

// Short-circuit with faster implementation some of the common cases.
// This includes all non-direct calls to move-range (i.e. insert/remove/append/split_off inside vector).
if length == 1 {
to_v.insert(insert_position, from_v.remove(removal_position));
} else if removal_position == 0 && length == from_v.len() && insert_position == to_v.len() {
to_v.append(&mut from_v);
} else if (removal_position + length == from_v.len() && insert_position == to_v.len()) {
to_v.append(&mut from_v.split_off(removal_position));
} else {
to_v.splice(insert_position..insert_position, from_v.splice(removal_position..(removal_position + length), []));
}
}};
}

match (from_c, to_c) {
(Container::VecU8(from_r), Container::VecU8(to_r)) => move_range!(from_r, to_r),
(Container::VecU16(from_r), Container::VecU16(to_r)) => move_range!(from_r, to_r),
(Container::VecU32(from_r), Container::VecU32(to_r)) => move_range!(from_r, to_r),

Check warning on line 2366 in third_party/move/move-vm/types/src/values/values_impl.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/values/values_impl.rs#L2364-L2366

Added lines #L2364 - L2366 were not covered by tests
(Container::VecU64(from_r), Container::VecU64(to_r)) => move_range!(from_r, to_r),
(Container::VecU128(from_r), Container::VecU128(to_r)) => move_range!(from_r, to_r),
(Container::VecU256(from_r), Container::VecU256(to_r)) => move_range!(from_r, to_r),
(Container::VecBool(from_r), Container::VecBool(to_r)) => move_range!(from_r, to_r),
(Container::VecAddress(from_r), Container::VecAddress(to_r)) => {

Check warning on line 2371 in third_party/move/move-vm/types/src/values/values_impl.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/values/values_impl.rs#L2368-L2371

Added lines #L2368 - L2371 were not covered by tests
move_range!(from_r, to_r)
},
(Container::Vec(from_r), Container::Vec(to_r)) => move_range!(from_r, to_r),
(_, _) => unreachable!(),

Check warning on line 2375 in third_party/move/move-vm/types/src/values/values_impl.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-vm/types/src/values/values_impl.rs#L2374-L2375

Added lines #L2374 - L2375 were not covered by tests
}

self.0.mark_dirty();
to_self.0.mark_dirty();
Ok(())
}
}

impl Vector {
Expand Down

0 comments on commit ae8e817

Please sign in to comment.