mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
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 <[email protected]>
This commit is contained in:
1 parent
c1d26d0466
commit
2b061460eb
4 files changed
+134
-6
No files matched your search
@@ -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);
|
||||
@@ -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`.
|
||||
@@ -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.
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user