Fixing the svm account serialization for performance

[godmodegalactus]

Problem

Performance issue with svm during serialization of accounts forces resize of memory because the initial size is not correctly calculated. This impact the performance of the svm a lot with current benchmarks and profiling with openbook contract we see the average performance jump up from 6360 tps to 8144 tps. This is around 30 % improvement which can be verified by profiling which spends 30 percent of the time in resizing.

Summary of Changes

Correctly calculating the initial size of the memory and adding few extra bytes to capacity to avoid potential memory reallocations.

Another change is making use of the method AlignedMemory::with_capacity_zeroed(size) which avoid any calls to resize which makes performance much faster.

• Here is detailed performance improved seen by the changes.

To benchmark the svm code we use following repository.
https://github.com/godmodegalactus/simulate-svm-contract

This repository creates continuously place order on openbook market and also runs crank instruction from time to time. The figures are generated after running the benchmark program for 500s to get the average tps.

Unoptimized serializer :

second: 500
tps (crank + market orders) : 6449.509284800379
tps (only market orders) : 5605.989915113488
average tps: 6360.181975384049

Optimized serialized with preallocated and zeroed memory

second: 500
tps (crank + market orders) : 8079.207521273256
tps (only market orders) : 6420.863942369388
average tps: 8144.030127013769

Optimized serialize without zeroed memory

second: 500
tps (crank + market orders) : 7710.865692534364
tps (only market orders) : 6029.487392405706
average tps: 7622.813683394391

Fixes #
No feature gates

[mergify]

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

[seanyoung]

It's not clear to me why this change is correct. When we calculate the size, what exactly are we missing?

I'm not saying that the code is perfect, it would just be good to know what the bug is rather than adding some random constants

[seanyoung]

Why would zeroing improve perf? In anything it should be slower

[godmodegalactus]

It is faster because fill_write during account serialization is already initialized so only set_len for vector is called, where as if we only increase the capacity vector resize is called which inturn call vector extend with value 0 which seems to be more expensive.

[godmodegalactus]

It's not clear to why this change is correct. When we calculate the size, what exactly are we missing?

I'm not saying that the code is perfect, it would just be good to know what the bug is rather than adding some random constants

There are few bytes missing, 4 bytes added after executable, datalen allocated 4 bytes instead of 8 bytes, each account has one align offset bytes not included.

[LucasSte]

Why is this padding necessary?

[godmodegalactus]

because of this :

agave/programs/bpf_loader/src/serialization.rs

Line 471 in f36a62d

s.write_all(&[0u8, 0, 0, 0]);

[seanyoung]

That matched up with + size_of::<u32>() // original_data_len

[seanyoung]

Please try to reproduce this issue with a test case that shows the size calculation is incorrect.

I'm running a mainet validator with the following patch:

agave$ git diff
diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs
index 2cb2f20b16..abde03cde6 100644
--- a/programs/bpf_loader/src/serialization.rs
+++ b/programs/bpf_loader/src/serialization.rs
@@ -495,6 +495,8 @@ fn serialize_parameters_aligned(
     s.write_all(program_id.as_ref());
 
     let (mem, regions) = s.finish();
+
+    assert_eq!(mem.len(), size);
     Ok((mem, regions, accounts_metadata))
 }

Nothing is coming up - I don't think there is a problem here