Skip to content
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.

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

Improve materialize_batch_and_maybe_update_state #1381

Merged
merged 1 commit into from
Dec 8, 2024

Conversation

haze518
Copy link
Collaborator

@haze518 haze518 commented Dec 8, 2024

Refactor materialize_batch_and_maybe_update_state to use drain for processing message batches, eliminating unnecessary copies of self.messages and enhancing performance.

Iggy bench results for messages_required_to_save = 1(via cargo run --release --bin iggy-bench -- send tcp)

new function:

2024-12-07T15:31:12.909732Z  INFO iggy_bench::benchmark_runner: Results: Total throughput: 971.09 MB/s, 948330 messages/s, average throughput: 97.11 MB/s, average p50 latency: 9.00 ms, average p90 latency: 17.96 ms, average p95 latency: 22.11 ms, average p99 latency: 31.07 ms, average p999 latency: 74.38 ms, average latency: 10.55 ms, median latency: 9.00 ms

old:

2024-12-07T15:24:44.084381Z  INFO iggy_bench::benchmark_runner: Results: Total throughput: 129.75 MB/s, 126708 messages/s, average throughput: 12.97 MB/s, average p50 latency: 35.13 ms, average p90 latency: 104.77 ms, average p95 latency: 283.10 ms, average p99 latency: 1101.02 ms, average p999 latency: 2727.17 ms, average latency: 78.92 ms, median latency: 35.13 ms

messages_required_to_save=5000:

new

5000
2024-12-08T05:26:41.258931Z  INFO iggy_bench::benchmark_runner: Results: Total throughput: 1678.50 MB/s, 1639159 messages/s, average throughput: 167.85 MB/s, average p50 latency: 4.16 ms, average p90 latency: 11.71 ms, average p95 latency: 15.38 ms, average p99 latency: 25.87 ms, average p999 latency: 88.84 ms, average latency: 6.10 ms, median latency: 4.16 ms

old:

2024-12-08T05:34:17.895922Z  INFO iggy_bench::benchmark_runner: Results: Total throughput: 1591.14 MB/s, 1553850 messages/s, average throughput: 159.11 MB/s, average p50 latency: 4.47 ms, average p90 latency: 13.01 ms, average p95 latency: 17.10 ms, average p99 latency: 28.18 ms, average p999 latency: 44.04 ms, average latency: 6.44 ms, median latency: 4.47 ms

The new function significantly improves performance for low values of messages_required_to_save, especially in throughput and latency. For higher values, the performance gap narrows

@haze518 haze518 marked this pull request as draft December 8, 2024 04:36
@haze518 haze518 force-pushed the improve-materialize_batch_and_maybe_update_state branch from 3db0a52 to 7c6e336 Compare December 8, 2024 05:11
@haze518 haze518 changed the title test Improve materialize_batch_and_maybe_update_state Dec 8, 2024
@haze518 haze518 force-pushed the improve-materialize_batch_and_maybe_update_state branch from 7c6e336 to d004066 Compare December 8, 2024 05:16
@haze518 haze518 requested review from numinnex, spetz and hubcio December 8, 2024 05:37
@haze518 haze518 marked this pull request as ready for review December 8, 2024 06:59
@iggy-rs iggy-rs deleted a comment from coveralls Dec 8, 2024
@numinnex
Copy link
Contributor

numinnex commented Dec 8, 2024

Good find! Approved.

@numinnex
Copy link
Contributor

numinnex commented Dec 8, 2024

@haze518 bump the server in the Cargo.toml before merging.

Copy link
Contributor

@numinnex numinnex left a comment

Choose a reason for hiding this comment

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

Could you also run the benchmark for the default value for messages_required_to_save

Refactor materialize_batch_and_maybe_update_state to use drain for processing message batches, eliminating unnecessary copies of self.messages and enhancing performance
@haze518 haze518 force-pushed the improve-materialize_batch_and_maybe_update_state branch from d004066 to fc67b2a Compare December 8, 2024 08:33
@spetz spetz merged commit 20a6d14 into master Dec 8, 2024
17 checks passed
@spetz spetz deleted the improve-materialize_batch_and_maybe_update_state branch December 8, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants