diff --git a/aptos-move/e2e-benchmark/data/calibration_values.tsv b/aptos-move/e2e-benchmark/data/calibration_values.tsv index 8368a691fea38f..6dcad1f949b796 100644 --- a/aptos-move/e2e-benchmark/data/calibration_values.tsv +++ b/aptos-move/e2e-benchmark/data/calibration_values.tsv @@ -33,11 +33,11 @@ VectorRangeMove { vec_len: 3000, element_len: 1, index: 1000, move_len: 500, rep VectorTrimAppend { vec_len: 100, element_len: 100, index: 0, repeats: 0 } 6 0.925 1.001 277.0 VectorTrimAppend { vec_len: 100, element_len: 100, index: 10, repeats: 1000 } 6 0.925 1.001 12146.6 VectorRangeMove { vec_len: 100, element_len: 100, index: 50, move_len: 10, repeats: 1000 } 6 0.925 1.001 7098 -MapInsertRemove { len: 10, repeats: 0, use_simple_map: false } 6 0.925 1.001 378 -MapInsertRemove { len: 10, repeats: 100, use_simple_map: false } 6 0.925 1.001 8184 -MapInsertRemove { len: 10, repeats: 100, use_simple_map: true } 6 0.925 1.001 6419 -MapInsertRemove { len: 100, repeats: 0, use_simple_map: false } 6 0.925 1.001 5094 -MapInsertRemove { len: 100, repeats: 100, use_simple_map: false } 6 0.925 1.001 15838 -MapInsertRemove { len: 100, repeats: 100, use_simple_map: true } 6 0.925 1.001 30962 -MapInsertRemove { len: 1000, repeats: 0, use_simple_map: false } 6 0.925 1.001 66878 -MapInsertRemove { len: 1000, repeats: 100, use_simple_map: false } 6 0.925 1.001 79826 +MapInsertRemove { len: 10, repeats: 0, map_type: OrderedMap } 6 0.925 1.001 378 +MapInsertRemove { len: 10, repeats: 100, map_type: OrderedMap } 6 0.925 1.001 8184 +MapInsertRemove { len: 10, repeats: 100, map_type: SimpleMap } 6 0.925 1.001 6419 +MapInsertRemove { len: 100, repeats: 0, map_type: OrderedMap } 6 0.925 1.001 5094 +MapInsertRemove { len: 100, repeats: 100, map_type: OrderedMap } 6 0.925 1.001 15838 +MapInsertRemove { len: 100, repeats: 100, map_type: SimpleMap } 6 0.925 1.001 30962 +MapInsertRemove { len: 1000, repeats: 0, map_type: OrderedMap } 6 0.925 1.001 66878 +MapInsertRemove { len: 1000, repeats: 100, map_type: OrderedMap } 6 0.925 1.001 79826 diff --git a/aptos-move/e2e-benchmark/src/main.rs b/aptos-move/e2e-benchmark/src/main.rs index c8d987a2b2556f..3b2bd02c139629 100644 --- a/aptos-move/e2e-benchmark/src/main.rs +++ b/aptos-move/e2e-benchmark/src/main.rs @@ -7,7 +7,7 @@ use aptos_language_e2e_tests::{ }; use aptos_transaction_generator_lib::{ publishing::{ - module_simple::{AutomaticArgs, LoopType, MultiSigConfig}, + module_simple::{AutomaticArgs, LoopType, MapType, MultiSigConfig}, publish_util::{Package, PackageHandler}, }, EntryPoints, @@ -239,42 +239,42 @@ fn main() { EntryPoints::MapInsertRemove { len: 10, repeats: 0, - use_simple_map: false, + map_type: MapType::OrderedMap, }, EntryPoints::MapInsertRemove { len: 10, repeats: 100, - use_simple_map: false, + map_type: MapType::OrderedMap, }, EntryPoints::MapInsertRemove { len: 10, repeats: 100, - use_simple_map: true, + map_type: MapType::SimpleMap, }, EntryPoints::MapInsertRemove { len: 100, repeats: 0, - use_simple_map: false, + map_type: MapType::OrderedMap, }, EntryPoints::MapInsertRemove { len: 100, repeats: 100, - use_simple_map: false, + map_type: MapType::OrderedMap, }, EntryPoints::MapInsertRemove { len: 100, repeats: 100, - use_simple_map: true, + map_type: MapType::SimpleMap, }, EntryPoints::MapInsertRemove { len: 1000, repeats: 0, - use_simple_map: false, + map_type: MapType::OrderedMap, }, EntryPoints::MapInsertRemove { len: 1000, repeats: 100, - use_simple_map: false, + map_type: MapType::OrderedMap, }, ]; diff --git a/aptos-move/framework/aptos-stdlib/doc/big_ordered_map.md b/aptos-move/framework/aptos-stdlib/doc/big_ordered_map.md index 54146fc8acca36..019e1f3ff3760b 100644 --- a/aptos-move/framework/aptos-stdlib/doc/big_ordered_map.md +++ b/aptos-move/framework/aptos-stdlib/doc/big_ordered_map.md @@ -36,6 +36,7 @@ once it contains enough keys - [Function `find`](#0x1_big_ordered_map_find) - [Function `contains`](#0x1_big_ordered_map_contains) - [Function `borrow`](#0x1_big_ordered_map_borrow) +- [Function `borrow_mut`](#0x1_big_ordered_map_borrow_mut) - [Function `new_begin_iter`](#0x1_big_ordered_map_new_begin_iter) - [Function `new_end_iter`](#0x1_big_ordered_map_new_end_iter) - [Function `iter_is_begin`](#0x1_big_ordered_map_iter_is_begin) @@ -115,13 +116,13 @@ So Leaf node contains multiple values, not just one.
-prev: storage_slots_allocator::RefToSlot +prev: u64
-next: storage_slots_allocator::RefToSlot +next: u64
@@ -235,7 +236,7 @@ An iterator to iterate all keys in the BigOrderedMap.
-node_index: storage_slots_allocator::RefToSlot +node_index: u64
The node index of the iterator pointing to. @@ -291,28 +292,22 @@ The BigOrderedMap data structure. root: big_ordered_map::Node<K, V>
- -
-
-root_index: storage_slots_allocator::RefToSlot -
-
- The node index of the root node. + Root node. It is stored directly in the resource itself, unlike all other nodes.
nodes: storage_slots_allocator::StorageSlotsAllocator<big_ordered_map::Node<K, V>>
- Mapping of node_index -> node. + Storage of all non-root nodes. They are stored in separate storage slots.
-min_leaf_index: storage_slots_allocator::RefToSlot +min_leaf_index: u64
The node index of the leftmost node.
-max_leaf_index: storage_slots_allocator::RefToSlot +max_leaf_index: u64
The node index of the rightmost node. @@ -352,15 +347,26 @@ The BigOrderedMap data structure. +Internal errors. + + +
const EINTERNAL_INVARIANT_BROKEN: u64 = 20;
+
+ + + + + -
const EINTERNAL_INVARIANT_BROKEN: u64 = 7;
+
const NULL_INDEX: u64 = 0;
 
+Trying to do an operation on an Iterator that would go out of bounds
const EITER_OUT_OF_BOUNDS: u64 = 3;
@@ -417,6 +423,7 @@ Map key is not found
 
 
 
+Trying to insert too large of an object into the mp.
 
 
 
const EARGUMENT_BYTES_TOO_LARGE: u64 = 6;
@@ -424,8 +431,20 @@ Map key is not found
 
 
 
+
+
+borrow_mut requires that key and value types have constant size
+(otherwise it wouldn't be able to guarantee size requirements are not violated)
+
+
+
const EBORROW_MUT_REQUIRES_CONSTANT_KV_SIZE: u64 = 7;
+
+ + + +The provided configuration parameter is invalid.
const EINVALID_CONFIG_PARAMETER: u64 = 4;
@@ -435,6 +454,7 @@ Map key is not found
 
 
 
+Map isn't empty
 
 
 
const EMAP_NOT_EMPTY: u64 = 5;
@@ -460,6 +480,15 @@ Map key is not found
 
 
 
+
+
+
+
+
const ROOT_INDEX: u64 = 1;
+
+ + + ## Function `new` @@ -507,16 +536,18 @@ If 0 is passed, then it is dynamically computed based on size of first key and v assert!(leaf_max_degree == 0 || leaf_max_degree >= DEFAULT_LEAF_MIN_DEGREE, error::invalid_argument(EINVALID_CONFIG_PARAMETER)); assert!(reuse_slots || num_to_preallocate == 0, error::invalid_argument(EINVALID_CONFIG_PARAMETER)); + // Assert that storage_slots_allocator special indices are aligned: + assert!(storage_slots_allocator::is_null_index(NULL_INDEX), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + assert!(storage_slots_allocator::is_special_unused_index(ROOT_INDEX), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + let nodes = storage_slots_allocator::new(storage_slots_allocator::new_config(reuse_slots, num_to_preallocate)); - let root_ref = storage_slots_allocator::special_ref(); let self = BigOrderedMap::BPlusTreeMap { root: new_node(/*is_leaf=*/true), - root_index: root_ref, nodes: nodes, - min_leaf_index: root_ref, - max_leaf_index: root_ref, - constant_kv_size: false, + min_leaf_index: ROOT_INDEX, + max_leaf_index: ROOT_INDEX, + constant_kv_size: false, // Will be initialized in validate_static_size_and_init_max_degrees below. inner_max_degree: inner_max_degree, leaf_max_degree: leaf_max_degree }; @@ -546,9 +577,11 @@ Destroys the map if it's empty, otherwise aborts.
public fun destroy_empty<K: store, V: store>(self: BigOrderedMap<K, V>) {
-    let BigOrderedMap::BPlusTreeMap { root, nodes, root_index: _, min_leaf_index: _, max_leaf_index: _, constant_kv_size: _, inner_max_degree: _, leaf_max_degree: _ } = self;
+    let BigOrderedMap::BPlusTreeMap { root, nodes, min_leaf_index: _, max_leaf_index: _, constant_kv_size: _, inner_max_degree: _, leaf_max_degree: _ } = self;
     root.destroy_empty_node();
-    nodes.destroy();
+    // If root node is empty, then we know that no storage slots are used,
+    // and so we can safely destroy all nodes.
+    nodes.destroy_known_empty_unsafe();
 }
 
@@ -635,6 +668,15 @@ Aborts if there is no entry for key.
public fun remove<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, key: &K): V {
+    // Optimize case where only root node exists
+    // (optimizes out borrowing and path creation in `find_leaf_with_path`)
+    if (self.root.is_leaf) {
+        let Child::Leaf {
+            value,
+        } = self.root.children.remove(key);
+        return value;
+    };
+
     let path_to_leaf = self.find_leaf_with_path(key);
 
     assert!(!path_to_leaf.is_empty(), error::invalid_argument(EKEY_NOT_FOUND));
@@ -642,7 +684,6 @@ Aborts if there is no entry for key.
     let Child::Leaf {
         value,
     } = self.remove_at(path_to_leaf, key);
-
     value
 }
 
@@ -670,7 +711,7 @@ key, or an end iterator if such element doesn't exist.
public fun lower_bound<K: drop + copy + store, V: store>(self: &BigOrderedMap<K, V>, key: &K): Iterator<K> {
     let leaf = self.find_leaf(key);
-    if (leaf.ref_is_null()) {
+    if (leaf == NULL_INDEX) {
         return self.new_end_iter()
     };
 
@@ -783,6 +824,36 @@ Returns a reference to the element with its key, aborts if the key is not found.
 
 
 
+
+
+
+
+## Function `borrow_mut`
+
+Returns a mutable reference to the element with its key at the given index, aborts if the key is not found.
+
+
+
public fun borrow_mut<K: copy, drop, store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, key: K): &mut V
+
+ + + +
+Implementation + + +
public fun borrow_mut<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, key: K): &mut V {
+    assert!(self.constant_kv_size, error::invalid_argument(EBORROW_MUT_REQUIRES_CONSTANT_KV_SIZE));
+    let iter = self.find(&key);
+
+    assert!(iter.iter_is_end(self), error::invalid_argument(EKEY_NOT_FOUND));
+    let children = &mut self.borrow_node_mut(iter.node_index).children;
+    &mut iter.child_iter.iter_borrow_mut(children).value
+}
+
+ + +
@@ -946,12 +1017,14 @@ Requires the map is not changed after the input iterator is generated. let child_iter = self.child_iter.iter_next(&node.children); if (!child_iter.iter_is_end(&node.children)) { + // next is in the same leaf node let iter_key = *child_iter.iter_borrow_key(&node.children); return new_iter(node_index, child_iter, iter_key); }; + // next is in a different leaf node let next_index = node.next; - if (!next_index.ref_is_null()) { + if (next_index != NULL_INDEX) { let next_node = map.borrow_node(next_index); let child_iter = next_node.children.new_begin_iter(); @@ -993,6 +1066,7 @@ Requires the map is not changed after the input iterator is generated. let node = map.borrow_node(node_index); if (!self.child_iter.iter_is_begin(&node.children)) { + // next is in the same leaf node let child_iter = self.child_iter.iter_prev(&node.children); let key = *child_iter.iter_borrow_key(&node.children); return new_iter(node_index, child_iter, key); @@ -1000,8 +1074,9 @@ Requires the map is not changed after the input iterator is generated. node.prev }; - assert!(!prev_index.ref_is_null(), error::invalid_argument(EITER_OUT_OF_BOUNDS)); + assert!(prev_index != NULL_INDEX, error::invalid_argument(EITER_OUT_OF_BOUNDS)); + // next is in a different leaf node let prev_node = map.borrow_node(prev_index); let prev_children = &prev_node.children; @@ -1019,9 +1094,10 @@ Requires the map is not changed after the input iterator is generated. ## Function `borrow_node` +Borrow a node, given an index. Works for both root (i.e. inline) node and separately stored nodes -
fun borrow_node<K: store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, node: storage_slots_allocator::RefToSlot): &big_ordered_map::Node<K, V>
+
fun borrow_node<K: store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, node_index: u64): &big_ordered_map::Node<K, V>
 
@@ -1030,11 +1106,11 @@ Requires the map is not changed after the input iterator is generated. Implementation -
inline fun borrow_node<K: store, V: store>(self: &BigOrderedMap<K, V>, node: RefToSlot): &Node<K, V> {
-    if (self.root_index == node) {
+
inline fun borrow_node<K: store, V: store>(self: &BigOrderedMap<K, V>, node_index: u64): &Node<K, V> {
+    if (node_index == ROOT_INDEX) {
         &self.root
     } else {
-        self.nodes.borrow(node)
+        self.nodes.borrow(node_index)
     }
 }
 
@@ -1047,9 +1123,10 @@ Requires the map is not changed after the input iterator is generated. ## Function `borrow_node_mut` +Borrow a node mutably, given an index. Works for both root (i.e. inline) node and separately stored nodes -
fun borrow_node_mut<K: store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, node: storage_slots_allocator::RefToSlot): &mut big_ordered_map::Node<K, V>
+
fun borrow_node_mut<K: store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, node_index: u64): &mut big_ordered_map::Node<K, V>
 
@@ -1058,11 +1135,11 @@ Requires the map is not changed after the input iterator is generated. Implementation -
inline fun borrow_node_mut<K: store, V: store>(self: &mut BigOrderedMap<K, V>, node: RefToSlot): &mut Node<K, V> {
-    if (self.root_index == node) {
+
inline fun borrow_node_mut<K: store, V: store>(self: &mut BigOrderedMap<K, V>, node_index: u64): &mut Node<K, V> {
+    if (node_index == ROOT_INDEX) {
         &mut self.root
     } else {
-        self.nodes.borrow_mut(node)
+        self.nodes.borrow_mut(node_index)
     }
 }
 
@@ -1091,12 +1168,26 @@ Requires the map is not changed after the input iterator is generated. self.validate_dynamic_size_and_init_max_degrees(&key, &value); }; + // Optimize case where only root node exists + // (optimizes out borrowing and path creation in `find_leaf_with_path`) + if (self.root.is_leaf) { + let children = &mut self.root.children; + let current_size = children.length(); + + if (current_size < (self.leaf_max_degree as u64)) { + let result = children.upsert(key, new_leaf_child(value)); + assert!(allow_overwrite || result.is_none(), error::invalid_argument(EKEY_ALREADY_EXISTS)); + return result; + }; + }; + let path_to_leaf = self.find_leaf_with_path(&key); if (path_to_leaf.is_empty()) { // In this case, the key is greater than all keys in the map. - - let current = self.root_index; + // So we need to update `key` in the pointers to the last child, + // to maintain the invariant of `add_at` + let current = ROOT_INDEX; loop { path_to_leaf.push_back(current); @@ -1106,13 +1197,11 @@ Requires the map is not changed after the input iterator is generated. break; }; let last_value = current_node.children.new_end_iter().iter_prev(¤t_node.children).iter_remove(&mut current_node.children); - current = last_value.node_index.stored_as_ref(); + current = last_value.node_index.stored_to_index(); current_node.children.add(key, last_value); }; }; - // aptos_std::debug::print(&std::string::utf8(b"add_or_upsert_impl::path_to_leaf")); - // aptos_std::debug::print(&path_to_leaf); self.add_at(path_to_leaf, key, new_leaf_child(value), allow_overwrite) }
@@ -1203,6 +1292,7 @@ Requires the map is not changed after the input iterator is generated. self.leaf_max_degree = max(min(MAX_DEGREE, DEFAULT_TARGET_NODE_SIZE / entry_size), DEFAULT_LEAF_MIN_DEGREE as u64) as u16; }; + // Make sure that no nodes can exceed the upper size limit. assert!(key_size * (self.inner_max_degree as u64) <= MAX_NODE_BYTES, error::invalid_argument(EARGUMENT_BYTES_TOO_LARGE)); assert!(entry_size * (self.leaf_max_degree as u64) <= MAX_NODE_BYTES, error::invalid_argument(EARGUMENT_BYTES_TOO_LARGE)); } @@ -1285,8 +1375,8 @@ Requires the map is not changed after the input iterator is generated. Node { is_leaf: is_leaf, children: ordered_map::new(), - prev: storage_slots_allocator::null_ref(), - next: storage_slots_allocator::null_ref(), + prev: NULL_INDEX, + next: NULL_INDEX, } }
@@ -1314,8 +1404,8 @@ Requires the map is not changed after the input iterator is generated. Node { is_leaf: is_leaf, children: children, - prev: storage_slots_allocator::null_ref(), - next: storage_slots_allocator::null_ref(), + prev: NULL_INDEX, + next: NULL_INDEX, } }
@@ -1382,7 +1472,7 @@ Requires the map is not changed after the input iterator is generated. -
fun new_iter<K>(node_index: storage_slots_allocator::RefToSlot, child_iter: ordered_map::Iterator, key: K): big_ordered_map::Iterator<K>
+
fun new_iter<K>(node_index: u64, child_iter: ordered_map::Iterator, key: K): big_ordered_map::Iterator<K>
 
@@ -1391,7 +1481,7 @@ Requires the map is not changed after the input iterator is generated. Implementation -
fun new_iter<K>(node_index: RefToSlot, child_iter: ordered_map::Iterator, key: K): Iterator<K> {
+
fun new_iter<K>(node_index: u64, child_iter: ordered_map::Iterator, key: K): Iterator<K> {
     Iterator::Some {
         node_index: node_index,
         child_iter: child_iter,
@@ -1408,9 +1498,12 @@ Requires the map is not changed after the input iterator is generated.
 
 ## Function `find_leaf`
 
+Find leaf where the given key would fall in.
+So the largest leaf with it's max_key <= key.
+return NULL_INDEX if key is larger than any key currently stored in the map.
 
 
-
fun find_leaf<K: copy, drop, store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, key: &K): storage_slots_allocator::RefToSlot
+
fun find_leaf<K: copy, drop, store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, key: &K): u64
 
@@ -1419,9 +1512,9 @@ Requires the map is not changed after the input iterator is generated. Implementation -
fun find_leaf<K: drop + copy + store, V: store>(self: &BigOrderedMap<K, V>, key: &K): RefToSlot {
-    let current = self.root_index;
-    while (!current.ref_is_null()) {
+
fun find_leaf<K: drop + copy + store, V: store>(self: &BigOrderedMap<K, V>, key: &K): u64 {
+    let current = ROOT_INDEX;
+    loop {
         let node = self.borrow_node(current);
         if (node.is_leaf) {
             return current;
@@ -1429,13 +1522,11 @@ Requires the map is not changed after the input iterator is generated.
         let children = &node.children;
         let child_iter = children.lower_bound(key);
         if (child_iter.iter_is_end(children)) {
-            return storage_slots_allocator::null_ref();
+            return NULL_INDEX;
         } else {
-            current = child_iter.iter_borrow(children).node_index.stored_as_ref();
-        }
-    };
-
-    storage_slots_allocator::null_ref()
+            current = child_iter.iter_borrow(children).node_index.stored_to_index();
+        };
+    }
 }
 
@@ -1447,9 +1538,13 @@ Requires the map is not changed after the input iterator is generated. ## Function `find_leaf_with_path` +Find leaf where the given key would fall in. +So the largest leaf with it's max_key <= key. +Return the path from root to that leaf (including the leaf itself) +Return empty path if key is larger than any key currently stored in the map. -
fun find_leaf_with_path<K: copy, drop, store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, key: &K): vector<storage_slots_allocator::RefToSlot>
+
fun find_leaf_with_path<K: copy, drop, store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, key: &K): vector<u64>
 
@@ -1458,11 +1553,11 @@ Requires the map is not changed after the input iterator is generated. Implementation -
fun find_leaf_with_path<K: drop + copy + store, V: store>(self: &BigOrderedMap<K, V>, key: &K): vector<RefToSlot> {
+
fun find_leaf_with_path<K: drop + copy + store, V: store>(self: &BigOrderedMap<K, V>, key: &K): vector<u64> {
     let vec = vector::empty();
 
-    let current = self.root_index;
-    while (!current.ref_is_null()) {
+    let current = ROOT_INDEX;
+    loop {
         vec.push_back(current);
 
         let node = self.borrow_node(current);
@@ -1474,11 +1569,9 @@ Requires the map is not changed after the input iterator is generated.
         if (child_iter.iter_is_end(children)) {
             return vector::empty();
         } else {
-            current = child_iter.iter_borrow(children).node_index.stored_as_ref();
-        }
-    };
-
-    abort error::invalid_state(EINTERNAL_INVARIANT_BROKEN)
+            current = child_iter.iter_borrow(children).node_index.stored_to_index();
+        };
+    }
 }
 
@@ -1518,9 +1611,15 @@ Requires the map is not changed after the input iterator is generated. ## Function `add_at` +Add a given child to a given node (last in the path_to_node), and update/rebalance the tree as necessary. +It is required that key pointers to the child node, on the path_to_node are greater or equal to the given key. +That means if we are adding a key larger than any currently existing in the map - we needed +to update key pointers on the path_to_node to include it, before calling this method. +If allow_overwrite is not set, function will abort if key is already present. -
fun add_at<K: copy, drop, store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, path_to_node: vector<storage_slots_allocator::RefToSlot>, key: K, child: big_ordered_map::Child<V>, allow_overwrite: bool): option::Option<big_ordered_map::Child<V>>
+
+
fun add_at<K: copy, drop, store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, path_to_node: vector<u64>, key: K, child: big_ordered_map::Child<V>, allow_overwrite: bool): option::Option<big_ordered_map::Child<V>>
 
@@ -1529,9 +1628,13 @@ Requires the map is not changed after the input iterator is generated. Implementation -
fun add_at<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, path_to_node: vector<RefToSlot>, key: K, child: Child<V>, allow_overwrite: bool): Option<Child<V>> {
+
fun add_at<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, path_to_node: vector<u64>, key: K, child: Child<V>, allow_overwrite: bool): Option<Child<V>> {
+    // Last node in the path is one where we need to add the child to.
     let node_index = path_to_node.pop_back();
     {
+        // First check if we can perform this operation, without changing structure of the tree (i.e. without adding any nodes).
+
+        // For that we can just borrow the single node
         let node = self.borrow_node_mut(node_index);
         let children = &mut node.children;
         let current_size = children.length();
@@ -1543,6 +1646,7 @@ Requires the map is not changed after the input iterator is generated.
         };
 
         if (current_size < max_degree) {
+            // Adding a child to a current node doesn't exceed the size, so we can just do that.
             let result = children.upsert(key, child);
 
             if (node.is_leaf) {
@@ -1554,63 +1658,90 @@ Requires the map is not changed after the input iterator is generated.
             };
         };
 
-        if (allow_overwrite) {
-            let iter = children.find(&key);
-            if (!iter.iter_is_end(children)) {
-                return option::some(iter.iter_replace(children, child));
-            }
+        // If we cannot add more nodes without exceeding the size,
+        // but node with `key` already exists, we either need to replace or abort.
+        let iter = children.find(&key);
+        if (!iter.iter_is_end(children)) {
+            assert!(node.is_leaf, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
+            assert!(allow_overwrite, error::invalid_argument(EKEY_ALREADY_EXISTS));
+
+            return option::some(iter.iter_replace(children, child));
         }
     };
 
     // # of children in the current node exceeds the threshold, need to split into two nodes.
 
-    let (right_node_slot, node) = if (path_to_node.is_empty()) {
-        // If we are at the root, we need to move root node to become a child and have a new root node.
-
-        assert!(node_index == self.root_index, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
-        // aptos_std::debug::print(&std::string::utf8(b"changing root"));
+    // If we are at the root, we need to move root node to become a child and have a new root node,
+    // in order to be able to split the node on the level it is.
+    let (reserved_slot, node) = if (node_index == ROOT_INDEX) {
+        assert!(path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
 
         // Splitting root now, need to create a new root.
-        // We keep root_index always the same
+        // Since root is stored direclty in the resource, we will swap-in the new node there.
         let new_root_node = new_node<K, V>(/*is_leaf=*/false);
 
-        let (replacement_node_stored_slot, replacement_node_slot) = self.nodes.reserve_slot();
-        // aptos_std::debug::print(&replacement_node_slot);
+        // Reserve a slot where the current root will be moved to.
+        let (replacement_node_slot, replacement_node_reserved_slot) = self.nodes.reserve_slot();
 
-        let root_children = &self.root.children;
-        let max_element = *root_children.new_end_iter().iter_prev(root_children).iter_borrow_key(root_children);
-        if (cmp::compare(&max_element, &key).is_less_than()) {
-            max_element = key;
+        let max_key = {
+            let root_children = &self.root.children;
+            let max_key = *root_children.new_end_iter().iter_prev(root_children).iter_borrow_key(root_children);
+            // need to check if key is largest, as invariant is that "parent's pointers" have been updated,
+            // but key itself can be larger than all previous ones.
+            if (cmp::compare(&max_key, &key).is_lt()) {
+                max_key = key;
+            };
+            max_key
         };
-        new_root_node.children.add(max_element, new_inner_child(replacement_node_stored_slot));
-
-        // aptos_std::debug::print(&cur_node_slot);
-        path_to_node.push_back(self.root_index);
-
+        // New root will have start with a single child - the existing root (which will be at replacement location).
+        new_root_node.children.add(max_key, new_inner_child(replacement_node_slot));
         let node = mem::replace(&mut self.root, new_root_node);
 
-        let replacement_ref = replacement_node_slot.reserved_as_ref();
+        // we moved the currently processing node one level down, so we need to update the path
+        path_to_node.push_back(ROOT_INDEX);
+
+        let replacement_index = replacement_node_reserved_slot.reserved_to_index();
         if (node.is_leaf) {
-            self.min_leaf_index = replacement_ref;
-            self.max_leaf_index = replacement_ref;
+            // replacement node is the only leaf, so we update the pointers:
+            self.min_leaf_index = replacement_index;
+            self.max_leaf_index = replacement_index;
         };
-        (replacement_node_slot, node)
+        (replacement_node_reserved_slot, node)
     } else {
-        let (cur_node_slot, node) = self.nodes.remove_and_reserve(node_index);
-        (cur_node_slot, node)
+        // In order to work on multiple nodes at the same time, we cannot borrow_mut, and need to be
+        // remove_and_reserve existing node.
+        let (cur_node_reserved_slot, node) = self.nodes.remove_and_reserve(node_index);
+        (cur_node_reserved_slot, node)
     };
 
-    // aptos_std::debug::print(&std::string::utf8(b"node that needs to be split"));
-    // aptos_std::debug::print(&node);
-
+    // move node_index out of scope, to make sure we don't accidentally access it, as we are done with it.
+    // (i.e. we should be using `reserved_slot` instead).
     move node_index;
-    let is_leaf = node.is_leaf;
-    let children = &mut node.children;
 
-    let right_node_ref = right_node_slot.reserved_as_ref();
-    let next = &mut node.next;
-    let prev = &mut node.prev;
+    // Now we can perform the split at the current level, as we know we are not at the root level.
+    assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
+
+    // Parent has a reference under max key to the current node, so existing index
+    // needs to be the right node.
+    // Since ordered_map::trim moves from the end (i.e. smaller keys stay),
+    // we are going to put the contents of the current node on the left side,
+    // and create a new right node.
+    // So if we had before (node_index, node), we will change that to end up having:
+    // (new_left_node_index, node trimmed off) and (node_index, new node with trimmed off children)
+    //
+    // So let's rename variables cleanly:
+    let right_node_reserved_slot = reserved_slot;
+    let left_node = node;
+
+
+    let is_leaf = left_node.is_leaf;
+    let left_children = &mut left_node.children;
+
+    let right_node_index = right_node_reserved_slot.reserved_to_index();
+    let left_next = &mut left_node.next;
+    let left_prev = &mut left_node.prev;
 
+    // compute the target size for the left node:
     let max_degree = if (is_leaf) {
         self.leaf_max_degree as u64
     } else {
@@ -1618,40 +1749,42 @@ Requires the map is not changed after the input iterator is generated.
     };
     let target_size = (max_degree + 1) / 2;
 
-    children.add(key, child);
-    let new_node_children = children.trim(target_size);
+    // Add child (which will exceed the size), and then trim off to create two sets of children of correct sizes.
+    left_children.add(key, child);
+    let right_node_children = left_children.trim(target_size);
 
-    assert!(children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
-    assert!(new_node_children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
+    assert!(left_children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
+    assert!(right_node_children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
 
-    let right_node = new_node_with_children(is_leaf, new_node_children);
+    let right_node = new_node_with_children(is_leaf, right_node_children);
 
-    let (left_node_stored_slot, left_node_slot) = self.nodes.reserve_slot();
-    let left_node_ref = left_node_stored_slot.stored_as_ref();
-    right_node.next = *next;
-    *next = right_node_ref;
-    right_node.prev = left_node_ref;
-    if (!prev.ref_is_null()) {
-        self.nodes.borrow_mut(*prev).next = left_node_ref;
-    };
+    let (left_node_slot, left_node_reserved_slot) = self.nodes.reserve_slot();
+    let left_node_index = left_node_slot.stored_to_index();
 
-    let split_key = *children.new_end_iter().iter_prev(children).iter_borrow_key(children);
+    // right nodes next is the node that was next of the left (previous) node, and next of left node is the right node.
+    right_node.next = *left_next;
+    *left_next = right_node_index;
 
-    // aptos_std::debug::print(&std::string::utf8(b"creating right node"));
-    // aptos_std::debug::print(&right_node_slot);
-    // aptos_std::debug::print(&right_node);
+    // right nodes previous is current left node
+    right_node.prev = left_node_index;
+    // Since the previuosly used index is going to the right node, `prev` pointer of the next node is correct,
+    // and we need to update next pointer of the previous node (if exists)
+    if (*left_prev != NULL_INDEX) {
+        self.nodes.borrow_mut(*left_prev).next = left_node_index;
+    };
+    // Otherwise, we were the smallest node on the level. if this is the leaf level, update the pointer.
+    if (right_node_index == self.min_leaf_index) {
+        self.min_leaf_index = left_node_index;
+    };
 
-    // aptos_std::debug::print(&std::string::utf8(b"updating left node"));
-    // aptos_std::debug::print(&left_node_slot);
-    // aptos_std::debug::print(&node);
+    // Largest left key is the split key.
+    let max_left_key = *left_children.new_end_iter().iter_prev(left_children).iter_borrow_key(left_children);
 
-    self.nodes.fill_reserved_slot(left_node_slot, node);
-    self.nodes.fill_reserved_slot(right_node_slot, right_node);
+    self.nodes.fill_reserved_slot(left_node_reserved_slot, left_node);
+    self.nodes.fill_reserved_slot(right_node_reserved_slot, right_node);
 
-    if (right_node_ref == self.min_leaf_index) {
-        self.min_leaf_index = left_node_ref;
-    };
-    self.add_at(path_to_node, split_key, new_inner_child(left_node_stored_slot), false).destroy_none();
+    // Add new Child (i.e. pointer to the left node) in the parent.
+    self.add_at(path_to_node, max_left_key, new_inner_child(left_node_slot), false).destroy_none();
     option::none()
 }
 
@@ -1664,9 +1797,10 @@ Requires the map is not changed after the input iterator is generated. ## Function `update_key` +Given a path to node (excluding the node itself), which is currently stored under "old_key", update "old_key" to "new_key". -
fun update_key<K: copy, drop, store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, path_to_node: vector<storage_slots_allocator::RefToSlot>, old_key: &K, new_key: K)
+
fun update_key<K: copy, drop, store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, path_to_node: vector<u64>, old_key: &K, new_key: K)
 
@@ -1675,19 +1809,18 @@ Requires the map is not changed after the input iterator is generated. Implementation -
fun update_key<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, path_to_node: vector<RefToSlot>, old_key: &K, new_key: K) {
-    if (path_to_node.is_empty()) {
-        return
-    };
-
-    let node_index = path_to_node.pop_back();
-    let node = self.borrow_node_mut(node_index);
-    let children = &mut node.children;
-    children.replace_key_inplace(old_key, new_key);
+
fun update_key<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, path_to_node: vector<u64>, old_key: &K, new_key: K) {
+    while (!path_to_node.is_empty()) {
+        let node_index = path_to_node.pop_back();
+        let node = self.borrow_node_mut(node_index);
+        let children = &mut node.children;
+        children.replace_key_inplace(old_key, new_key);
 
-    if (children.new_end_iter().iter_prev(children).iter_borrow_key(children) == &new_key) {
-        self.update_key(path_to_node, old_key, new_key);
-    };
+        // If we were not updating the largest child, we don't need to continue.
+        if (children.new_end_iter().iter_prev(children).iter_borrow_key(children) != &new_key) {
+            return
+        };
+    }
 }
 
@@ -1701,7 +1834,7 @@ Requires the map is not changed after the input iterator is generated. -
fun remove_at<K: copy, drop, store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, path_to_node: vector<storage_slots_allocator::RefToSlot>, key: &K): big_ordered_map::Child<V>
+
fun remove_at<K: copy, drop, store, V: store>(self: &mut big_ordered_map::BigOrderedMap<K, V>, path_to_node: vector<u64>, key: &K): big_ordered_map::Child<V>
 
@@ -1710,37 +1843,42 @@ Requires the map is not changed after the input iterator is generated. Implementation -
fun remove_at<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, path_to_node: vector<RefToSlot>, key: &K): Child<V> {
+
fun remove_at<K: drop + copy + store, V: store>(self: &mut BigOrderedMap<K, V>, path_to_node: vector<u64>, key: &K): Child<V> {
+    // Last node in the path is one where we need to add the child to.
     let node_index = path_to_node.pop_back();
     let old_child = {
+        // First check if we can perform this operation, without changing structure of the tree (i.e. without adding any nodes).
+
+        // For that we can just borrow the single node
         let node = self.borrow_node_mut(node_index);
 
         let children = &mut node.children;
-
         let is_leaf = node.is_leaf;
 
         let old_child = children.remove(key);
-        if (path_to_node.is_empty()) {
-            assert!(node_index == self.root_index, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
+        if (node_index == ROOT_INDEX) {
+            // If current node is root, lower limit of max_degree/2 nodes doesn't apply.
+            // So we can adjust internally
+
+            assert!(path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
 
             if (!is_leaf && children.length() == 1) {
-                // promote only child to root, and drop current root.
-                // keep the root index the same.
+                // If root is not leaf, but has a single child, promote only child to root,
+                // and drop current root. Since root is stored directly in the resource, we
+                // "move" the child into the root.
+
                 let Child::Inner {
                     node_index: inner_child_index,
                 } = children.new_end_iter().iter_prev(children).iter_remove(children);
-                move children;
-                move node;
 
                 let inner_child = self.nodes.remove(inner_child_index);
                 if (inner_child.is_leaf) {
-                    let root_ref = self.root_index;
-                    self.min_leaf_index = root_ref;
-                    self.max_leaf_index = root_ref;
+                    self.min_leaf_index = ROOT_INDEX;
+                    self.max_leaf_index = ROOT_INDEX;
                 };
 
                 mem::replace(&mut self.root, inner_child).destroy_empty_node();
-            }; // else: nothing to change
+            };
             return old_child;
         };
 
@@ -1749,31 +1887,33 @@ Requires the map is not changed after the input iterator is generated.
         } else {
             self.inner_max_degree as u64
         };
-
         let current_size = children.length();
+
+        // See if the node is big enough, or we need to merge it with another node on this level.
         let big_enough = current_size * 2 >= max_degree;
 
         let new_max_key = *children.new_end_iter().iter_prev(children).iter_borrow_key(children);
-        let max_key_updated = cmp::compare(&new_max_key, key).is_less_than();
-        if (!max_key_updated && big_enough) {
-            return old_child;
-        };
 
+        // See if max key was updated for the current node, and if so - update it on the path.
+        let max_key_updated = cmp::compare(&new_max_key, key).is_lt();
         if (max_key_updated) {
             assert!(current_size >= 1, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
 
             self.update_key(path_to_node, key, new_max_key);
+        };
 
-            if (big_enough) {
-                return old_child;
-            }
+        // If node is big enough after removal, we are done.
+        if (big_enough) {
+            return old_child;
         };
 
         old_child
     };
 
-    // Children size is below threshold, we need to rebalance
+    // Children size is below threshold, we need to rebalance with a neighbor on the same level.
 
+    // In order to work on multiple nodes at the same time, we cannot borrow_mut, and need to be
+    // remove_and_reserve existing node.
     let (node_slot, node) = self.nodes.remove_and_reserve(node_index);
 
     let is_leaf = node.is_leaf;
@@ -1781,9 +1921,12 @@ Requires the map is not changed after the input iterator is generated.
     let prev = node.prev;
     let next = node.next;
 
-    let brother_index = {
-        let parent_children = &self.nodes.borrow(*path_to_node.borrow(path_to_node.length() - 1)).children;
-        if (parent_children.new_end_iter().iter_prev(parent_children).iter_borrow(parent_children).node_index.stored_as_ref() == node_index) {
+    // index of the node we will rebalance with.
+    let sibling_index = {
+        let parent_children = &self.borrow_node(*path_to_node.borrow(path_to_node.length() - 1)).children;
+        assert!(parent_children.length() >= 2, error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
+        // We merge with previous node - if it has the same parent, otherwise with next node (which then needs to have the same parent)
+        if (parent_children.new_end_iter().iter_prev(parent_children).iter_borrow(parent_children).node_index.stored_to_index() == node_index) {
             prev
         } else {
             next
@@ -1791,84 +1934,96 @@ Requires the map is not changed after the input iterator is generated.
     };
 
     let children = &mut node.children;
-    let (brother_slot, brother_node) = self.nodes.remove_and_reserve(brother_index);
 
-    let brother_children = &mut brother_node.children;
+    let (sibling_slot, sibling_node) = self.nodes.remove_and_reserve(sibling_index);
+    let sibling_children = &mut sibling_node.children;
 
-    if ((brother_children.length() - 1) * 2 >= max_degree) {
-        // The brother node has enough elements, borrow an element from the brother node.
-        if (brother_index == next) {
+    if ((sibling_children.length() - 1) * 2 >= max_degree) {
+        // The sibling node has enough elements, we can just borrow an element from the sibling node.
+        if (sibling_index == next) {
+            // if sibling is a larger node, we remove a child from the start
             let old_max_key = *children.new_end_iter().iter_prev(children).iter_borrow_key(children);
-            let brother_begin_iter = brother_children.new_begin_iter();
-            let borrowed_max_key = *brother_begin_iter.iter_borrow_key(brother_children);
-            let borrowed_element = brother_begin_iter.iter_remove(brother_children);
+            let sibling_begin_iter = sibling_children.new_begin_iter();
+            let borrowed_max_key = *sibling_begin_iter.iter_borrow_key(sibling_children);
+            let borrowed_element = sibling_begin_iter.iter_remove(sibling_children);
 
             children.add(borrowed_max_key, borrowed_element);
+
+            // max_key of the current node changed, so update
             self.update_key(path_to_node, &old_max_key, borrowed_max_key);
         } else {
-            let brother_end_iter = brother_children.new_end_iter().iter_prev(brother_children);
-            let borrowed_max_key = *brother_end_iter.iter_borrow_key(brother_children);
-            let borrowed_element = brother_end_iter.iter_remove(brother_children);
+            // if sibling is a smaller node, we remove a child from the end
+            let sibling_end_iter = sibling_children.new_end_iter().iter_prev(sibling_children);
+            let borrowed_max_key = *sibling_end_iter.iter_borrow_key(sibling_children);
+            let borrowed_element = sibling_end_iter.iter_remove(sibling_children);
 
             children.add(borrowed_max_key, borrowed_element);
-            self.update_key(path_to_node, &borrowed_max_key, *brother_children.new_end_iter().iter_prev(brother_children).iter_borrow_key(brother_children));
+
+            // max_key of the sibling node changed, so update
+            self.update_key(path_to_node, &borrowed_max_key, *sibling_children.new_end_iter().iter_prev(sibling_children).iter_borrow_key(sibling_children));
         };
 
         self.nodes.fill_reserved_slot(node_slot, node);
-        self.nodes.fill_reserved_slot(brother_slot, brother_node);
+        self.nodes.fill_reserved_slot(sibling_slot, sibling_node);
         return old_child;
     };
 
-    // The brother node doesn't have enough elements to borrow, merge with the brother node.
-    if (brother_index == next) {
-        let Node { children: brother_children, is_leaf: _, prev: _, next: brother_next } = brother_node;
+    // The sibling node doesn't have enough elements to borrow, merge with the sibling node.
+    // Keep the slot of the larger node of the two, to not require updating key on the parent nodes.
+    // But append to the smaller node, as ordered_map::append is more efficient when adding to the end.
+    let (key_to_remove, reserved_slot_to_remove) = if (sibling_index == next) {
+        // destroying larger sibling node, keeping sibling_slot.
+        let Node { children: sibling_children, is_leaf: _, prev: _, next: sibling_next } = sibling_node;
         let key_to_remove = *children.new_end_iter().iter_prev(children).iter_borrow_key(children);
-        children.append(brother_children);
-        node.next = brother_next;
+        children.append(sibling_children);
+        node.next = sibling_next;
 
-        move children;
-
-        if (!node.next.ref_is_null()) {
-            self.nodes.borrow_mut(node.next).prev = brother_index;
-        };
-        if (!node.prev.ref_is_null()) {
-            self.nodes.borrow_mut(node.prev).next = brother_index;
+        if (node.next != NULL_INDEX) {
+            assert!(self.nodes.borrow_mut(node.next).prev == sibling_index, 1);
         };
 
-        self.nodes.fill_reserved_slot(brother_slot, node);
-
+        // we are removing node_index, which previous's node's next was pointing to,
+        // so update the pointer
+        if (node.prev != NULL_INDEX) {
+            self.nodes.borrow_mut(node.prev).next = sibling_index;
+        };
+        // Otherwise, we were the smallest node on the level. if this is the leaf level, update the pointer.
         if (self.min_leaf_index == node_index) {
-            self.min_leaf_index = brother_index;
+            self.min_leaf_index = sibling_index;
         };
 
-        assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
-        let node_stored_slot = destroy_inner_child(self.remove_at(path_to_node, &key_to_remove));
-        self.nodes.free_reserved_slot(node_slot, node_stored_slot);
+        self.nodes.fill_reserved_slot(sibling_slot, node);
+
+        (key_to_remove, node_slot)
     } else {
+        // destroying larger current node, keeping node_slot
         let Node { children: node_children, is_leaf: _, prev: _, next: node_next } = node;
-        let key_to_remove = *brother_children.new_end_iter().iter_prev(brother_children).iter_borrow_key(brother_children);
-        brother_children.append(node_children);
-        brother_node.next = node_next;
-
-        move brother_children;
+        let key_to_remove = *sibling_children.new_end_iter().iter_prev(sibling_children).iter_borrow_key(sibling_children);
+        sibling_children.append(node_children);
+        sibling_node.next = node_next;
 
-        if (!brother_node.next.ref_is_null()) {
-            self.nodes.borrow_mut(brother_node.next).prev = node_index;
+        if (sibling_node.next != NULL_INDEX) {
+            assert!(self.nodes.borrow_mut(sibling_node.next).prev == node_index, 1);
         };
-        if (!brother_node.prev.ref_is_null()) {
-            self.nodes.borrow_mut(brother_node.prev).next = node_index;
+        // we are removing sibling node_index, which previous's node's next was pointing to,
+        // so update the pointer
+        if (sibling_node.prev != NULL_INDEX) {
+            self.nodes.borrow_mut(sibling_node.prev).next = node_index;
         };
-
-        self.nodes.fill_reserved_slot(node_slot, brother_node);
-
-        if (self.min_leaf_index == brother_index) {
+        // Otherwise, sibling was the smallest node on the level. if this is the leaf level, update the pointer.
+        if (self.min_leaf_index == sibling_index) {
             self.min_leaf_index = node_index;
         };
 
-        assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
-        let node_stored_slot = destroy_inner_child(self.remove_at(path_to_node, &key_to_remove));
-        self.nodes.free_reserved_slot(brother_slot, node_stored_slot);
+        self.nodes.fill_reserved_slot(node_slot, sibling_node);
+
+        (key_to_remove, sibling_slot)
     };
+
+    assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN));
+    let slot_to_remove = destroy_inner_child(self.remove_at(path_to_node, &key_to_remove));
+    self.nodes.free_reserved_slot(reserved_slot_to_remove, slot_to_remove);
+
     old_child
 }
 
@@ -1894,7 +2049,7 @@ Returns the number of elements in the BigOrderedMap.
fun length<K: store, V: store>(self: &BigOrderedMap<K, V>): u64 {
-    self.length_for_node(self.root_index)
+    self.length_for_node(ROOT_INDEX)
 }
 
@@ -1908,7 +2063,7 @@ Returns the number of elements in the BigOrderedMap. -
fun length_for_node<K: store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, node_index: storage_slots_allocator::RefToSlot): u64
+
fun length_for_node<K: store, V: store>(self: &big_ordered_map::BigOrderedMap<K, V>, node_index: u64): u64
 
@@ -1917,7 +2072,7 @@ Returns the number of elements in the BigOrderedMap. Implementation -
fun length_for_node<K: store, V: store>(self: &BigOrderedMap<K, V>, node_index: RefToSlot): u64 {
+
fun length_for_node<K: store, V: store>(self: &BigOrderedMap<K, V>, node_index: u64): u64 {
     let node = self.borrow_node(node_index);
     if (node.is_leaf) {
         node.children.length()
@@ -1925,7 +2080,7 @@ Returns the number of elements in the BigOrderedMap.
         let size = 0;
 
         node.children.for_each_ref(|_key, child| {
-            size = size + self.length_for_node(child.node_index.stored_as_ref());
+            size = size + self.length_for_node(child.node_index.stored_to_index());
         });
         size
     }
diff --git a/aptos-move/framework/aptos-stdlib/doc/storage_slots_allocator.md b/aptos-move/framework/aptos-stdlib/doc/storage_slots_allocator.md
index d14846eac6b273..a604a9090ae2b9 100644
--- a/aptos-move/framework/aptos-stdlib/doc/storage_slots_allocator.md
+++ b/aptos-move/framework/aptos-stdlib/doc/storage_slots_allocator.md
@@ -18,6 +18,7 @@ later (i.e. if we need address to initialize the value itself).
 
 In the future, more sophisticated strategies can be added, without breaking/modifying callers,
 for example:
+* inlining some nodes
 * having a fee-payer for any storage creation operations
 
 
@@ -26,25 +27,23 @@ for example:
 -  [Enum `StorageSlotsAllocator`](#0x1_storage_slots_allocator_StorageSlotsAllocator)
 -  [Struct `ReservedSlot`](#0x1_storage_slots_allocator_ReservedSlot)
 -  [Struct `StoredSlot`](#0x1_storage_slots_allocator_StoredSlot)
--  [Struct `RefToSlot`](#0x1_storage_slots_allocator_RefToSlot)
 -  [Constants](#@Constants_0)
 -  [Function `new`](#0x1_storage_slots_allocator_new)
 -  [Function `new_default_config`](#0x1_storage_slots_allocator_new_default_config)
 -  [Function `new_config`](#0x1_storage_slots_allocator_new_config)
 -  [Function `add`](#0x1_storage_slots_allocator_add)
 -  [Function `remove`](#0x1_storage_slots_allocator_remove)
--  [Function `destroy`](#0x1_storage_slots_allocator_destroy)
+-  [Function `destroy_known_empty_unsafe`](#0x1_storage_slots_allocator_destroy_known_empty_unsafe)
 -  [Function `borrow`](#0x1_storage_slots_allocator_borrow)
 -  [Function `borrow_mut`](#0x1_storage_slots_allocator_borrow_mut)
 -  [Function `reserve_slot`](#0x1_storage_slots_allocator_reserve_slot)
 -  [Function `fill_reserved_slot`](#0x1_storage_slots_allocator_fill_reserved_slot)
 -  [Function `remove_and_reserve`](#0x1_storage_slots_allocator_remove_and_reserve)
 -  [Function `free_reserved_slot`](#0x1_storage_slots_allocator_free_reserved_slot)
--  [Function `reserved_as_ref`](#0x1_storage_slots_allocator_reserved_as_ref)
--  [Function `stored_as_ref`](#0x1_storage_slots_allocator_stored_as_ref)
--  [Function `null_ref`](#0x1_storage_slots_allocator_null_ref)
--  [Function `special_ref`](#0x1_storage_slots_allocator_special_ref)
--  [Function `ref_is_null`](#0x1_storage_slots_allocator_ref_is_null)
+-  [Function `reserved_to_index`](#0x1_storage_slots_allocator_reserved_to_index)
+-  [Function `stored_to_index`](#0x1_storage_slots_allocator_stored_to_index)
+-  [Function `is_null_index`](#0x1_storage_slots_allocator_is_null_index)
+-  [Function `is_special_unused_index`](#0x1_storage_slots_allocator_is_special_unused_index)
 -  [Function `maybe_pop_from_reuse_queue`](#0x1_storage_slots_allocator_maybe_pop_from_reuse_queue)
 -  [Function `maybe_push_to_reuse_queue`](#0x1_storage_slots_allocator_maybe_push_to_reuse_queue)
 -  [Function `next_slot_index`](#0x1_storage_slots_allocator_next_slot_index)
@@ -272,36 +271,6 @@ and there is unique owner for each slot.
 
 
 
-
-Fields - - -
-
-slot_index: u64 -
-
- -
-
- - -
- - - -## Struct `RefToSlot` - -(Weak) Reference to a slot. -We can have variety of RefToSlot, but only a single StoredSlot. -It is on the caller to make sure references are not used after slot is freed. - - -
struct RefToSlot has copy, drop, store
-
- - -
Fields @@ -359,15 +328,6 @@ It is on the caller to make sure references are not used after slot is freed. - - - - -
const SPECIAL_SLOT_INDEX: u64 = 1;
-
- - - ## Function `new` @@ -501,7 +461,7 @@ It is on the caller to make sure references are not used after slot is freed.
public fun remove<T: store>(self: &mut StorageSlotsAllocator<T>, slot: StoredSlot): T {
-    let (reserved_slot, value) = self.remove_and_reserve(slot.stored_as_ref());
+    let (reserved_slot, value) = self.remove_and_reserve(slot.stored_to_index());
     self.free_reserved_slot(reserved_slot, slot);
     value
 }
@@ -511,13 +471,15 @@ It is on the caller to make sure references are not used after slot is freed.
 
 
- + -## Function `destroy` +## Function `destroy_known_empty_unsafe` +We cannot know if allocator is empty or not, so this method is not public, +and can be used only in modules that know by themselves that allocator is empty. -
public(friend) fun destroy<T: store>(self: storage_slots_allocator::StorageSlotsAllocator<T>)
+
public(friend) fun destroy_known_empty_unsafe<T: store>(self: storage_slots_allocator::StorageSlotsAllocator<T>)
 
@@ -526,7 +488,7 @@ It is on the caller to make sure references are not used after slot is freed. Implementation -
public(friend) fun destroy<T: store>(self: StorageSlotsAllocator<T>) {
+
public(friend) fun destroy_known_empty_unsafe<T: store>(self: StorageSlotsAllocator<T>) {
     loop {
         let reuse_index = self.maybe_pop_from_reuse_queue();
         if (reuse_index == NULL_INDEX) {
@@ -542,7 +504,7 @@ It is on the caller to make sure references are not used after slot is freed.
             reuse_spare_count: _,
         } => {
             assert!(reuse_head_index == NULL_INDEX, EINTERNAL_INVARIANT_BROKEN);
-            slots.destroy_some().destroy();
+            slots.destroy_some().destroy_known_empty_unsafe();
         },
     };
 }
@@ -558,7 +520,7 @@ It is on the caller to make sure references are not used after slot is freed.
 
 
 
-
public fun borrow<T: store>(self: &storage_slots_allocator::StorageSlotsAllocator<T>, slot: storage_slots_allocator::RefToSlot): &T
+
public fun borrow<T: store>(self: &storage_slots_allocator::StorageSlotsAllocator<T>, slot_index: u64): &T
 
@@ -567,8 +529,8 @@ It is on the caller to make sure references are not used after slot is freed. Implementation -
public fun borrow<T: store>(self: &StorageSlotsAllocator<T>, slot: RefToSlot): &T {
-    &self.slots.borrow().borrow(slot.slot_index).value
+
public fun borrow<T: store>(self: &StorageSlotsAllocator<T>, slot_index: u64): &T {
+    &self.slots.borrow().borrow(slot_index).value
 }
 
@@ -582,7 +544,7 @@ It is on the caller to make sure references are not used after slot is freed. -
public fun borrow_mut<T: store>(self: &mut storage_slots_allocator::StorageSlotsAllocator<T>, slot: storage_slots_allocator::RefToSlot): &mut T
+
public fun borrow_mut<T: store>(self: &mut storage_slots_allocator::StorageSlotsAllocator<T>, slot_index: u64): &mut T
 
@@ -591,8 +553,8 @@ It is on the caller to make sure references are not used after slot is freed. Implementation -
public fun borrow_mut<T: store>(self: &mut StorageSlotsAllocator<T>, slot: RefToSlot): &mut T {
-    &mut self.slots.borrow_mut().borrow_mut(slot.slot_index).value
+
public fun borrow_mut<T: store>(self: &mut StorageSlotsAllocator<T>, slot_index: u64): &mut T {
+    &mut self.slots.borrow_mut().borrow_mut(slot_index).value
 }
 
@@ -664,7 +626,7 @@ It is on the caller to make sure references are not used after slot is freed. Remove storage slot, but reserve it for later. -
public fun remove_and_reserve<T: store>(self: &mut storage_slots_allocator::StorageSlotsAllocator<T>, slot: storage_slots_allocator::RefToSlot): (storage_slots_allocator::ReservedSlot, T)
+
public fun remove_and_reserve<T: store>(self: &mut storage_slots_allocator::StorageSlotsAllocator<T>, slot_index: u64): (storage_slots_allocator::ReservedSlot, T)
 
@@ -673,8 +635,7 @@ Remove storage slot, but reserve it for later. Implementation -
public fun remove_and_reserve<T: store>(self: &mut StorageSlotsAllocator<T>, slot: RefToSlot): (ReservedSlot, T) {
-    let slot_index = slot.slot_index;
+
public fun remove_and_reserve<T: store>(self: &mut StorageSlotsAllocator<T>, slot_index: u64): (ReservedSlot, T) {
     let Link::Occupied { value } = self.remove_link(slot_index);
     (ReservedSlot { slot_index }, value)
 }
@@ -711,37 +672,13 @@ Remove storage slot, but reserve it for later.
 
 
 
-
-
-## Function `reserved_as_ref`
-
-
-
-
public fun reserved_as_ref(self: &storage_slots_allocator::ReservedSlot): storage_slots_allocator::RefToSlot
-
- - - -
-Implementation - - -
public fun reserved_as_ref(self: &ReservedSlot): RefToSlot {
-    RefToSlot { slot_index: self.slot_index }
-}
-
- - - -
- - + -## Function `stored_as_ref` +## Function `reserved_to_index` -
public fun stored_as_ref(self: &storage_slots_allocator::StoredSlot): storage_slots_allocator::RefToSlot
+
public fun reserved_to_index(self: &storage_slots_allocator::ReservedSlot): u64
 
@@ -750,8 +687,8 @@ Remove storage slot, but reserve it for later. Implementation -
public fun stored_as_ref(self: &StoredSlot): RefToSlot {
-    RefToSlot { slot_index: self.slot_index }
+
public fun reserved_to_index(self: &ReservedSlot): u64 {
+    self.slot_index
 }
 
@@ -759,13 +696,13 @@ Remove storage slot, but reserve it for later. - + -## Function `null_ref` +## Function `stored_to_index` -
public fun null_ref(): storage_slots_allocator::RefToSlot
+
public fun stored_to_index(self: &storage_slots_allocator::StoredSlot): u64
 
@@ -774,8 +711,8 @@ Remove storage slot, but reserve it for later. Implementation -
public fun null_ref(): RefToSlot {
-    RefToSlot { slot_index: NULL_INDEX }
+
public fun stored_to_index(self: &StoredSlot): u64 {
+    self.slot_index
 }
 
@@ -783,13 +720,13 @@ Remove storage slot, but reserve it for later. - + -## Function `special_ref` +## Function `is_null_index` -
public fun special_ref(): storage_slots_allocator::RefToSlot
+
public fun is_null_index(slot_index: u64): bool
 
@@ -798,8 +735,8 @@ Remove storage slot, but reserve it for later. Implementation -
public fun special_ref(): RefToSlot {
-    RefToSlot { slot_index: SPECIAL_SLOT_INDEX }
+
public fun is_null_index(slot_index: u64): bool {
+    slot_index == NULL_INDEX
 }
 
@@ -807,13 +744,13 @@ Remove storage slot, but reserve it for later. - + -## Function `ref_is_null` +## Function `is_special_unused_index` -
public fun ref_is_null(self: &storage_slots_allocator::RefToSlot): bool
+
public fun is_special_unused_index(slot_index: u64): bool
 
@@ -822,8 +759,8 @@ Remove storage slot, but reserve it for later. Implementation -
public fun ref_is_null(self: &RefToSlot): bool {
-    self.slot_index == NULL_INDEX
+
public fun is_special_unused_index(slot_index: u64): bool {
+    slot_index != NULL_INDEX && slot_index < FIRST_INDEX
 }
 
diff --git a/aptos-move/framework/aptos-stdlib/doc/table.md b/aptos-move/framework/aptos-stdlib/doc/table.md index 129c1c38fd7943..95b4ee8a8c07a1 100644 --- a/aptos-move/framework/aptos-stdlib/doc/table.md +++ b/aptos-move/framework/aptos-stdlib/doc/table.md @@ -22,7 +22,7 @@ struct itself, while the operations are implemented as native functions. No trav - [Function `upsert`](#0x1_table_upsert) - [Function `remove`](#0x1_table_remove) - [Function `contains`](#0x1_table_contains) -- [Function `destroy`](#0x1_table_destroy) +- [Function `destroy_known_empty_unsafe`](#0x1_table_destroy_known_empty_unsafe) - [Function `new_table_handle`](#0x1_table_new_table_handle) - [Function `add_box`](#0x1_table_add_box) - [Function `borrow_box`](#0x1_table_borrow_box) @@ -41,7 +41,7 @@ struct itself, while the operations are implemented as native functions. No trav - [Function `upsert`](#@Specification_0_upsert) - [Function `remove`](#@Specification_0_remove) - [Function `contains`](#@Specification_0_contains) - - [Function `destroy`](#@Specification_0_destroy) + - [Function `destroy_known_empty_unsafe`](#@Specification_0_destroy_known_empty_unsafe)
@@ -352,15 +352,15 @@ Returns true iff self contains an entry for key. - + -## Function `destroy` +## Function `destroy_known_empty_unsafe` Table cannot know if it is empty or not, so this method is not public, and can be used only in modules that know by themselves that table is empty. -
public(friend) fun destroy<K: copy, drop, V>(self: table::Table<K, V>)
+
public(friend) fun destroy_known_empty_unsafe<K: copy, drop, V>(self: table::Table<K, V>)
 
@@ -369,7 +369,7 @@ and can be used only in modules that know by themselves that table is empty. Implementation -
public(friend) fun destroy<K: copy + drop, V>(self: Table<K, V>) {
+
public(friend) fun destroy_known_empty_unsafe<K: copy + drop, V>(self: Table<K, V>) {
     destroy_empty_box<K, V, Box<V>>(&self);
     drop_unchecked_box<K, V, Box<V>>(self)
 }
@@ -583,7 +583,7 @@ and can be used only in modules that know by themselves that table is empty.
 
 
pragma intrinsic = map,
     map_new = new,
-    map_destroy_empty = destroy,
+    map_destroy_empty = destroy_known_empty_unsafe,
     map_has_key = contains,
     map_add_no_override = add,
     map_add_override_if_exists = upsert,
@@ -763,12 +763,12 @@ and can be used only in modules that know by themselves that table is empty.
 
 
 
-
+
 
-### Function `destroy`
+### Function `destroy_known_empty_unsafe`
 
 
-
public(friend) fun destroy<K: copy, drop, V>(self: table::Table<K, V>)
+
public(friend) fun destroy_known_empty_unsafe<K: copy, drop, V>(self: table::Table<K, V>)
 
diff --git a/aptos-move/framework/aptos-stdlib/doc/table_with_length.md b/aptos-move/framework/aptos-stdlib/doc/table_with_length.md index 9bf3b27091eba9..809638f07c29de 100644 --- a/aptos-move/framework/aptos-stdlib/doc/table_with_length.md +++ b/aptos-move/framework/aptos-stdlib/doc/table_with_length.md @@ -153,7 +153,7 @@ Destroy a table. The table must be empty to succeed.
public fun destroy_empty<K: copy + drop, V>(self: TableWithLength<K, V>) {
     assert!(self.length == 0, error::invalid_state(ENOT_EMPTY));
     let TableWithLength { inner, length: _ } = self;
-    table::destroy(inner)
+    table::destroy_known_empty_unsafe(inner)
 }
 
diff --git a/aptos-move/framework/aptos-stdlib/sources/data_structures/big_ordered_map.move b/aptos-move/framework/aptos-stdlib/sources/data_structures/big_ordered_map.move index 1680ac5fcc03ea..8daab94747b72b 100644 --- a/aptos-move/framework/aptos-stdlib/sources/data_structures/big_ordered_map.move +++ b/aptos-move/framework/aptos-stdlib/sources/data_structures/big_ordered_map.move @@ -22,24 +22,27 @@ module aptos_std::big_ordered_map { use std::mem; use aptos_std::ordered_map::{Self, OrderedMap}; use aptos_std::cmp; - use aptos_std::storage_slots_allocator::{Self, StorageSlotsAllocator, StoredSlot, RefToSlot}; + use aptos_std::storage_slots_allocator::{Self, StorageSlotsAllocator, StoredSlot}; use aptos_std::math64::{max, min}; /// Map key already exists const EKEY_ALREADY_EXISTS: u64 = 1; /// Map key is not found const EKEY_NOT_FOUND: u64 = 2; - // Trying to do an operation on an Iterator that would go out of bounds + /// Trying to do an operation on an Iterator that would go out of bounds const EITER_OUT_OF_BOUNDS: u64 = 3; - // The provided configuration parameter is invalid. + /// The provided configuration parameter is invalid. const EINVALID_CONFIG_PARAMETER: u64 = 4; - // Map isn't empty + /// Map isn't empty const EMAP_NOT_EMPTY: u64 = 5; - // Trying to insert too large of an object into the mp. + /// Trying to insert too large of an object into the mp. const EARGUMENT_BYTES_TOO_LARGE: u64 = 6; + /// borrow_mut requires that key and value types have constant size + /// (otherwise it wouldn't be able to guarantee size requirements are not violated) + const EBORROW_MUT_REQUIRES_CONSTANT_KV_SIZE: u64 = 7; - // Internal errors. - const EINTERNAL_INVARIANT_BROKEN: u64 = 7; + /// Internal errors. + const EINTERNAL_INVARIANT_BROKEN: u64 = 20; // Internal constants. @@ -52,6 +55,9 @@ module aptos_std::big_ordered_map { const MAX_NODE_BYTES: u64 = 204800; // 200 KB, well bellow the max resource limit. + const NULL_INDEX: u64 = 0; + const ROOT_INDEX: u64 = 1; + /// A node of the BigOrderedMap. /// /// Inner node will have all children be Child::Inner, pointing to the child nodes. @@ -65,10 +71,10 @@ module aptos_std::big_ordered_map { // When node is inner node, K represents max_key within the child subtree, and values are Child::Inner. // When the node is leaf node, K represents key of the leaf, and values are Child::Leaf. children: OrderedMap>, - // The node index of its previous node at the same level, or `null_ref()` if it doesn't have a previous node. - prev: RefToSlot, - // The node index of its next node at the same level, or `null_ref()` if it doesn't have a next node. - next: RefToSlot, + // The node index of its previous node at the same level, or `NULL_INDEX` if it doesn't have a previous node. + prev: u64, + // The node index of its next node at the same level, or `NULL_INDEX` if it doesn't have a next node. + next: u64, } /// The metadata of a child of a node. @@ -88,7 +94,7 @@ module aptos_std::big_ordered_map { End, Some { /// The node index of the iterator pointing to. - node_index: RefToSlot, + node_index: u64, /// Child iter it is pointing to child_iter: ordered_map::Iterator, @@ -102,15 +108,14 @@ module aptos_std::big_ordered_map { /// The BigOrderedMap data structure. enum BigOrderedMap has store { BPlusTreeMap { + /// Root node. It is stored directly in the resource itself, unlike all other nodes. root: Node, - /// The node index of the root node. - root_index: RefToSlot, - /// Mapping of node_index -> node. + /// Storage of all non-root nodes. They are stored in separate storage slots. nodes: StorageSlotsAllocator>, /// The node index of the leftmost node. - min_leaf_index: RefToSlot, + min_leaf_index: u64, /// The node index of the rightmost node. - max_leaf_index: RefToSlot, + max_leaf_index: u64, /// Whether Key and Value have constant serialized size, and if so /// optimize out size checks on every insert, if so. @@ -136,16 +141,18 @@ module aptos_std::big_ordered_map { assert!(leaf_max_degree == 0 || leaf_max_degree >= DEFAULT_LEAF_MIN_DEGREE, error::invalid_argument(EINVALID_CONFIG_PARAMETER)); assert!(reuse_slots || num_to_preallocate == 0, error::invalid_argument(EINVALID_CONFIG_PARAMETER)); + // Assert that storage_slots_allocator special indices are aligned: + assert!(storage_slots_allocator::is_null_index(NULL_INDEX), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + assert!(storage_slots_allocator::is_special_unused_index(ROOT_INDEX), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + let nodes = storage_slots_allocator::new(storage_slots_allocator::new_config(reuse_slots, num_to_preallocate)); - let root_ref = storage_slots_allocator::special_ref(); let self = BigOrderedMap::BPlusTreeMap { root: new_node(/*is_leaf=*/true), - root_index: root_ref, nodes: nodes, - min_leaf_index: root_ref, - max_leaf_index: root_ref, - constant_kv_size: false, + min_leaf_index: ROOT_INDEX, + max_leaf_index: ROOT_INDEX, + constant_kv_size: false, // Will be initialized in validate_static_size_and_init_max_degrees below. inner_max_degree: inner_max_degree, leaf_max_degree: leaf_max_degree }; @@ -155,9 +162,11 @@ module aptos_std::big_ordered_map { /// Destroys the map if it's empty, otherwise aborts. public fun destroy_empty(self: BigOrderedMap) { - let BigOrderedMap::BPlusTreeMap { root, nodes, root_index: _, min_leaf_index: _, max_leaf_index: _, constant_kv_size: _, inner_max_degree: _, leaf_max_degree: _ } = self; + let BigOrderedMap::BPlusTreeMap { root, nodes, min_leaf_index: _, max_leaf_index: _, constant_kv_size: _, inner_max_degree: _, leaf_max_degree: _ } = self; root.destroy_empty_node(); - nodes.destroy(); + // If root node is empty, then we know that no storage slots are used, + // and so we can safely destroy all nodes. + nodes.destroy_known_empty_unsafe(); } // ======================= Section with Modifiers ========================= @@ -186,6 +195,15 @@ module aptos_std::big_ordered_map { /// Removes the entry from BigOrderedMap and returns the value which `key` maps to. /// Aborts if there is no entry for `key`. public fun remove(self: &mut BigOrderedMap, key: &K): V { + // Optimize case where only root node exists + // (optimizes out borrowing and path creation in `find_leaf_with_path`) + if (self.root.is_leaf) { + let Child::Leaf { + value, + } = self.root.children.remove(key); + return value; + }; + let path_to_leaf = self.find_leaf_with_path(key); assert!(!path_to_leaf.is_empty(), error::invalid_argument(EKEY_NOT_FOUND)); @@ -193,7 +211,6 @@ module aptos_std::big_ordered_map { let Child::Leaf { value, } = self.remove_at(path_to_leaf, key); - value } @@ -203,7 +220,7 @@ module aptos_std::big_ordered_map { /// key, or an end iterator if such element doesn't exist. public fun lower_bound(self: &BigOrderedMap, key: &K): Iterator { let leaf = self.find_leaf(key); - if (leaf.ref_is_null()) { + if (leaf == NULL_INDEX) { return self.new_end_iter() }; @@ -253,15 +270,15 @@ module aptos_std::big_ordered_map { &iter.child_iter.iter_borrow(children).value } - // TODO: Add back for fixed sized structs only. - // /// Returns a mutable reference to the element with its key at the given index, aborts if the key is not found. - // public fun borrow_mut(self: &mut BigOrderedMap, key: K): &mut V { - // let iter = self.find(&key); - // - // assert!(iter.iter_is_end(self), error::invalid_argument(EKEY_NOT_FOUND)); - // let children = &mut self.nodes.borrow_mut(iter.node_index).children; - // &mut iter.child_iter.iter_borrow_mut(children).value - // } + /// Returns a mutable reference to the element with its key at the given index, aborts if the key is not found. + public fun borrow_mut(self: &mut BigOrderedMap, key: K): &mut V { + assert!(self.constant_kv_size, error::invalid_argument(EBORROW_MUT_REQUIRES_CONSTANT_KV_SIZE)); + let iter = self.find(&key); + + assert!(iter.iter_is_end(self), error::invalid_argument(EKEY_NOT_FOUND)); + let children = &mut self.borrow_node_mut(iter.node_index).children; + &mut iter.child_iter.iter_borrow_mut(children).value + } // ========================= Iterator functions =========================== @@ -313,12 +330,14 @@ module aptos_std::big_ordered_map { let child_iter = self.child_iter.iter_next(&node.children); if (!child_iter.iter_is_end(&node.children)) { + // next is in the same leaf node let iter_key = *child_iter.iter_borrow_key(&node.children); return new_iter(node_index, child_iter, iter_key); }; + // next is in a different leaf node let next_index = node.next; - if (!next_index.ref_is_null()) { + if (next_index != NULL_INDEX) { let next_node = map.borrow_node(next_index); let child_iter = next_node.children.new_begin_iter(); @@ -340,6 +359,7 @@ module aptos_std::big_ordered_map { let node = map.borrow_node(node_index); if (!self.child_iter.iter_is_begin(&node.children)) { + // next is in the same leaf node let child_iter = self.child_iter.iter_prev(&node.children); let key = *child_iter.iter_borrow_key(&node.children); return new_iter(node_index, child_iter, key); @@ -347,8 +367,9 @@ module aptos_std::big_ordered_map { node.prev }; - assert!(!prev_index.ref_is_null(), error::invalid_argument(EITER_OUT_OF_BOUNDS)); + assert!(prev_index != NULL_INDEX, error::invalid_argument(EITER_OUT_OF_BOUNDS)); + // next is in a different leaf node let prev_node = map.borrow_node(prev_index); let prev_children = &prev_node.children; @@ -359,19 +380,21 @@ module aptos_std::big_ordered_map { // ====================== Internal Implementations ======================== - inline fun borrow_node(self: &BigOrderedMap, node: RefToSlot): &Node { - if (self.root_index == node) { + /// Borrow a node, given an index. Works for both root (i.e. inline) node and separately stored nodes + inline fun borrow_node(self: &BigOrderedMap, node_index: u64): &Node { + if (node_index == ROOT_INDEX) { &self.root } else { - self.nodes.borrow(node) + self.nodes.borrow(node_index) } } - inline fun borrow_node_mut(self: &mut BigOrderedMap, node: RefToSlot): &mut Node { - if (self.root_index == node) { + /// Borrow a node mutably, given an index. Works for both root (i.e. inline) node and separately stored nodes + inline fun borrow_node_mut(self: &mut BigOrderedMap, node_index: u64): &mut Node { + if (node_index == ROOT_INDEX) { &mut self.root } else { - self.nodes.borrow_mut(node) + self.nodes.borrow_mut(node_index) } } @@ -380,12 +403,26 @@ module aptos_std::big_ordered_map { self.validate_dynamic_size_and_init_max_degrees(&key, &value); }; + // Optimize case where only root node exists + // (optimizes out borrowing and path creation in `find_leaf_with_path`) + if (self.root.is_leaf) { + let children = &mut self.root.children; + let current_size = children.length(); + + if (current_size < (self.leaf_max_degree as u64)) { + let result = children.upsert(key, new_leaf_child(value)); + assert!(allow_overwrite || result.is_none(), error::invalid_argument(EKEY_ALREADY_EXISTS)); + return result; + }; + }; + let path_to_leaf = self.find_leaf_with_path(&key); if (path_to_leaf.is_empty()) { // In this case, the key is greater than all keys in the map. - - let current = self.root_index; + // So we need to update `key` in the pointers to the last child, + // to maintain the invariant of `add_at` + let current = ROOT_INDEX; loop { path_to_leaf.push_back(current); @@ -395,13 +432,11 @@ module aptos_std::big_ordered_map { break; }; let last_value = current_node.children.new_end_iter().iter_prev(¤t_node.children).iter_remove(&mut current_node.children); - current = last_value.node_index.stored_as_ref(); + current = last_value.node_index.stored_to_index(); current_node.children.add(key, last_value); }; }; - // aptos_std::debug::print(&std::string::utf8(b"add_or_upsert_impl::path_to_leaf")); - // aptos_std::debug::print(&path_to_leaf); self.add_at(path_to_leaf, key, new_leaf_child(value), allow_overwrite) } @@ -432,6 +467,7 @@ module aptos_std::big_ordered_map { self.leaf_max_degree = max(min(MAX_DEGREE, DEFAULT_TARGET_NODE_SIZE / entry_size), DEFAULT_LEAF_MIN_DEGREE as u64) as u16; }; + // Make sure that no nodes can exceed the upper size limit. assert!(key_size * (self.inner_max_degree as u64) <= MAX_NODE_BYTES, error::invalid_argument(EARGUMENT_BYTES_TOO_LARGE)); assert!(entry_size * (self.leaf_max_degree as u64) <= MAX_NODE_BYTES, error::invalid_argument(EARGUMENT_BYTES_TOO_LARGE)); } @@ -454,8 +490,8 @@ module aptos_std::big_ordered_map { Node { is_leaf: is_leaf, children: ordered_map::new(), - prev: storage_slots_allocator::null_ref(), - next: storage_slots_allocator::null_ref(), + prev: NULL_INDEX, + next: NULL_INDEX, } } @@ -463,8 +499,8 @@ module aptos_std::big_ordered_map { Node { is_leaf: is_leaf, children: children, - prev: storage_slots_allocator::null_ref(), - next: storage_slots_allocator::null_ref(), + prev: NULL_INDEX, + next: NULL_INDEX, } } @@ -480,7 +516,7 @@ module aptos_std::big_ordered_map { } } - fun new_iter(node_index: RefToSlot, child_iter: ordered_map::Iterator, key: K): Iterator { + fun new_iter(node_index: u64, child_iter: ordered_map::Iterator, key: K): Iterator { Iterator::Some { node_index: node_index, child_iter: child_iter, @@ -488,9 +524,12 @@ module aptos_std::big_ordered_map { } } - fun find_leaf(self: &BigOrderedMap, key: &K): RefToSlot { - let current = self.root_index; - while (!current.ref_is_null()) { + /// Find leaf where the given key would fall in. + /// So the largest leaf with it's `max_key <= key`. + /// return NULL_INDEX if `key` is larger than any key currently stored in the map. + fun find_leaf(self: &BigOrderedMap, key: &K): u64 { + let current = ROOT_INDEX; + loop { let node = self.borrow_node(current); if (node.is_leaf) { return current; @@ -498,20 +537,22 @@ module aptos_std::big_ordered_map { let children = &node.children; let child_iter = children.lower_bound(key); if (child_iter.iter_is_end(children)) { - return storage_slots_allocator::null_ref(); + return NULL_INDEX; } else { - current = child_iter.iter_borrow(children).node_index.stored_as_ref(); - } - }; - - storage_slots_allocator::null_ref() + current = child_iter.iter_borrow(children).node_index.stored_to_index(); + }; + } } - fun find_leaf_with_path(self: &BigOrderedMap, key: &K): vector { + /// Find leaf where the given key would fall in. + /// So the largest leaf with it's `max_key <= key`. + /// Return the path from root to that leaf (including the leaf itself) + /// Return empty path if `key` is larger than any key currently stored in the map. + fun find_leaf_with_path(self: &BigOrderedMap, key: &K): vector { let vec = vector::empty(); - let current = self.root_index; - while (!current.ref_is_null()) { + let current = ROOT_INDEX; + loop { vec.push_back(current); let node = self.borrow_node(current); @@ -523,11 +564,9 @@ module aptos_std::big_ordered_map { if (child_iter.iter_is_end(children)) { return vector::empty(); } else { - current = child_iter.iter_borrow(children).node_index.stored_as_ref(); - } - }; - - abort error::invalid_state(EINTERNAL_INVARIANT_BROKEN) + current = child_iter.iter_borrow(children).node_index.stored_to_index(); + }; + } } fun get_max_degree(self: &BigOrderedMap, leaf: bool): u64 { @@ -538,9 +577,19 @@ module aptos_std::big_ordered_map { } } - fun add_at(self: &mut BigOrderedMap, path_to_node: vector, key: K, child: Child, allow_overwrite: bool): Option> { + /// Add a given child to a given node (last in the `path_to_node`), and update/rebalance the tree as necessary. + /// It is required that `key` pointers to the child node, on the `path_to_node` are greater or equal to the given key. + /// That means if we are adding a `key` larger than any currently existing in the map - we needed + /// to update `key` pointers on the `path_to_node` to include it, before calling this method. + /// + /// If `allow_overwrite` is not set, function will abort if `key` is already present. + fun add_at(self: &mut BigOrderedMap, path_to_node: vector, key: K, child: Child, allow_overwrite: bool): Option> { + // Last node in the path is one where we need to add the child to. let node_index = path_to_node.pop_back(); { + // First check if we can perform this operation, without changing structure of the tree (i.e. without adding any nodes). + + // For that we can just borrow the single node let node = self.borrow_node_mut(node_index); let children = &mut node.children; let current_size = children.length(); @@ -552,6 +601,7 @@ module aptos_std::big_ordered_map { }; if (current_size < max_degree) { + // Adding a child to a current node doesn't exceed the size, so we can just do that. let result = children.upsert(key, child); if (node.is_leaf) { @@ -563,63 +613,90 @@ module aptos_std::big_ordered_map { }; }; - if (allow_overwrite) { - let iter = children.find(&key); - if (!iter.iter_is_end(children)) { - return option::some(iter.iter_replace(children, child)); - } + // If we cannot add more nodes without exceeding the size, + // but node with `key` already exists, we either need to replace or abort. + let iter = children.find(&key); + if (!iter.iter_is_end(children)) { + assert!(node.is_leaf, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + assert!(allow_overwrite, error::invalid_argument(EKEY_ALREADY_EXISTS)); + + return option::some(iter.iter_replace(children, child)); } }; // # of children in the current node exceeds the threshold, need to split into two nodes. - let (right_node_slot, node) = if (path_to_node.is_empty()) { - // If we are at the root, we need to move root node to become a child and have a new root node. - - assert!(node_index == self.root_index, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); - // aptos_std::debug::print(&std::string::utf8(b"changing root")); + // If we are at the root, we need to move root node to become a child and have a new root node, + // in order to be able to split the node on the level it is. + let (reserved_slot, node) = if (node_index == ROOT_INDEX) { + assert!(path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); // Splitting root now, need to create a new root. - // We keep root_index always the same + // Since root is stored direclty in the resource, we will swap-in the new node there. let new_root_node = new_node(/*is_leaf=*/false); - let (replacement_node_stored_slot, replacement_node_slot) = self.nodes.reserve_slot(); - // aptos_std::debug::print(&replacement_node_slot); + // Reserve a slot where the current root will be moved to. + let (replacement_node_slot, replacement_node_reserved_slot) = self.nodes.reserve_slot(); - let root_children = &self.root.children; - let max_element = *root_children.new_end_iter().iter_prev(root_children).iter_borrow_key(root_children); - if (cmp::compare(&max_element, &key).is_less_than()) { - max_element = key; + let max_key = { + let root_children = &self.root.children; + let max_key = *root_children.new_end_iter().iter_prev(root_children).iter_borrow_key(root_children); + // need to check if key is largest, as invariant is that "parent's pointers" have been updated, + // but key itself can be larger than all previous ones. + if (cmp::compare(&max_key, &key).is_lt()) { + max_key = key; + }; + max_key }; - new_root_node.children.add(max_element, new_inner_child(replacement_node_stored_slot)); - - // aptos_std::debug::print(&cur_node_slot); - path_to_node.push_back(self.root_index); - + // New root will have start with a single child - the existing root (which will be at replacement location). + new_root_node.children.add(max_key, new_inner_child(replacement_node_slot)); let node = mem::replace(&mut self.root, new_root_node); - let replacement_ref = replacement_node_slot.reserved_as_ref(); + // we moved the currently processing node one level down, so we need to update the path + path_to_node.push_back(ROOT_INDEX); + + let replacement_index = replacement_node_reserved_slot.reserved_to_index(); if (node.is_leaf) { - self.min_leaf_index = replacement_ref; - self.max_leaf_index = replacement_ref; + // replacement node is the only leaf, so we update the pointers: + self.min_leaf_index = replacement_index; + self.max_leaf_index = replacement_index; }; - (replacement_node_slot, node) + (replacement_node_reserved_slot, node) } else { - let (cur_node_slot, node) = self.nodes.remove_and_reserve(node_index); - (cur_node_slot, node) + // In order to work on multiple nodes at the same time, we cannot borrow_mut, and need to be + // remove_and_reserve existing node. + let (cur_node_reserved_slot, node) = self.nodes.remove_and_reserve(node_index); + (cur_node_reserved_slot, node) }; - // aptos_std::debug::print(&std::string::utf8(b"node that needs to be split")); - // aptos_std::debug::print(&node); - + // move node_index out of scope, to make sure we don't accidentally access it, as we are done with it. + // (i.e. we should be using `reserved_slot` instead). move node_index; - let is_leaf = node.is_leaf; - let children = &mut node.children; - let right_node_ref = right_node_slot.reserved_as_ref(); - let next = &mut node.next; - let prev = &mut node.prev; + // Now we can perform the split at the current level, as we know we are not at the root level. + assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + + // Parent has a reference under max key to the current node, so existing index + // needs to be the right node. + // Since ordered_map::trim moves from the end (i.e. smaller keys stay), + // we are going to put the contents of the current node on the left side, + // and create a new right node. + // So if we had before (node_index, node), we will change that to end up having: + // (new_left_node_index, node trimmed off) and (node_index, new node with trimmed off children) + // + // So let's rename variables cleanly: + let right_node_reserved_slot = reserved_slot; + let left_node = node; + + let is_leaf = left_node.is_leaf; + let left_children = &mut left_node.children; + + let right_node_index = right_node_reserved_slot.reserved_to_index(); + let left_next = &mut left_node.next; + let left_prev = &mut left_node.prev; + + // compute the target size for the left node: let max_degree = if (is_leaf) { self.leaf_max_degree as u64 } else { @@ -627,89 +704,96 @@ module aptos_std::big_ordered_map { }; let target_size = (max_degree + 1) / 2; - children.add(key, child); - let new_node_children = children.trim(target_size); + // Add child (which will exceed the size), and then trim off to create two sets of children of correct sizes. + left_children.add(key, child); + let right_node_children = left_children.trim(target_size); - assert!(children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); - assert!(new_node_children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + assert!(left_children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + assert!(right_node_children.length() <= max_degree, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); - let right_node = new_node_with_children(is_leaf, new_node_children); + let right_node = new_node_with_children(is_leaf, right_node_children); - let (left_node_stored_slot, left_node_slot) = self.nodes.reserve_slot(); - let left_node_ref = left_node_stored_slot.stored_as_ref(); - right_node.next = *next; - *next = right_node_ref; - right_node.prev = left_node_ref; - if (!prev.ref_is_null()) { - self.nodes.borrow_mut(*prev).next = left_node_ref; - }; + let (left_node_slot, left_node_reserved_slot) = self.nodes.reserve_slot(); + let left_node_index = left_node_slot.stored_to_index(); - let split_key = *children.new_end_iter().iter_prev(children).iter_borrow_key(children); + // right nodes next is the node that was next of the left (previous) node, and next of left node is the right node. + right_node.next = *left_next; + *left_next = right_node_index; - // aptos_std::debug::print(&std::string::utf8(b"creating right node")); - // aptos_std::debug::print(&right_node_slot); - // aptos_std::debug::print(&right_node); + // right nodes previous is current left node + right_node.prev = left_node_index; + // Since the previuosly used index is going to the right node, `prev` pointer of the next node is correct, + // and we need to update next pointer of the previous node (if exists) + if (*left_prev != NULL_INDEX) { + self.nodes.borrow_mut(*left_prev).next = left_node_index; + }; + // Otherwise, we were the smallest node on the level. if this is the leaf level, update the pointer. + if (right_node_index == self.min_leaf_index) { + self.min_leaf_index = left_node_index; + }; - // aptos_std::debug::print(&std::string::utf8(b"updating left node")); - // aptos_std::debug::print(&left_node_slot); - // aptos_std::debug::print(&node); + // Largest left key is the split key. + let max_left_key = *left_children.new_end_iter().iter_prev(left_children).iter_borrow_key(left_children); - self.nodes.fill_reserved_slot(left_node_slot, node); - self.nodes.fill_reserved_slot(right_node_slot, right_node); + self.nodes.fill_reserved_slot(left_node_reserved_slot, left_node); + self.nodes.fill_reserved_slot(right_node_reserved_slot, right_node); - if (right_node_ref == self.min_leaf_index) { - self.min_leaf_index = left_node_ref; - }; - self.add_at(path_to_node, split_key, new_inner_child(left_node_stored_slot), false).destroy_none(); + // Add new Child (i.e. pointer to the left node) in the parent. + self.add_at(path_to_node, max_left_key, new_inner_child(left_node_slot), false).destroy_none(); option::none() } - fun update_key(self: &mut BigOrderedMap, path_to_node: vector, old_key: &K, new_key: K) { - if (path_to_node.is_empty()) { - return - }; - - let node_index = path_to_node.pop_back(); - let node = self.borrow_node_mut(node_index); - let children = &mut node.children; - children.replace_key_inplace(old_key, new_key); + /// Given a path to node (excluding the node itself), which is currently stored under "old_key", update "old_key" to "new_key". + fun update_key(self: &mut BigOrderedMap, path_to_node: vector, old_key: &K, new_key: K) { + while (!path_to_node.is_empty()) { + let node_index = path_to_node.pop_back(); + let node = self.borrow_node_mut(node_index); + let children = &mut node.children; + children.replace_key_inplace(old_key, new_key); - if (children.new_end_iter().iter_prev(children).iter_borrow_key(children) == &new_key) { - self.update_key(path_to_node, old_key, new_key); - }; + // If we were not updating the largest child, we don't need to continue. + if (children.new_end_iter().iter_prev(children).iter_borrow_key(children) != &new_key) { + return + }; + } } - fun remove_at(self: &mut BigOrderedMap, path_to_node: vector, key: &K): Child { + fun remove_at(self: &mut BigOrderedMap, path_to_node: vector, key: &K): Child { + // Last node in the path is one where we need to add the child to. let node_index = path_to_node.pop_back(); let old_child = { + // First check if we can perform this operation, without changing structure of the tree (i.e. without adding any nodes). + + // For that we can just borrow the single node let node = self.borrow_node_mut(node_index); let children = &mut node.children; - let is_leaf = node.is_leaf; let old_child = children.remove(key); - if (path_to_node.is_empty()) { - assert!(node_index == self.root_index, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + if (node_index == ROOT_INDEX) { + // If current node is root, lower limit of max_degree/2 nodes doesn't apply. + // So we can adjust internally + + assert!(path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); if (!is_leaf && children.length() == 1) { - // promote only child to root, and drop current root. - // keep the root index the same. + // If root is not leaf, but has a single child, promote only child to root, + // and drop current root. Since root is stored directly in the resource, we + // "move" the child into the root. + let Child::Inner { node_index: inner_child_index, } = children.new_end_iter().iter_prev(children).iter_remove(children); - move children; - move node; let inner_child = self.nodes.remove(inner_child_index); if (inner_child.is_leaf) { - let root_ref = self.root_index; - self.min_leaf_index = root_ref; - self.max_leaf_index = root_ref; + self.min_leaf_index = ROOT_INDEX; + self.max_leaf_index = ROOT_INDEX; }; mem::replace(&mut self.root, inner_child).destroy_empty_node(); - }; // else: nothing to change + }; return old_child; }; @@ -718,31 +802,33 @@ module aptos_std::big_ordered_map { } else { self.inner_max_degree as u64 }; - let current_size = children.length(); + + // See if the node is big enough, or we need to merge it with another node on this level. let big_enough = current_size * 2 >= max_degree; let new_max_key = *children.new_end_iter().iter_prev(children).iter_borrow_key(children); - let max_key_updated = cmp::compare(&new_max_key, key).is_less_than(); - if (!max_key_updated && big_enough) { - return old_child; - }; + // See if max key was updated for the current node, and if so - update it on the path. + let max_key_updated = cmp::compare(&new_max_key, key).is_lt(); if (max_key_updated) { assert!(current_size >= 1, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); self.update_key(path_to_node, key, new_max_key); + }; - if (big_enough) { - return old_child; - } + // If node is big enough after removal, we are done. + if (big_enough) { + return old_child; }; old_child }; - // Children size is below threshold, we need to rebalance + // Children size is below threshold, we need to rebalance with a neighbor on the same level. + // In order to work on multiple nodes at the same time, we cannot borrow_mut, and need to be + // remove_and_reserve existing node. let (node_slot, node) = self.nodes.remove_and_reserve(node_index); let is_leaf = node.is_leaf; @@ -750,9 +836,12 @@ module aptos_std::big_ordered_map { let prev = node.prev; let next = node.next; - let brother_index = { + // index of the node we will rebalance with. + let sibling_index = { let parent_children = &self.borrow_node(*path_to_node.borrow(path_to_node.length() - 1)).children; - if (parent_children.new_end_iter().iter_prev(parent_children).iter_borrow(parent_children).node_index.stored_as_ref() == node_index) { + assert!(parent_children.length() >= 2, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + // We merge with previous node - if it has the same parent, otherwise with next node (which then needs to have the same parent) + if (parent_children.new_end_iter().iter_prev(parent_children).iter_borrow(parent_children).node_index.stored_to_index() == node_index) { prev } else { next @@ -760,93 +849,105 @@ module aptos_std::big_ordered_map { }; let children = &mut node.children; - let (brother_slot, brother_node) = self.nodes.remove_and_reserve(brother_index); - let brother_children = &mut brother_node.children; + let (sibling_slot, sibling_node) = self.nodes.remove_and_reserve(sibling_index); + let sibling_children = &mut sibling_node.children; - if ((brother_children.length() - 1) * 2 >= max_degree) { - // The brother node has enough elements, borrow an element from the brother node. - if (brother_index == next) { + if ((sibling_children.length() - 1) * 2 >= max_degree) { + // The sibling node has enough elements, we can just borrow an element from the sibling node. + if (sibling_index == next) { + // if sibling is a larger node, we remove a child from the start let old_max_key = *children.new_end_iter().iter_prev(children).iter_borrow_key(children); - let brother_begin_iter = brother_children.new_begin_iter(); - let borrowed_max_key = *brother_begin_iter.iter_borrow_key(brother_children); - let borrowed_element = brother_begin_iter.iter_remove(brother_children); + let sibling_begin_iter = sibling_children.new_begin_iter(); + let borrowed_max_key = *sibling_begin_iter.iter_borrow_key(sibling_children); + let borrowed_element = sibling_begin_iter.iter_remove(sibling_children); children.add(borrowed_max_key, borrowed_element); + + // max_key of the current node changed, so update self.update_key(path_to_node, &old_max_key, borrowed_max_key); } else { - let brother_end_iter = brother_children.new_end_iter().iter_prev(brother_children); - let borrowed_max_key = *brother_end_iter.iter_borrow_key(brother_children); - let borrowed_element = brother_end_iter.iter_remove(brother_children); + // if sibling is a smaller node, we remove a child from the end + let sibling_end_iter = sibling_children.new_end_iter().iter_prev(sibling_children); + let borrowed_max_key = *sibling_end_iter.iter_borrow_key(sibling_children); + let borrowed_element = sibling_end_iter.iter_remove(sibling_children); children.add(borrowed_max_key, borrowed_element); - self.update_key(path_to_node, &borrowed_max_key, *brother_children.new_end_iter().iter_prev(brother_children).iter_borrow_key(brother_children)); + + // max_key of the sibling node changed, so update + self.update_key(path_to_node, &borrowed_max_key, *sibling_children.new_end_iter().iter_prev(sibling_children).iter_borrow_key(sibling_children)); }; self.nodes.fill_reserved_slot(node_slot, node); - self.nodes.fill_reserved_slot(brother_slot, brother_node); + self.nodes.fill_reserved_slot(sibling_slot, sibling_node); return old_child; }; - // The brother node doesn't have enough elements to borrow, merge with the brother node. - if (brother_index == next) { - let Node { children: brother_children, is_leaf: _, prev: _, next: brother_next } = brother_node; + // The sibling node doesn't have enough elements to borrow, merge with the sibling node. + // Keep the slot of the larger node of the two, to not require updating key on the parent nodes. + // But append to the smaller node, as ordered_map::append is more efficient when adding to the end. + let (key_to_remove, reserved_slot_to_remove) = if (sibling_index == next) { + // destroying larger sibling node, keeping sibling_slot. + let Node { children: sibling_children, is_leaf: _, prev: _, next: sibling_next } = sibling_node; let key_to_remove = *children.new_end_iter().iter_prev(children).iter_borrow_key(children); - children.append(brother_children); - node.next = brother_next; + children.append(sibling_children); + node.next = sibling_next; - move children; - - if (!node.next.ref_is_null()) { - self.nodes.borrow_mut(node.next).prev = brother_index; - }; - if (!node.prev.ref_is_null()) { - self.nodes.borrow_mut(node.prev).next = brother_index; + if (node.next != NULL_INDEX) { + assert!(self.nodes.borrow_mut(node.next).prev == sibling_index, 1); }; - self.nodes.fill_reserved_slot(brother_slot, node); - + // we are removing node_index, which previous's node's next was pointing to, + // so update the pointer + if (node.prev != NULL_INDEX) { + self.nodes.borrow_mut(node.prev).next = sibling_index; + }; + // Otherwise, we were the smallest node on the level. if this is the leaf level, update the pointer. if (self.min_leaf_index == node_index) { - self.min_leaf_index = brother_index; + self.min_leaf_index = sibling_index; }; - assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); - let node_stored_slot = destroy_inner_child(self.remove_at(path_to_node, &key_to_remove)); - self.nodes.free_reserved_slot(node_slot, node_stored_slot); + self.nodes.fill_reserved_slot(sibling_slot, node); + + (key_to_remove, node_slot) } else { + // destroying larger current node, keeping node_slot let Node { children: node_children, is_leaf: _, prev: _, next: node_next } = node; - let key_to_remove = *brother_children.new_end_iter().iter_prev(brother_children).iter_borrow_key(brother_children); - brother_children.append(node_children); - brother_node.next = node_next; + let key_to_remove = *sibling_children.new_end_iter().iter_prev(sibling_children).iter_borrow_key(sibling_children); + sibling_children.append(node_children); + sibling_node.next = node_next; - move brother_children; - - if (!brother_node.next.ref_is_null()) { - self.nodes.borrow_mut(brother_node.next).prev = node_index; + if (sibling_node.next != NULL_INDEX) { + assert!(self.nodes.borrow_mut(sibling_node.next).prev == node_index, 1); }; - if (!brother_node.prev.ref_is_null()) { - self.nodes.borrow_mut(brother_node.prev).next = node_index; + // we are removing sibling node_index, which previous's node's next was pointing to, + // so update the pointer + if (sibling_node.prev != NULL_INDEX) { + self.nodes.borrow_mut(sibling_node.prev).next = node_index; }; - - self.nodes.fill_reserved_slot(node_slot, brother_node); - - if (self.min_leaf_index == brother_index) { + // Otherwise, sibling was the smallest node on the level. if this is the leaf level, update the pointer. + if (self.min_leaf_index == sibling_index) { self.min_leaf_index = node_index; }; - assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); - let node_stored_slot = destroy_inner_child(self.remove_at(path_to_node, &key_to_remove)); - self.nodes.free_reserved_slot(brother_slot, node_stored_slot); + self.nodes.fill_reserved_slot(node_slot, sibling_node); + + (key_to_remove, sibling_slot) }; + + assert!(!path_to_node.is_empty(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + let slot_to_remove = destroy_inner_child(self.remove_at(path_to_node, &key_to_remove)); + self.nodes.free_reserved_slot(reserved_slot_to_remove, slot_to_remove); + old_child } /// Returns the number of elements in the BigOrderedMap. fun length(self: &BigOrderedMap): u64 { - self.length_for_node(self.root_index) + self.length_for_node(ROOT_INDEX) } - fun length_for_node(self: &BigOrderedMap, node_index: RefToSlot): u64 { + fun length_for_node(self: &BigOrderedMap, node_index: u64): u64 { let node = self.borrow_node(node_index); if (node.is_leaf) { node.children.length() @@ -854,7 +955,7 @@ module aptos_std::big_ordered_map { let size = 0; node.children.for_each_ref(|_key, child| { - size = size + self.length_for_node(child.node_index.stored_as_ref()); + size = size + self.length_for_node(child.node_index.stored_to_index()); }); size } @@ -873,11 +974,11 @@ module aptos_std::big_ordered_map { fun print_map(self: &BigOrderedMap) { aptos_std::debug::print(&std::string::utf8(b"print map")); aptos_std::debug::print(self); - self.print_map_for_node(self.root_index, 0); + self.print_map_for_node(ROOT_INDEX, 0); } #[test_only] - fun print_map_for_node(self: &BigOrderedMap, node_index: RefToSlot, level: u64) { + fun print_map_for_node(self: &BigOrderedMap, node_index: u64, level: u64) { let node = self.borrow_node(node_index); aptos_std::debug::print(&level); @@ -886,7 +987,7 @@ module aptos_std::big_ordered_map { if (!node.is_leaf) { node.children.for_each_ref(|_key, node| { - self.print_map_for_node(node.node_index.stored_as_ref(), level + 1); + self.print_map_for_node(node.node_index.stored_to_index(), level + 1); }); }; } @@ -934,14 +1035,14 @@ module aptos_std::big_ordered_map { } #[test_only] - fun validate_subtree(self: &BigOrderedMap, node_index: RefToSlot, expected_lower_bound_key: Option, expected_max_key: Option) { + fun validate_subtree(self: &BigOrderedMap, node_index: u64, expected_lower_bound_key: Option, expected_max_key: Option) { let node = self.borrow_node(node_index); let len = node.children.length(); assert!(len <= self.get_max_degree(node.is_leaf), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); - if (node_index != self.root_index) { + if (node_index != ROOT_INDEX) { assert!(len >= 1, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); - assert!(len * 2 >= self.get_max_degree(node.is_leaf) || node_index == self.root_index, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + assert!(len * 2 >= self.get_max_degree(node.is_leaf) || node_index == ROOT_INDEX, error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); }; node.children.validate_ordered(); @@ -949,7 +1050,7 @@ module aptos_std::big_ordered_map { let previous_max_key = expected_lower_bound_key; node.children.for_each_ref(|key: &K, child: &Child| { if (!node.is_leaf) { - self.validate_subtree(child.node_index.stored_as_ref(), previous_max_key, option::some(*key)); + self.validate_subtree(child.node_index.stored_to_index(), previous_max_key, option::some(*key)); } else { assert!((child is Child::Leaf), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); }; @@ -963,13 +1064,13 @@ module aptos_std::big_ordered_map { if (option::is_some(&expected_lower_bound_key)) { let expected_lower_bound_key = option::extract(&mut expected_lower_bound_key); - assert!(cmp::compare(&expected_lower_bound_key, node.children.new_begin_iter().iter_borrow_key(&node.children)).is_less_than(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); + assert!(cmp::compare(&expected_lower_bound_key, node.children.new_begin_iter().iter_borrow_key(&node.children)).is_lt(), error::invalid_state(EINTERNAL_INVARIANT_BROKEN)); }; } #[test_only] fun validate_map(self: &BigOrderedMap) { - self.validate_subtree(self.root_index, option::none(), option::none()); + self.validate_subtree(ROOT_INDEX, option::none(), option::none()); self.validate_iteration(); } diff --git a/aptos-move/framework/aptos-stdlib/sources/data_structures/storage_slots_allocator.move b/aptos-move/framework/aptos-stdlib/sources/data_structures/storage_slots_allocator.move index a4e480721267f2..cd4d287552d571 100644 --- a/aptos-move/framework/aptos-stdlib/sources/data_structures/storage_slots_allocator.move +++ b/aptos-move/framework/aptos-stdlib/sources/data_structures/storage_slots_allocator.move @@ -13,6 +13,7 @@ /// /// In the future, more sophisticated strategies can be added, without breaking/modifying callers, /// for example: +/// * inlining some nodes /// * having a fee-payer for any storage creation operations module aptos_std::storage_slots_allocator { friend aptos_std::big_ordered_map; @@ -24,8 +25,7 @@ module aptos_std::storage_slots_allocator { const EINTERNAL_INVARIANT_BROKEN: u64 = 7; const NULL_INDEX: u64 = 0; - const SPECIAL_SLOT_INDEX: u64 = 1; - const FIRST_INDEX: u64 = 10; // keeping space for new special values + const FIRST_INDEX: u64 = 10; // keeping space for usecase-specific values /// Data stored in an individual slot enum Link has store { @@ -71,13 +71,6 @@ module aptos_std::storage_slots_allocator { slot_index: u64, } - /// (Weak) Reference to a slot. - /// We can have variety of `RefToSlot`, but only a single `StoredSlot`. - /// It is on the caller to make sure references are not used after slot is freed. - struct RefToSlot has copy, drop, store { - slot_index: u64, - } - public fun new(config: StorageSlotsAllocatorConfig): StorageSlotsAllocator { let result = StorageSlotsAllocator::V1 { slots: option::none(), @@ -116,12 +109,14 @@ module aptos_std::storage_slots_allocator { } public fun remove(self: &mut StorageSlotsAllocator, slot: StoredSlot): T { - let (reserved_slot, value) = self.remove_and_reserve(slot.stored_as_ref()); + let (reserved_slot, value) = self.remove_and_reserve(slot.stored_to_index()); self.free_reserved_slot(reserved_slot, slot); value } - public(friend) fun destroy(self: StorageSlotsAllocator) { + /// We cannot know if allocator is empty or not, so this method is not public, + /// and can be used only in modules that know by themselves that allocator is empty. + public(friend) fun destroy_known_empty_unsafe(self: StorageSlotsAllocator) { loop { let reuse_index = self.maybe_pop_from_reuse_queue(); if (reuse_index == NULL_INDEX) { @@ -137,17 +132,17 @@ module aptos_std::storage_slots_allocator { reuse_spare_count: _, } => { assert!(reuse_head_index == NULL_INDEX, EINTERNAL_INVARIANT_BROKEN); - slots.destroy_some().destroy(); + slots.destroy_some().destroy_known_empty_unsafe(); }, }; } - public fun borrow(self: &StorageSlotsAllocator, slot: RefToSlot): &T { - &self.slots.borrow().borrow(slot.slot_index).value + public fun borrow(self: &StorageSlotsAllocator, slot_index: u64): &T { + &self.slots.borrow().borrow(slot_index).value } - public fun borrow_mut(self: &mut StorageSlotsAllocator, slot: RefToSlot): &mut T { - &mut self.slots.borrow_mut().borrow_mut(slot.slot_index).value + public fun borrow_mut(self: &mut StorageSlotsAllocator, slot_index: u64): &mut T { + &mut self.slots.borrow_mut().borrow_mut(slot_index).value } // We also provide here operations where `add()` is split into `reserve_slot`, @@ -173,8 +168,7 @@ module aptos_std::storage_slots_allocator { } /// Remove storage slot, but reserve it for later. - public fun remove_and_reserve(self: &mut StorageSlotsAllocator, slot: RefToSlot): (ReservedSlot, T) { - let slot_index = slot.slot_index; + public fun remove_and_reserve(self: &mut StorageSlotsAllocator, slot_index: u64): (ReservedSlot, T) { let Link::Occupied { value } = self.remove_link(slot_index); (ReservedSlot { slot_index }, value) } @@ -188,24 +182,20 @@ module aptos_std::storage_slots_allocator { // ========== Section for methods handling references ======== - public fun reserved_as_ref(self: &ReservedSlot): RefToSlot { - RefToSlot { slot_index: self.slot_index } - } - - public fun stored_as_ref(self: &StoredSlot): RefToSlot { - RefToSlot { slot_index: self.slot_index } + public fun reserved_to_index(self: &ReservedSlot): u64 { + self.slot_index } - public fun null_ref(): RefToSlot { - RefToSlot { slot_index: NULL_INDEX } + public fun stored_to_index(self: &StoredSlot): u64 { + self.slot_index } - public fun special_ref(): RefToSlot { - RefToSlot { slot_index: SPECIAL_SLOT_INDEX } + public fun is_null_index(slot_index: u64): bool { + slot_index == NULL_INDEX } - public fun ref_is_null(self: &RefToSlot): bool { - self.slot_index == NULL_INDEX + public fun is_special_unused_index(slot_index: u64): bool { + slot_index != NULL_INDEX && slot_index < FIRST_INDEX } // ========== Section for private internal utility methods ======== diff --git a/aptos-move/framework/aptos-stdlib/sources/table.move b/aptos-move/framework/aptos-stdlib/sources/table.move index 8c176983f995a7..67b3f7ffb7d6ac 100644 --- a/aptos-move/framework/aptos-stdlib/sources/table.move +++ b/aptos-move/framework/aptos-stdlib/sources/table.move @@ -90,7 +90,7 @@ module aptos_std::table { /// Table cannot know if it is empty or not, so this method is not public, /// and can be used only in modules that know by themselves that table is empty. - public(friend) fun destroy(self: Table) { + public(friend) fun destroy_known_empty_unsafe(self: Table) { destroy_empty_box>(&self); drop_unchecked_box>(self) } diff --git a/aptos-move/framework/aptos-stdlib/sources/table.spec.move b/aptos-move/framework/aptos-stdlib/sources/table.spec.move index 139ae93a641e08..07c46f257abb95 100644 --- a/aptos-move/framework/aptos-stdlib/sources/table.spec.move +++ b/aptos-move/framework/aptos-stdlib/sources/table.spec.move @@ -6,7 +6,7 @@ spec aptos_std::table { spec Table { pragma intrinsic = map, map_new = new, - map_destroy_empty = destroy, + map_destroy_empty = destroy_known_empty_unsafe, map_has_key = contains, map_add_no_override = add, map_add_override_if_exists = upsert, @@ -24,7 +24,7 @@ spec aptos_std::table { pragma intrinsic; } - spec destroy { + spec destroy_known_empty_unsafe { pragma intrinsic; } diff --git a/aptos-move/framework/aptos-stdlib/sources/table_with_length.move b/aptos-move/framework/aptos-stdlib/sources/table_with_length.move index e4ca2415bc9390..f278eef8402faa 100644 --- a/aptos-move/framework/aptos-stdlib/sources/table_with_length.move +++ b/aptos-move/framework/aptos-stdlib/sources/table_with_length.move @@ -28,7 +28,7 @@ module aptos_std::table_with_length { public fun destroy_empty(self: TableWithLength) { assert!(self.length == 0, error::invalid_state(ENOT_EMPTY)); let TableWithLength { inner, length: _ } = self; - table::destroy(inner) + table::destroy_known_empty_unsafe(inner) } /// Add a new entry to the table. Aborts if an entry for this diff --git a/aptos-move/framework/move-stdlib/doc/bcs.md b/aptos-move/framework/move-stdlib/doc/bcs.md index 0a73d399023415..fcc176e1751c0a 100644 --- a/aptos-move/framework/move-stdlib/doc/bcs.md +++ b/aptos-move/framework/move-stdlib/doc/bcs.md @@ -85,7 +85,7 @@ On the other hand, if function has returned None for some type, it might change in the future to return Some() instead, if size becomes "known". -
public(friend) fun constant_serialized_size<MoveValue>(): option::Option<u64>
+
public fun constant_serialized_size<MoveValue>(): option::Option<u64>
 
@@ -94,7 +94,7 @@ it might change in the future to return Some() instead, if size becomes "known". Implementation -
native public(friend) fun constant_serialized_size<MoveValue>(): Option<u64>;
+
native public fun constant_serialized_size<MoveValue>(): Option<u64>;
 
diff --git a/crates/transaction-generator-lib/src/publishing/module_simple.rs b/crates/transaction-generator-lib/src/publishing/module_simple.rs index 04ec6f9f347dab..cf5897a9e2ba89 100644 --- a/crates/transaction-generator-lib/src/publishing/module_simple.rs +++ b/crates/transaction-generator-lib/src/publishing/module_simple.rs @@ -115,7 +115,10 @@ pub enum LoopType { pub enum MapType { SimpleMap, OrderedMap, - BigOrderedMap{ inner_max_degree: u16, leaf_max_degree: u16 } + BigOrderedMap { + inner_max_degree: u16, + leaf_max_degree: u16, + }, } /// Automatic arguments function expects (i.e. signer, or multiple signers, etc) @@ -645,14 +648,14 @@ impl EntryPoints { repeats, map_type, } => { - let mut args = vec![ - bcs::to_bytes(len).unwrap(), - bcs::to_bytes(repeats).unwrap(), - ]; + let mut args = vec![bcs::to_bytes(len).unwrap(), bcs::to_bytes(repeats).unwrap()]; let func = match map_type { MapType::SimpleMap => ident_str!("test_add_remove_simple_map").to_owned(), MapType::OrderedMap => ident_str!("test_add_remove_ordered_map").to_owned(), - MapType::BigOrderedMap { inner_max_degree, leaf_max_degree } => { + MapType::BigOrderedMap { + inner_max_degree, + leaf_max_degree, + } => { args.push(bcs::to_bytes(inner_max_degree).unwrap()); args.push(bcs::to_bytes(leaf_max_degree).unwrap()); ident_str!("test_add_remove_big_ordered_map").to_owned() @@ -660,7 +663,7 @@ impl EntryPoints { }; get_payload(module_id, func, args) - } , + }, EntryPoints::TokenV1InitializeCollection => get_payload_void( module_id, ident_str!("token_v1_initialize_collection").to_owned(), @@ -933,7 +936,7 @@ impl EntryPoints { EntryPoints::VectorTrimAppend { .. } | EntryPoints::VectorRemoveInsert { .. } | EntryPoints::VectorRangeMove { .. } => AutomaticArgs::None, - | EntryPoints::MapInsertRemove { .. } => AutomaticArgs::Signer, + EntryPoints::MapInsertRemove { .. } => AutomaticArgs::Signer, EntryPoints::TokenV1InitializeCollection | EntryPoints::TokenV1MintAndStoreNFTParallel | EntryPoints::TokenV1MintAndStoreNFTSequential diff --git a/testsuite/module-publish/src/packages/framework_usecases/sources/maps_example.move b/testsuite/module-publish/src/packages/framework_usecases/sources/maps_example.move index b0017b6fe73c71..698060c4fa8a44 100644 --- a/testsuite/module-publish/src/packages/framework_usecases/sources/maps_example.move +++ b/testsuite/module-publish/src/packages/framework_usecases/sources/maps_example.move @@ -69,7 +69,7 @@ module 0xABCD::maps_example { } public entry fun test_add_remove_big_ordered_map(sender: &signer, len: u64, repeats: u64, inner_max_degree: u16, leaf_max_degree: u16) { - let map = big_ordered_map::new_with_config(inner_max_degree, leaf_max_degree, true, false, 0); + let map = big_ordered_map::new_with_config(inner_max_degree, leaf_max_degree, false, 0); do_test_add_remove( len, repeats,