From 2b061460eb9bc66933c527ad6d1a390bde3ac604 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 13 Feb 2026 22:09:30 +0000 Subject: [PATCH] perf: limit reaction event concurrency Weekly perf deep dive: - Replaced unbounded `Promise.all` in `js/nostr/reactionEvents.js` with `pMap` limited to `RELAY_BACKGROUND_CONCURRENCY` (3). - Added `benchmarks/reaction_loading_bench.mjs` harness to verify concurrency limits. - Added weekly perf report in `docs/perf-reports/weekly-perf-report-2026-02-13.md`. - Added completion log. Co-authored-by: PR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> --- benchmarks/reaction_loading_bench.mjs | 50 +++++++++++++++++++ ..._20-00-00_perf-deepdive-agent_completed.md | 22 ++++++++ .../weekly-perf-report-2026-02-13.md | 50 +++++++++++++++++++ js/nostr/reactionEvents.js | 18 ++++--- 4 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 benchmarks/reaction_loading_bench.mjs create mode 100644 docs/agents/task-logs/weekly/2026-02-13_20-00-00_perf-deepdive-agent_completed.md create mode 100644 docs/perf-reports/weekly-perf-report-2026-02-13.md diff --git a/benchmarks/reaction_loading_bench.mjs b/benchmarks/reaction_loading_bench.mjs new file mode 100644 index 00000000..1287086c --- /dev/null +++ b/benchmarks/reaction_loading_bench.mjs @@ -0,0 +1,50 @@ + +import { listVideoReactions } from '../js/nostr/reactionEvents.js'; +import { RELAY_BACKGROUND_CONCURRENCY } from '../js/nostr/relayConstants.js'; + +const NUM_RELAYS = 20; +const DELAY_MS = 100; + +const mockRelays = Array.from({ length: NUM_RELAYS }, (_, i) => `wss://mock-relay-${i}.com`); + +let activeRequests = 0; +let peakRequests = 0; + +const mockPool = { + list: async (relays, filters) => { + activeRequests++; + if (activeRequests > peakRequests) { + peakRequests = activeRequests; + } + + // Simulate network delay + await new Promise(resolve => setTimeout(resolve, DELAY_MS)); + + activeRequests--; + return []; // Return empty events for now + } +}; + +const mockClient = { + pool: mockPool, + relays: mockRelays, // Allow these relays + ensurePool: async () => mockPool +}; + +async function runBenchmark() { + console.log(`Starting benchmark with ${NUM_RELAYS} relays...`); + console.log(`Expected concurrency limit (after fix): ${RELAY_BACKGROUND_CONCURRENCY}`); + + const start = performance.now(); + + await listVideoReactions(mockClient, { id: 'test-video-id' }, { relays: mockRelays }); + + const end = performance.now(); + + console.log('---------------------------------------------------'); + console.log(`Total time: ${(end - start).toFixed(2)}ms`); + console.log(`Peak concurrent requests: ${peakRequests}`); + console.log('---------------------------------------------------'); +} + +runBenchmark().catch(console.error); diff --git a/docs/agents/task-logs/weekly/2026-02-13_20-00-00_perf-deepdive-agent_completed.md b/docs/agents/task-logs/weekly/2026-02-13_20-00-00_perf-deepdive-agent_completed.md new file mode 100644 index 00000000..6fd69025 --- /dev/null +++ b/docs/agents/task-logs/weekly/2026-02-13_20-00-00_perf-deepdive-agent_completed.md @@ -0,0 +1,22 @@ +# Weekly Agent Task Log: perf-deepdive-agent + +- **Date:** 2026-02-13 +- **Agent:** perf-deepdive-agent +- **Status:** Completed +- **Outcome:** Success + +## Summary + +The `perf-deepdive-agent` identified a concurrency issue in `js/nostr/reactionEvents.js` where fetching and publishing reactions was unbounded, risking network saturation. This was fixed by introducing `pMap` with a concurrency limit of 3 (`RELAY_BACKGROUND_CONCURRENCY`). + +A new benchmark harness `benchmarks/reaction_loading_bench.mjs` was added to verify the fix and prevent regression. + +## Key Changes + +1. **Refactor**: `js/nostr/reactionEvents.js` now uses `pMap` instead of `Promise.all` for relay operations. +2. **Report**: `docs/perf-reports/weekly-perf-report-2026-02-13.md` documents the findings and measurements. +3. **Harness**: `benchmarks/reaction_loading_bench.mjs` simulates load against mock relays. + +## Next Steps + +- Similar optimizations should be applied to `js/nostr/viewEvents.js` and `js/nostr/dmSignalEvents.js`. diff --git a/docs/perf-reports/weekly-perf-report-2026-02-13.md b/docs/perf-reports/weekly-perf-report-2026-02-13.md new file mode 100644 index 00000000..d6710f52 --- /dev/null +++ b/docs/perf-reports/weekly-perf-report-2026-02-13.md @@ -0,0 +1,50 @@ +# Weekly Performance Report — 2026-02-13 + +**Agent:** `bitvid-perf-deepdive-agent` +**Focus:** Relay Request Concurrency + +## Executive Summary + +Identified a critical performance risk in `js/nostr/reactionEvents.js` where reaction list and publish operations were using unbounded `Promise.all` across all configured relays. This could lead to network saturation, browser connection limits (blocking other requests), and potential timeouts for users with many relays. + +**Optimization:** Replaced `Promise.all` with `pMap` to enforce a concurrency limit of 3 (`RELAY_BACKGROUND_CONCURRENCY`), aligning with best practices established in `commentEvents.js`. + +## Methodology + +### Scenario +Simulate `listVideoReactions` call with a client configured for 20 relays. +- **Tools:** `benchmarks/reaction_loading_bench.mjs` (new harness) +- **Environment:** Node.js 22 (Agent Environment) +- **Metric:** Peak concurrent `pool.list()` calls. + +### Baseline Measurements + +| Metric | Value | Notes | +| :--- | :--- | :--- | +| Peak Concurrency | **20** | Unbounded; fired all requests simultaneously. | +| Total Time (Mock) | ~111ms | Limited only by the longest single request (mock delay 100ms). | + +### After Optimization + +| Metric | Value | Notes | +| :--- | :--- | :--- | +| Peak Concurrency | **3** | Capped by `RELAY_BACKGROUND_CONCURRENCY`. | +| Total Time (Mock) | ~705ms | Increased latency is expected and acceptable for background operations to preserve network health. | + +## Changes + +### 1. Concurrency Limiting in `js/nostr/reactionEvents.js` +- **Before:** `await Promise.all(relayList.map(...))` +- **After:** `await pMap(relayList, ..., { concurrency: 3 })` +- **Impact:** Prevents "thundering herd" of WebSocket requests. + +### 2. New Benchmark Harness +- Added `benchmarks/reaction_loading_bench.mjs` to reproducible measure concurrency of reaction loading. + +## Verification +- Unit tests (`tests/nostr/reaction-events.test.mjs`) passed. +- Benchmark confirmed concurrency cap is active. + +## Next Steps +- **Audit `js/nostr/viewEvents.js`**: Similar unbounded `Promise.all` pattern exists there and should be patched in a future run. +- **Audit `js/nostr/dmSignalEvents.js`**: Another potential candidate for concurrency limiting. diff --git a/js/nostr/reactionEvents.js b/js/nostr/reactionEvents.js index 3311f9b3..1b222af8 100644 --- a/js/nostr/reactionEvents.js +++ b/js/nostr/reactionEvents.js @@ -8,6 +8,8 @@ import { CACHE_POLICIES } from "./cachePolicies.js"; import { NOTE_TYPES } from "../nostrEventSchemas.js"; import { queueSignEvent } from "./signRequestQueue.js"; import { sanitizeRelayList as sanitizeRelayUrls } from "./nip46Client.js"; +import { pMap } from "../utils/asyncUtils.js"; +import { RELAY_BACKGROUND_CONCURRENCY } from "./relayConstants.js"; const CACHE_POLICY = CACHE_POLICIES[NOTE_TYPES.VIDEO_REACTION]; const reactionCache = new LRUCache({ maxSize: 100 }); @@ -205,12 +207,13 @@ export async function listVideoReactions(client, targetInput, options = {}) { const filters = [{ kinds: [7], "#e": [videoEventId] }]; // Parallel fetch from relays with incremental logic - await Promise.all( - relayList.map(async (url) => { + await pMap( + relayList, + async (url) => { if (!url) return; const lastSeen = lastSeens[url] || 0; // Deep clone filters to apply 'since' safely per relay - const relayFilters = filters.map(f => { + const relayFilters = filters.map((f) => { const copy = { ...f }; if (lastSeen > 0) { copy.since = lastSeen + 1; @@ -235,7 +238,8 @@ export async function listVideoReactions(client, targetInput, options = {}) { } catch (err) { devLogger.warn(`[nostr] Failed to fetch reactions from ${url}:`, err); } - }) + }, + { concurrency: RELAY_BACKGROUND_CONCURRENCY }, ); // Merge with cached items @@ -481,8 +485,10 @@ export async function publishVideoReaction( const relayList = sanitizeRelayList(options.relays, client.relays); - const publishResults = await Promise.all( - relayList.map((url) => publishEventToRelay(client.pool, url, signedEvent)) + const publishResults = await pMap( + relayList, + (url) => publishEventToRelay(client.pool, url, signedEvent), + { concurrency: RELAY_BACKGROUND_CONCURRENCY }, ); const acceptedRelays = publishResults