Change Map iter to not create many Maps

[leighmcculloch]

What

Change Map iter to not create many Maps.

Why

The Map iterator currently makes a host copy of the map on every step of the iteration. This is because it removes seen items rom the map which causes a new map to be created host side. Even though it results in optimized guest code because the iterator is only a single u64 handle, those optimizations are outsized by the calls to the host that repeatedly duplicate the map.

The contract size increases slightly, but the cost

Close #191

Alternatives

There might be some advantage in having the map use a shared backing structure for non-adding mutations. The host could lazily reuse the map backing data structure for read, and remove operations. We can adjust the guest side implementation of the map iterator relatively easily if the host side is improved.

Perf Diff

Code Size

Code size was marginally higher for the iterator itself, but smaller for the iterations themselves.

#![no_std]
use soroban_sdk::{contractimpl, Map};

pub struct Contract;

#[contractimpl]
impl Contract {
    pub fn iter(map: Map<i32, i32>) {
        let mut iter = map.iter();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
        _ = iter.next();
    }
}

@@ -1 +1 @@
--rwxr-xr-x  1 leighmcculloch  staff  1123 Jun 21 21:43 test_map.wasm
+-rwxr-xr-x  1 leighmcculloch  staff  1045 Jun 21 21:42 test_map.wasm

Budget

Budget was significantly smaller without the copies.

let env = Env::default();

let map = map![&env, (0, 0), (1, 10), (2, 20), (3, 30), (4, 40)];

env.budget().reset_default();

let mut iter = map.iter();
_ = iter.next();
_ = iter.next();
_ = iter.next();
_ = iter.next();
_ = iter.next();

env.budget().print();

@@ -1,21 +1,21 @@
 =======================================================
-Cpu limit: 40000000; used: 34156
-Mem limit: 52428800; used: 560
+Cpu limit: 40000000; used: 6638
+Mem limit: 52428800; used: 0
 =======================================================
 CostType                 cpu_insns      mem_bytes      
 WasmInsnExec             0              0              
 WasmMemAlloc             0              0              
-HostMemAlloc             23500          560            
-HostMemCpy               345            0              
+HostMemAlloc             0              0              
+HostMemCpy               0              0              
 HostMemCmp               0              0              
 InvokeHostFunction       0              0              
-VisitObject              285            0              
+VisitObject              247            0              
 ValXdrConv               0              0              
 ValSer                   0              0              
 ValDeser                 0              0              
 ComputeSha256Hash        0              0              
 ComputeEd25519PubKey     0              0              
-MapEntry                 2226           0              
+MapEntry                 2491           0              
 VecEntry                 0              0              
 GuardFrame               0              0              
 VerifyEd25519Sig         0              0              
@@ -23,7 +23,7 @@ VmMemRead                0              0
 VmMemWrite               0              0              
 VmInstantiation          0              0              
 InvokeVmFunction         0              0              
-ChargeBudget             7800           0              
+ChargeBudget             3900           0              
 ComputeKeccak256Hash     0              0              
 ComputeEcdsaSecp256k1Key 0              0              
 ComputeEcdsaSecp256k1Sig 0              0              

[graydon]

Looks great! We can probably do it with integer positions at some point, but for now this is likely fine, might not be much different cost-wise.

[leighmcculloch]

We can probably do it with integer positions at some point

That'd keep the size of the guest side value down. Right now it's 224 bits (64bit map, 64bit min, 64bit max, 32bit len). If we could use u32 indexes that'd make it the same size as Vec, 128 bits (64bit map, 32bit start, 32bit end).

Iteration cost would probably be the similar because we'd still need to get the key and value out of the host, which we do now with two host calls, one to map_next_key and one to map_get. Unless we added a fn that copied both key and map via linear memory in a single call?