-
-
Notifications
You must be signed in to change notification settings - Fork 126
perf: integration benchmarks + allocation optimizations (-23% total, -43% connect allocs) #551
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
Changes from all commits
62ca987
8efa7a8
f46417e
da9bf0c
a1f68b9
d692743
5e20548
140a487
e801322
79a282c
9e1adbc
f47371a
1563f49
2ba5f6e
8c599aa
c1abd3d
36224a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| name: Integration Benchmark | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main] | ||
| push: | ||
| branches: [main] | ||
|
|
||
| concurrency: | ||
| group: bench-integration-${{ github.head_ref || github.run_id }} | ||
| cancel-in-progress: true | ||
|
|
||
| env: | ||
| CARGO_TERM_COLOR: always | ||
| PROTOC_VERSION: "3.25.3" | ||
| SCCACHE_GHA_ENABLED: "true" | ||
| RUSTC_WRAPPER: "sccache" | ||
|
|
||
| jobs: | ||
| bench-integration-pr: | ||
| if: github.event_name == 'pull_request' | ||
| name: Integration Benchmark (PR) | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| services: | ||
| mock-server: | ||
| image: ghcr.io/whiskeysockets-devtools/bartender:latest | ||
| credentials: | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.BARTENDER_GHCR_TOKEN }} | ||
| ports: | ||
| - 8080:8080 | ||
| env: | ||
| CHATSTATE_TTL_SECS: "3" | ||
| options: --log-driver none | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - uses: dtolnay/rust-toolchain@master | ||
| with: | ||
| toolchain: nightly-2026-04-05 | ||
|
|
||
|
Comment on lines
+28
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n .github/workflows/bench-integration.yml | sed -n '1,60p'Repository: jlucaso1/whatsapp-rust Length of output: 1873 🏁 Script executed: # Search for any version tags or references to bartender in the repo
rg -i "bartender" --type yaml --type mdRepository: jlucaso1/whatsapp-rust Length of output: 48 🌐 Web query:
💡 Result: Best practices for pinning the dtolnay/rust-toolchain GitHub Action: Use specific tags or commit SHAs for reproducibility and security. The action's Citations:
🌐 Web query:
💡 Result: No official whiskeysockets/bartender Docker image or releases found. WhiskeySockets maintains Baileys (WhatsApp Web API library) with latest release v7.0.0-rc.9 on 2025-11-21. "Bartender" appears in Baileys PR Citations:
Pin the benchmark environment to immutable refs.
Proposed pinning pattern- image: ghcr.io/whiskeysockets-devtools/bartender:latest
+ image: ghcr.io/whiskeysockets-devtools/bartender@sha256:<verified-digest>
...
- - uses: dtolnay/rust-toolchain@master
+ - uses: dtolnay/rust-toolchain@<verified-commit-sha>🤖 Prompt for AI Agents |
||
| - name: Install protoc | ||
| uses: taiki-e/install-action@v2 | ||
| with: | ||
| tool: protoc@${{ env.PROTOC_VERSION }} | ||
|
|
||
| - name: Setup sccache | ||
| uses: mozilla-actions/sccache-action@v0.0.9 | ||
|
|
||
| - name: Cache Rust registry | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| prefix-key: ${{ runner.os }}-cargo-bench-int | ||
| cache-targets: "false" | ||
|
|
||
| - name: Wait for mock server | ||
| run: | | ||
| for i in $(seq 1 30); do | ||
| if curl -sk https://localhost:8080/ > /dev/null 2>&1; then | ||
| echo "Mock server is ready" | ||
| exit 0 | ||
| fi | ||
| sleep 1 | ||
| done | ||
| echo "Mock server failed to become ready" | ||
| exit 1 | ||
|
|
||
| - name: Run integration benchmarks | ||
| env: | ||
| MOCK_SERVER_URL: "wss://127.0.0.1:8080/ws/chat" | ||
| RUST_LOG: warn | ||
| run: | | ||
| cargo run -p bench-integration --release \ | ||
| > bench_integration_results.json \ | ||
| 2> bench_integration.log | ||
| cat bench_integration.log >&2 | ||
| echo "Results:" | ||
| cat bench_integration_results.json | ||
|
|
||
| - name: Upload benchmark artifacts | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: bench-integration-results | ||
| path: | | ||
| bench_integration_results.json | ||
| bench_integration.log | ||
| retention-days: 30 | ||
|
|
||
| - name: Compare against baseline | ||
| uses: benchmark-action/github-action-benchmark@v1 | ||
| with: | ||
| name: "whatsapp-rust integration benchmarks" | ||
| tool: "customSmallerIsBetter" | ||
| output-file-path: bench_integration_results.json | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| auto-push: false | ||
| save-data-file: false | ||
| benchmark-data-dir-path: dev/bench-integration | ||
| summary-always: true | ||
|
|
||
| bench-integration-push: | ||
| if: github.event_name == 'push' | ||
| name: Integration Benchmark (push) | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: write | ||
| deployments: write | ||
| services: | ||
| mock-server: | ||
| image: ghcr.io/whiskeysockets-devtools/bartender:latest | ||
| credentials: | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.BARTENDER_GHCR_TOKEN }} | ||
| ports: | ||
| - 8080:8080 | ||
| env: | ||
| CHATSTATE_TTL_SECS: "3" | ||
| options: --log-driver none | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - uses: dtolnay/rust-toolchain@master | ||
| with: | ||
| toolchain: nightly-2026-04-05 | ||
|
|
||
| - name: Install protoc | ||
| uses: taiki-e/install-action@v2 | ||
| with: | ||
| tool: protoc@${{ env.PROTOC_VERSION }} | ||
|
|
||
| - name: Setup sccache | ||
| uses: mozilla-actions/sccache-action@v0.0.9 | ||
|
|
||
| - name: Cache Rust registry | ||
| uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| prefix-key: ${{ runner.os }}-cargo-bench-int | ||
| cache-targets: "false" | ||
|
|
||
| - name: Wait for mock server | ||
| run: | | ||
| for i in $(seq 1 30); do | ||
| if curl -sk https://localhost:8080/ > /dev/null 2>&1; then | ||
| echo "Mock server is ready" | ||
| exit 0 | ||
| fi | ||
| sleep 1 | ||
| done | ||
| echo "Mock server failed to become ready" | ||
| exit 1 | ||
|
|
||
| - name: Run integration benchmarks | ||
| env: | ||
| MOCK_SERVER_URL: "wss://127.0.0.1:8080/ws/chat" | ||
| RUST_LOG: warn | ||
| run: | | ||
| cargo run -p bench-integration --release \ | ||
| > bench_integration_results.json \ | ||
| 2> bench_integration.log | ||
| cat bench_integration.log >&2 | ||
| echo "Results:" | ||
| cat bench_integration_results.json | ||
|
|
||
| - name: Upload benchmark artifacts | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: bench-integration-results | ||
| path: | | ||
| bench_integration_results.json | ||
| bench_integration.log | ||
| retention-days: 30 | ||
|
|
||
| - name: Store baseline | ||
| uses: benchmark-action/github-action-benchmark@v1 | ||
| with: | ||
| name: "whatsapp-rust integration benchmarks" | ||
| tool: "customSmallerIsBetter" | ||
| output-file-path: bench_integration_results.json | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| auto-push: true | ||
| benchmark-data-dir-path: dev/bench-integration | ||
| max-items-in-chart: 100 | ||
| summary-always: true | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,3 +10,4 @@ docs | |
| .claude | ||
| __pycache__ | ||
| .codex | ||
| dhat-heap.json | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,51 +38,11 @@ impl StanzaHandler for MessageHandler { | |
| } | ||
| }; | ||
|
|
||
| // Single cache lookup: get or create the lane (lock + queue + worker). | ||
| // Single-flight: get_with_by_ref guarantees exactly one init runs per key, | ||
| // preventing duplicate workers for the same chat (TOCTOU race). | ||
| let lane = client | ||
| .chat_lanes | ||
| .get_with_by_ref(&chat_jid, async { | ||
| let (tx, rx) = async_channel::unbounded::<Arc<wacore_binary::OwnedNodeRef>>(); | ||
|
|
||
| let client_for_worker = client.clone(); | ||
| let spawn_generation = client | ||
| .connection_generation | ||
| .load(std::sync::atomic::Ordering::Acquire); | ||
|
|
||
| client | ||
| .runtime | ||
| .spawn(Box::pin(async move { | ||
| while let Ok(msg_node) = rx.recv().await { | ||
| if client_for_worker | ||
| .connection_generation | ||
| .load(std::sync::atomic::Ordering::Acquire) | ||
| != spawn_generation | ||
| { | ||
| log::debug!(target: "MessageQueue", "Stale worker exiting; remaining messages will be redelivered by server"); | ||
| break; | ||
| } | ||
| let start = wacore::time::now_millis() as u64; | ||
| let client = client_for_worker.clone(); | ||
| Box::pin(client.handle_incoming_message(msg_node)).await; | ||
| let elapsed = | ||
| (wacore::time::now_millis() as u64).saturating_sub(start); | ||
| if elapsed > MAX_MESSAGE_DELAY_MS { | ||
| warn!( | ||
| target: "MessageQueue", | ||
| "Message processing took {:.1}s (MAX_MESSAGE_DELAY is {}s)", | ||
| elapsed as f64 / 1000.0, | ||
| MAX_MESSAGE_DELAY_MS / 1000 | ||
| ); | ||
| } | ||
| } | ||
| })) | ||
| .detach(); | ||
|
|
||
| ChatLane { | ||
| enqueue_lock: Arc::new(async_lock::Mutex::new(())), | ||
| queue_tx: tx, | ||
| } | ||
| }) | ||
| .get_with_by_ref(&chat_jid, async { create_chat_lane(&client) }) | ||
| .await; | ||
|
|
||
| // Lock serializes enqueue order for this chat | ||
|
|
@@ -97,3 +57,47 @@ impl StanzaHandler for MessageHandler { | |
| true | ||
| } | ||
| } | ||
|
|
||
| /// Construct a ChatLane with a spawned worker task. Extracted to keep the | ||
| /// init closure passed to `get_with_by_ref` small. | ||
| fn create_chat_lane(client: &Arc<Client>) -> ChatLane { | ||
| let (tx, rx) = async_channel::unbounded::<Arc<wacore_binary::OwnedNodeRef>>(); | ||
|
|
||
| let client_for_worker = client.clone(); | ||
| let spawn_generation = client | ||
| .connection_generation | ||
| .load(std::sync::atomic::Ordering::Acquire); | ||
|
|
||
| client | ||
| .runtime | ||
| .spawn(Box::pin(async move { | ||
| while let Ok(msg_node) = rx.recv().await { | ||
| if client_for_worker | ||
| .connection_generation | ||
| .load(std::sync::atomic::Ordering::Acquire) | ||
| != spawn_generation | ||
| { | ||
| log::debug!(target: "MessageQueue", "Stale worker exiting; remaining messages will be redelivered by server"); | ||
| break; | ||
| } | ||
| let start = wacore::time::now_millis() as u64; | ||
| let client = client_for_worker.clone(); | ||
| Box::pin(client.handle_incoming_message(msg_node)).await; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify handle_incoming_message signature and current call sites.
# Expectation: if it returns an awaitable future directly, Box::pin is unnecessary.
set -euo pipefail
rg -n -C3 '\bhandle_incoming_message\s*\(' --type rustRepository: jlucaso1/whatsapp-rust Length of output: 3552 Remove unnecessary
♻️ Suggested change- let client = client_for_worker.clone();
- Box::pin(client.handle_incoming_message(msg_node)).await;
+ let client = client_for_worker.clone();
+ client.handle_incoming_message(msg_node).await;🤖 Prompt for AI Agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Box::pin here is intentional -- handle_incoming_message takes self: Arc and returns a large async future. Without Box::pin, that future is inlined into the worker loop's state machine, making the outer spawned future ~29KB (the exact problem we were trying to fix). With Box::pin, the outer future stays small and the large message-handling future is heap-allocated only when actually processing a message. This is the opposite of unnecessary -- it's a deliberate optimization. Removing it would undo the future-size reduction we achieved. The Box::pin on line 73 (runtime.spawn(Box::pin(async move { ... }))) is required by the spawn API which takes Pin<Box>. No change needed here. The review suggestion is incorrect for this case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
✏️ Learnings added
🧠 Learnings used |
||
| let elapsed = (wacore::time::now_millis() as u64).saturating_sub(start); | ||
| if elapsed > MAX_MESSAGE_DELAY_MS { | ||
| warn!( | ||
| target: "MessageQueue", | ||
| "Message processing took {:.1}s (MAX_MESSAGE_DELAY is {}s)", | ||
| elapsed as f64 / 1000.0, | ||
| MAX_MESSAGE_DELAY_MS / 1000 | ||
| ); | ||
| } | ||
| } | ||
| })) | ||
| .detach(); | ||
|
|
||
| ChatLane { | ||
| enqueue_lock: Arc::new(async_lock::Mutex::new(())), | ||
| queue_tx: tx, | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workflow runs on
pull_request, but the PR benchmark job authenticates the service container withsecrets.BARTENDER_GHCR_TOKEN. For PRs from forks, repository secrets are not provided, so the mock-server service cannot authenticate/pull and the benchmark job fails before tests run. That makes the new benchmark CI path unreliable for external contributions.Useful? React with 👍 / 👎.