feat: Update contract leaf size

[bvrooman]

Related issue:

• Closes Use 16kib leaf sizes when calculating code roots #434
• Closes Large contracts take a very long time to merkleize #385

[Voxelot]

should we have a unit test to verify padding behavior?

e.g. compare with a manually constructed leaf vs the chunking algorithm above

[xgreenx]

I think it is a good idea to add a new test to verify padding. The change looks good for me, could you also update the changelog please?

[xgreenx]

Still waiting for updated changelog=D

[xgreenx]

You don't need to create a vector here. You can still use a slice. But chunk during iteration, you need(maybe) to pad the last leaf.

[bvrooman]

Yep, that's a good point. I updated the algorithm to allocate and pad only if the last leaf is not already a multiple of 8.

[bvrooman]

Still waiting for updated changelog=D

Will update now

[xgreenx]

You can do that without vector=) You can create array [PADDING_BYTE; MULTIPLE] and only copy into first several bytes=)

[bvrooman]

I'm not sure that it's enough to create [PADDING_BYTE; MULTIPLE], since your padding size will be some factor of MULTIPLE (i.e., n * MULTIPLE) if the remainder is already greater than MULTIPLE. We don't know the size at compile time, so I use a vector to accommodate that unknown size.

Edit:

I tried modifying the algorithm to:

let mut padded_leaf = [PADDING_BYTE; MULTIPLE];
padded_leaf[0..len].clone_from_slice(leaf);
tree.push(padded_leaf.as_ref());

This results in some test failures. Maybe I don't understand the suggestion exactly?

[xgreenx]

I meant the following=) But I don't know which is faster, your variant or mine=)

[bvrooman]

Do you have a preference on which version to use? The advantage of the vector approach is that it will have a smaller average memory allocation than the array approach (0 to LEAF_SIZE - 1 vs. constant LEAF_SIZE). The advantage of the array approach is, of course, that it allocates on the stack, rather than the heap.

[xgreenx]

We need to decide it based on the benchmarks=) Could you do that, please?

[bvrooman]

I did some benchmarking using Criterion. I compared the two variations of the algorithm.

The first version v1 (using vec!):

...
let padding_size = next_multiple::<MULTIPLE>(len);
let mut padded_leaf = vec![PADDING_BYTE; padding_size];
padded_leaf[0..len].clone_from_slice(leaf);
tree.push(padded_leaf.as_ref());

The second version V2 (using an array):

...
let padding_size = next_multiple::<MULTIPLE>(len);
let mut padded_leaf = [PADDING_BYTE; LEAF_SIZE];
padded_leaf[0..len].clone_from_slice(leaf);
tree.push(padded_leaf[..padding_size].as_ref());

The bench setup is as follows:

const LEAF_SIZE: usize = 1024 * 16;

fn bench(c: &mut Criterion) {
    let mut rng = StdRng::seed_from_u64(0xF00D);
    let code_len = 3 * LEAF_SIZE + 100;
    let mut code = alloc::vec![0u8; code_len];
    rng.fill_bytes(code.as_mut_slice());

    let mut group = c.benchmark_group("contract");
    group.bench_with_input("root_from_code", &code, |b, code| {
        b.iter(|| Contract::root_from_code(code))
    });
}

criterion_group!(benches, bench);
criterion_main!(benches);

First, I ran the bench using v1. The output is as follows:

Running benches/bench.rs (/Volumes/Fuel/projects/FuelLabs/fuel-vm/target/release/deps/bench-7aacf08cf5869b41)
contract/root_from_code time:   [152.83 µs 153.27 µs 153.76 µs]
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild

Second, I ran the bench using v2. the output is as follows:

Running benches/bench.rs (/Volumes/Fuel/projects/FuelLabs/fuel-vm/target/release/deps/bench-7aacf08cf5869b41)
contract/root_from_code time:   [152.03 µs 152.39 µs 152.77 µs]
                        change: [-0.2665% +0.1694% +0.6561%] (p = 0.47 > 0.05)
                        No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe

I went back and forth between the two implementations, and further results were generally consistent with these initial benchmarks. In (almost*) every case, Criterion reports No change in performance detected.

*There were a couple of outlier tests where the difference was +/- ~30%, but I simply reran the benchmarks and these anomalies self-corrected.

In conclusion, it seems that there is negligible difference in performance between the two implementations. Since performance is not affected by implementation, I might suggest we go with v1 since we know that it will allocate less memory on average. But v2 is also good, and we can go with that if anyone feels strongly about that option.

I can also include the benchmark code in this PR if that would be helpful. Just let me know and I can commit it here.

[xgreenx]

Could you also try let code_len = 4 * LEAF_SIZE - 1; please?

If the results are the same, then I prefer a variant with an array=)

[bvrooman]

Using let code_len = 4 * LEAF_SIZE - 1;, the results are:

v1:

Running benches/bench.rs (/Volumes/Fuel/projects/FuelLabs/fuel-vm/target/release/deps/bench-7aacf08cf5869b41)
contract/root_from_code time:   [200.90 µs 201.42 µs 201.97 µs]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

v2:

Running benches/bench.rs (/Volumes/Fuel/projects/FuelLabs/fuel-vm/target/release/deps/bench-7aacf08cf5869b41)
contract/root_from_code time:   [200.95 µs 201.41 µs 201.86 µs]
                        change: [-0.1991% +0.1953% +0.6080%] (p = 0.34 > 0.05)
                        No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
  3 (3.00%) high mild
  2 (2.00%) high severe

Yes, the results appear to be the same, even when changing the partial leaf size.

In that case, I will update the code to use the array version.

Please let me know if you want me to include the bench code as well and I will clean it up and commit it for review.

[xgreenx]

If there is nothing special, then maybe it is better not to keep this benchmark because the chance that we need it again is low=)

[bvrooman]

Sounds good. I've updated the code now to use the array version, and it should be good to go.