Skip to content

perf: Directly serialize values to pages, no extra copies/allocs #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
BrianBland merged 1 commit into from
Feb 28, 2025
Merged

perf: Directly serialize values to pages, no extra copies/allocs #38

BrianBland merged 1 commit into from
Feb 28, 2025

Conversation

@BrianBland
Copy link
Collaborator

@BrianBland BrianBland commented Feb 27, 2025

Changes the Value trait for more efficient writing to disk. Replaces to_bytes with size and serialize_into, which allows for data to be written directly to the page buffer without needing an intermediate allocation or copy. This shows somewhere around a 15% improvement for 1K batch benchmarks.

Benchmarking read_operations/random_reads/1000000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 9.0s, enable flat sampling, or reduce sample count to 50.
read_operations/random_reads/1000000
                        time:   [1.7884 ms 1.7964 ms 1.8065 ms]
                        thrpt:  [553.56 Kelem/s 556.66 Kelem/s 559.17 Kelem/s]
                 change:
                        time:   [-2.8088% -2.2301% -1.6298%] (p = 0.00 < 0.05)
                        thrpt:  [+1.6568% +2.2810% +2.8900%]
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  5 (5.00%) high mild
  1 (1.00%) high severe
read_operations/random_reads/3000000
                        time:   [1.9856 ms 1.9909 ms 1.9962 ms]
                        thrpt:  [500.96 Kelem/s 502.29 Kelem/s 503.63 Kelem/s]
                 change:
                        time:   [-1.4452% -0.9779% -0.5300%] (p = 0.00 < 0.05)
                        thrpt:  [+0.5328% +0.9875% +1.4664%]
                        Change within noise threshold.

insert_operations/batch_inserts/1000000
                        time:   [47.194 ms 47.671 ms 48.210 ms]
                        thrpt:  [20.743 Kelem/s 20.977 Kelem/s 21.189 Kelem/s]
                 change:
                        time:   [-24.493% -21.124% -17.489%] (p = 0.00 < 0.05)
                        thrpt:  [+21.196% +26.781% +32.438%]
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  3 (3.00%) high mild
  5 (5.00%) high severe
Benchmarking insert_operations/batch_inserts/3000000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.2s, or reduce sample count to 60.
insert_operations/batch_inserts/3000000
                        time:   [75.130 ms 78.473 ms 81.812 ms]
                        thrpt:  [12.223 Kelem/s 12.743 Kelem/s 13.310 Kelem/s]
                 change:
                        time:   [-1.9497% +4.5814% +11.578%] (p = 0.17 > 0.05)
                        thrpt:  [-10.376% -4.3807% +1.9885%]
                        No change in performance detected.

Benchmarking update_operations/batch_updates/1000000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.9s, or reduce sample count to 80.
update_operations/batch_updates/1000000
                        time:   [53.427 ms 55.976 ms 58.509 ms]
                        thrpt:  [17.091 Kelem/s 17.865 Kelem/s 18.717 Kelem/s]
                 change:
                        time:   [-12.345% -6.4739% -1.0271%] (p = 0.04 < 0.05)
                        thrpt:  [+1.0377% +6.9220% +14.084%]
                        Performance has improved.
Benchmarking update_operations/batch_updates/3000000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.6s, or reduce sample count to 50.
update_operations/batch_updates/3000000
                        time:   [78.914 ms 82.468 ms 86.051 ms]
                        thrpt:  [11.621 Kelem/s 12.126 Kelem/s 12.672 Kelem/s]
                 change:
                        time:   [-3.7372% +2.7086% +9.5811%] (p = 0.42 > 0.05)
                        thrpt:  [-8.7434% -2.6372% +3.8823%]
                        No change in performance detected.

mixed_operations/mixed_workload/1000000
                        time:   [37.786 ms 38.165 ms 38.573 ms]
                        thrpt:  [25.925 Kelem/s 26.202 Kelem/s 26.465 Kelem/s]
                 change:
                        time:   [-16.253% -15.026% -13.774%] (p = 0.00 < 0.05)
                        thrpt:  [+15.974% +17.683% +19.407%]
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe
Benchmarking mixed_operations/mixed_workload/3000000: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.1s, or reduce sample count to 70.
mixed_operations/mixed_workload/3000000
                        time:   [64.606 ms 67.367 ms 70.197 ms]
                        thrpt:  [14.246 Kelem/s 14.844 Kelem/s 15.478 Kelem/s]
                 change:
                        time:   [-17.272% -12.415% -7.4973%] (p = 0.00 < 0.05)
                        thrpt:  [+8.1050% +14.174% +20.879%]
                        Performance has improved.

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Feb 27, 2025 •
edited
Loading

✅ Heimdall Review Status

Requirement Status More Info
Reviews ✅ 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@BrianBland BrianBland force-pushed the brianbland/more-efficient-value-serialization branch from ac8ef21 to 2305698 Compare February 27, 2025 23:19
@BrianBland BrianBland requested review from jjtny1 and nqd February 28, 2025 16:54
jjtny1
jjtny1 reviewed Feb 28, 2025
View reviewed changes
src/storage/value.rs
fn from_bytes(bytes: &[u8]) -> Result<Self>;

/// Serialize this value into a buffer
fn serialize(&self) -> Result<Vec<u8>> {
Copy link
Contributor

@jjtny1 jjtny1 Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Copy link
Collaborator Author

@BrianBland BrianBland Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely we won't use this method outside of testing, but it's a nice convenience factor

jjtny1
jjtny1 approved these changes Feb 28, 2025
View reviewed changes
Copy link
Contributor

@jjtny1 jjtny1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice this is a great low hanging fruit that actually gives a sizable improvement 💯

@BrianBland BrianBland merged commit cd9367e into main Feb 28, 2025
4 checks passed
@BrianBland BrianBland deleted the brianbland/more-efficient-value-serialization branch February 28, 2025 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jjtny1 jjtny1 jjtny1 approved these changes

@nqd nqd Awaiting requested review from nqd

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@BrianBland @cb-heimdall @jjtny1