kaspanet / rusty-kaspa

Kaspa full-node and related libraries in the Rust programming language. This is a stable version at the initial rollout phases.
ISC License
359 stars 109 forks source link

Add serialization benchmarks and dependencies to the project #223

Closed biryukovmaxim closed 10 months ago

biryukovmaxim commented 10 months ago

resolves #121 This mr introduces bincode and serde_bytes as dependencies. These are added to improve the handling of transaction serialization/deserialization in tx.rs, applying the serde_bytes specifically to the signature_script and payload of a transaction.

Moreover, benchmarks for serialization and deserialization are also introduced within a new file serde_benchmark.rs. This would help us evaluate the performance of these processes.

The criterion is added as a development dependency to aid in benchmarking.

Changes in the Cargo.toml files reflect these added dependencies.

This should have a positive effect on DbBlockTransactionsStore, which returns Vec<Transaction> internally deserializing them with bincode

elichai commented 10 months ago

It seems like the signature_script and payload are empty in the benchmark, so I don't think this is benchmarking this, It looks like it mostly benchmarks the allocations, so try to serialize into a pre allocated buffer as a benchmark (deserialize is a bit harder to benchmark as these contain vecs)

biryukovmaxim commented 10 months ago

It seems like the signature_script

You are right, I forgot to fill it, but payload wasn't empty

try to serialize into a pre allocated buffer

Done

D-Stacks commented 10 months ago

Just for reference, so not everyone has to compare, these are benchmark comparisons:

with serde bytes:

Serialize Transaction   time:   [170.64 ns 172.15 ns 174.00 ns]
Found 14 outliers among 100 measurements (14.00%)
  5 (5.00%) high mild
  9 (9.00%) high severe

Deserialize Transaction time:   [343.80 ns 348.69 ns 354.06 ns]
Found 9 outliers among 100 measurements (9.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild
  4 (4.00%) high severe

without serde_bytes:

     Running benches/serde_benchmark.rs (target/release/deps/serde_benchmark-2db7ac81462d12be)
Serialize Transaction   time:   [282.00 ns 287.67 ns 293.82 ns]
                        change: [+60.962% +69.534% +78.169%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 9 outliers among 100 measurements (9.00%)
  2 (2.00%) high mild
  7 (7.00%) high severe

Deserialize Transaction time:   [420.95 ns 430.08 ns 439.58 ns]
                        change: [+17.345% +22.597% +27.728%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 8 outliers among 100 measurements (8.00%)
  3 (3.00%) high mild
  5 (5.00%) high severe

furthermore, i think script public keys can also be done with serde_bytes to possibly increase the efficiency, possibly hashes themselves as well (instead of u64s).

biryukovmaxim commented 10 months ago

furthermore, i think script public keys can also be done with serde_bytes to possibly increase the efficiency, possibly hashes themselves as well (instead of u64s).

Do you mean, ScriptPublicKey or script: ScriptVec inside of it?currently ScriptVec is SmallVec, which has its own implementation of serialization

biryukovmaxim commented 10 months ago

I implement Deserializer interface for ScriptPublicKey using almost the same struct, but containing &[u8] instead of SmallVec. these are benchmark comparisons on my pc: before all changes(without serde_bytes, without manual implementation):

Serialize Transaction   time:   [233.20 ns 234.03 ns 234.92 ns]
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe

Deserialize Transaction time:   [421.70 ns 423.60 ns 425.47 ns]
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low mild
  2 (2.00%) high mild
  1 (1.00%) high severe

Serialize ScriptPublicKey
                        time:   [34.393 ns 34.475 ns 34.555 ns]
Found 8 outliers among 100 measurements (8.00%)
  3 (3.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe

Deserialize ScriptPublicKey
                        time:   [87.941 ns 88.625 ns 89.324 ns]
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

after all changes

Serialize Transaction   time:   [132.52 ns 132.68 ns 132.85 ns]
                        change: [-43.609% -43.307% -43.003%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low severe
  2 (2.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe

Deserialize Transaction time:   [245.13 ns 245.40 ns 245.71 ns]
                        change: [-42.375% -42.051% -41.733%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  2 (2.00%) high severe

Serialize ScriptPublicKey
                        time:   [33.753 ns 33.816 ns 33.884 ns]
                        change: [-2.2664% -1.7131% -1.0717%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  6 (6.00%) high severe

Deserialize ScriptPublicKey
                        time:   [36.755 ns 36.829 ns 36.914 ns]
                        change: [-58.523% -58.172% -57.809%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  4 (4.00%) high mild
  7 (7.00%) high severe

Serialize ScriptPublicKey there is measurement uncertainty here.

Also I tried to change Serialize implementation for ScriptPublicKey, but I reached only regression or nothing

@D-Stacks any other suggestions?

D-Stacks commented 10 months ago

not really, like i could imagine hashes etc, to increase in performance, but iirc we serialize / deserialize via 4 unit64s which might have other performance benifits. but script public key would be a nice addition, mostly because this is a main constituent in 2-3 separate db stores, and will scale more in transactions, as transactions can have multiple script_public_key fields in both their inputs and outputs.

biryukovmaxim commented 10 months ago

script public key would be a nice addition

This pr contains improvements in deserializing of script public key. It can be seen in the last benchmark

elichai commented 10 months ago

This looks great! I think having a benchmark suite for the db+network serializations too would be very helpful, then we could maybe ditch protobuf in favor of something like Capt'n Proto or our own serialization and we can check the differences with the benchmarks

biryukovmaxim commented 10 months ago

clippy suggests this code:

148 ~                 bincode::serialize_into(&mut buf, &script_public_key).unwrap();
149 ~                 black_box(());

that't really funny

michaelsutton commented 10 months ago

clippy suggests this code:

148 ~                 bincode::serialize_into(&mut buf, &script_public_key).unwrap();
149 ~                 black_box(());

that't really funny

Lol. Yes I've seen that before