-
-
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 10 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,119 @@ | ||
| name: Integration Benchmark | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main] | ||
| push: | ||
| branches: [main] | ||
|
|
||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
| deployments: write | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| 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: | ||
| name: Integration Benchmark | ||
| runs-on: ubuntu-latest | ||
| 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 | ||
|
|
||
| # Push to main: store baseline | ||
| - name: Store baseline (push to main) | ||
| if: github.event_name == 'push' | ||
| 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 | ||
|
|
||
| # PR: compare against baseline | ||
| - name: Compare against baseline (pull request) | ||
| if: github.event_name == 'pull_request' | ||
| 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 | ||
| 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,52 +38,57 @@ impl StanzaHandler for MessageHandler { | |
| } | ||
| }; | ||
|
|
||
| // Single cache lookup: get or create the lane (lock + queue + worker). | ||
| let lane = client | ||
| .chat_lanes | ||
| .get_with_by_ref(&chat_jid, async { | ||
| let (tx, rx) = async_channel::unbounded::<Arc<wacore_binary::OwnedNodeRef>>(); | ||
| // Fast path: cache hit avoids building the init future entirely. | ||
| // This shrinks the handler's async state machine because the large | ||
| // worker-spawning closure isn't captured across the .await. | ||
| let lane = if let Some(existing) = client.chat_lanes.get(&chat_jid).await { | ||
| existing | ||
| } else { | ||
|
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.
This miss-path now does a separate Useful? React with 👍 / 👎. |
||
| 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); | ||
| 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 | ||
| ); | ||
| } | ||
| 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; | ||
| } | ||
| })) | ||
| .detach(); | ||
| 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, | ||
| } | ||
| }) | ||
| .await; | ||
| let lane = ChatLane { | ||
| enqueue_lock: Arc::new(async_lock::Mutex::new(())), | ||
| queue_tx: tx, | ||
| }; | ||
| client | ||
| .chat_lanes | ||
| .insert(chat_jid.clone(), lane.clone()) | ||
| .await; | ||
| lane | ||
| }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| // Lock serializes enqueue order for this chat | ||
| let _guard = lane.enqueue_lock.lock().await; | ||
|
|
||
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 👍 / 👎.