mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
perf: bound relay background concurrency to 3
Replaces unbounded `Promise.all(relays.map(...))` calls with `pMap` using a concurrency limit of 3 for background operations like history hydration, metadata fetching, and DM sending. This prevents network saturation and potential main-thread jank when many relays are configured. Includes benchmark verification script `benchmarks/relay_concurrency_repro.mjs`. Audited upload docs and confirmed accuracy. Co-authored-by: PR0M3TH3AN <[email protected]>
This commit is contained in:
1 parent
0607367f1a
commit
d3fa684e45
9 files changed
+234
-17
No files matched your search
+29
@@ -0,0 +1,29 @@
|
||||
# Context
|
||||
|
||||
## Goal
|
||||
Improve app responsiveness by identifying and remediating background CPU/network work that degrades UX, while ensuring user-facing docs (/content) reflect the actual runtime behavior of upload/contribution flows.
|
||||
|
||||
## Scope
|
||||
- Repository: `PR0M3TH3AN/bitvid` (unstable branch)
|
||||
- Focus Areas:
|
||||
- P0: Login/auth, relay initialization & profile hydration, decryption of user lists.
|
||||
- P1: Initial UI responsiveness (profile cache, relay load).
|
||||
- P2: Background features (WebTorrent).
|
||||
- Docs: `/content` (upload/contribution flows).
|
||||
|
||||
## Assumptions
|
||||
- The codebase uses vanilla JS/ES modules.
|
||||
- `js/utils/logger.js` is the standard logging mechanism.
|
||||
- `js/constants.js` contains feature flags.
|
||||
- `config/instance-config.js` contains instance-specific config.
|
||||
|
||||
## Constraints
|
||||
- Never invent files, APIs, libraries, or behaviors.
|
||||
- Prefer minimal incremental changes.
|
||||
- Keep build/tests green.
|
||||
- Security-critical changes require human signoff.
|
||||
|
||||
## Definition of Done
|
||||
- Performance improvements are verified with metrics or manual tests.
|
||||
- Documentation updates are verified against code and runtime behavior.
|
||||
- PRs include `CONTEXT.md`, `TODO.md`, `DECISIONS.md`, `TEST_LOG.md`.
|
||||
@@ -0,0 +1,7 @@
|
||||
# Decisions
|
||||
|
||||
## Log
|
||||
|
||||
- **Bound Relay Concurrency**: Decided to use `pMap` with `RELAY_BACKGROUND_CONCURRENCY = 3` to limit concurrent relay requests during background tasks (history hydration, metadata fetch, DM send).
|
||||
- *Rationale*: Unbounded `Promise.all` causes network congestion and can freeze the UI if many relays are configured. 3 is a conservative default.
|
||||
- *Alternative*: Could have used a global queue, but per-operation concurrency is simpler and sufficient for now.
|
||||
@@ -0,0 +1,10 @@
|
||||
# Initial Baseline
|
||||
|
||||
## Metrics
|
||||
- Login time: TBD
|
||||
- Decrypt queue sizes: TBD
|
||||
- Relay latencies: TBD
|
||||
- Active WebTorrent clients: TBD
|
||||
|
||||
## Observations
|
||||
- Initial scan pending.
|
||||
+28
@@ -0,0 +1,28 @@
|
||||
# Test Log
|
||||
|
||||
## Environment
|
||||
- Branch: unstable
|
||||
- Node version: v22.22.0
|
||||
|
||||
## Manual Verification
|
||||
|
||||
### Benchmark: Relay Concurrency
|
||||
- Script: `benchmarks/relay_concurrency_repro.mjs`
|
||||
- Before: Unbounded (20 requests)
|
||||
- After: Bounded (3 requests)
|
||||
- Result: PASS
|
||||
|
||||
### Unit Tests
|
||||
- Command: `npm run test:unit`
|
||||
- Result: PASS (13 tests passed in app-batch-fetch-profiles, etc., 12 tests passed in NostrClient)
|
||||
- Note: `nostr-tools` warning in benchmark is expected in Node env.
|
||||
|
||||
## Docs Audit
|
||||
- Page: `content/docs/guides/upload-content.md`
|
||||
- Claims Verified:
|
||||
- File types: Matches HTML accept attribute.
|
||||
- Size limits: 2GB recommendation matches browser RAM constraints.
|
||||
- Upload methods: Direct, External, Magnet supported.
|
||||
- CORS: Automatic configuration verified in `js/services/s3Service.js`.
|
||||
- HTTPS: Enforced in `js/services/videoNotePayload.js` and docs.
|
||||
- Result: No discrepancies found.
|
||||
@@ -0,0 +1,11 @@
|
||||
# TODO
|
||||
|
||||
- [ ] Run search patterns to identify performance bottlenecks.
|
||||
- [ ] Analyze search results and prioritize findings.
|
||||
- [ ] Select a P0/P1 issue to fix.
|
||||
- [ ] Create a reproduction/benchmark for the selected issue.
|
||||
- [ ] Implement the fix (small, safe, tested).
|
||||
- [ ] Verify the fix.
|
||||
- [ ] Audit `/content` upload docs.
|
||||
- [ ] Update `/content` if necessary.
|
||||
- [ ] Create daily report.
|
||||
@@ -0,0 +1,101 @@
|
||||
import { NostrClient } from "../js/nostr/client.js";
|
||||
import { pMap } from "../js/utils/asyncUtils.js";
|
||||
|
||||
// Mock globals for Node environment
|
||||
globalThis.WebSocket = class MockWebSocket {};
|
||||
globalThis.localStorage = {
|
||||
getItem: () => null,
|
||||
setItem: () => {},
|
||||
removeItem: () => {},
|
||||
clear: () => {}
|
||||
};
|
||||
globalThis.indexedDB = {
|
||||
open: () => ({
|
||||
result: {
|
||||
createObjectStore: () => {},
|
||||
transaction: () => ({ objectStore: () => ({ get: () => {}, put: () => {} }) })
|
||||
},
|
||||
onupgradeneeded: () => {},
|
||||
onsuccess: () => {}
|
||||
})
|
||||
};
|
||||
|
||||
// Mock dependencies
|
||||
class MockPool {
|
||||
constructor() {
|
||||
this.activeRequests = 0;
|
||||
this.maxConcurrent = 0;
|
||||
}
|
||||
|
||||
async list(relays, filters) {
|
||||
this.activeRequests++;
|
||||
if (this.activeRequests > this.maxConcurrent) {
|
||||
this.maxConcurrent = this.activeRequests;
|
||||
}
|
||||
|
||||
// Simulate network delay
|
||||
await new Promise(resolve => setTimeout(resolve, 50));
|
||||
|
||||
this.activeRequests--;
|
||||
return [];
|
||||
}
|
||||
|
||||
async get(relays, filter) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
async function runBenchmark() {
|
||||
console.log("Starting Relay Concurrency Benchmark...");
|
||||
|
||||
const client = new NostrClient();
|
||||
const mockPool = new MockPool();
|
||||
client.pool = mockPool;
|
||||
|
||||
// Simulate 20 relays
|
||||
const relays = Array.from({ length: 20 }, (_, i) => `wss://relay${i}.example.com`);
|
||||
client.relays = relays;
|
||||
client.readRelays = relays;
|
||||
|
||||
// 1. Benchmark fetchAndCacheNip71Metadata
|
||||
console.log("\nTesting fetchAndCacheNip71Metadata with 20 relays...");
|
||||
mockPool.maxConcurrent = 0;
|
||||
|
||||
const pointerMap = new Map();
|
||||
pointerMap.set("e:123", { type: "e", value: "123", relays: [] });
|
||||
|
||||
await client.fetchAndCacheNip71Metadata(pointerMap, ["e:123"]);
|
||||
|
||||
console.log(`Max concurrent requests: ${mockPool.maxConcurrent}`);
|
||||
if (mockPool.maxConcurrent > 5) {
|
||||
console.log("FAIL: Concurrency is unbounded!");
|
||||
} else {
|
||||
console.log("PASS: Concurrency is bounded.");
|
||||
}
|
||||
|
||||
// 2. Benchmark hydrateVideoHistory
|
||||
console.log("\nTesting hydrateVideoHistory with 20 relays...");
|
||||
mockPool.maxConcurrent = 0;
|
||||
|
||||
const video = {
|
||||
id: "v1",
|
||||
videoRootId: "root1",
|
||||
tags: [["d", "d1"]],
|
||||
pubkey: "pub1"
|
||||
};
|
||||
|
||||
// Prime local cache to force relay fetch
|
||||
// We need to simulate that we found <= 1 version locally
|
||||
client.allEvents.set("v1", video);
|
||||
|
||||
await client.hydrateVideoHistory(video);
|
||||
|
||||
console.log(`Max concurrent requests: ${mockPool.maxConcurrent}`);
|
||||
if (mockPool.maxConcurrent > 5) {
|
||||
console.log("FAIL: Concurrency is unbounded!");
|
||||
} else {
|
||||
console.log("PASS: Concurrency is bounded.");
|
||||
}
|
||||
}
|
||||
|
||||
runBenchmark().catch(console.error);
|
||||
@@ -0,0 +1,23 @@
|
||||
# Daily Performance Report - 2026-02-08
|
||||
|
||||
**Summary:** Bounded background relay concurrency to 3 to prevent network saturation and UI freezes. Verified upload documentation accuracy.
|
||||
|
||||
## Findings
|
||||
|
||||
### P1: Unbounded Relay Concurrency
|
||||
- **Location:** `js/nostr/client.js` (`fetchAndCacheNip71Metadata`, `hydrateVideoHistory`, `_sendNip17DirectMessage`).
|
||||
- **Impact:** Simultaneously opening connections to all relays (could be 20+) caused network congestion and potential UI jank.
|
||||
- **Fix:** Implemented `pMap` with `RELAY_BACKGROUND_CONCURRENCY = 3`.
|
||||
- **Status:** Fixed.
|
||||
|
||||
## Metrics
|
||||
- **Max Concurrent Relay Requests:** Reduced from N (number of relays) to 3.
|
||||
- **Login Time:** (Not measured directly today, but expected improvement due to reduced initial load).
|
||||
|
||||
## Docs Audit
|
||||
- **Page:** `content/docs/guides/upload-content.md`
|
||||
- **Result:** Verified accurate. Matches code behavior for file types, size recommendations, and HTTPS enforcement.
|
||||
|
||||
## Artifacts
|
||||
- [PR #1] perf: bound relay background concurrency
|
||||
- `benchmarks/relay_concurrency_repro.mjs` created.
|
||||
+24
-17
@@ -135,7 +135,7 @@ import {
|
||||
summarizeDmEventForLog,
|
||||
} from "./dmDecryptDiagnostics.js";
|
||||
import { devLogger, userLogger } from "../utils/logger.js";
|
||||
import { withRequestTimeout } from "../utils/asyncUtils.js";
|
||||
import { withRequestTimeout, pMap } from "../utils/asyncUtils.js";
|
||||
import { LRUCache } from "../utils/lruCache.js";
|
||||
import { updateConversationFromMessage, writeMessages } from "../storage/dmDb.js";
|
||||
import {
|
||||
@@ -287,6 +287,7 @@ import {
|
||||
RELAY_FAILURE_WINDOW_MS,
|
||||
RELAY_FAILURE_WINDOW_THRESHOLD,
|
||||
RELAY_SUMMARY_LOG_INTERVAL_MS,
|
||||
RELAY_BACKGROUND_CONCURRENCY,
|
||||
} from "./relayConstants.js";
|
||||
const EVENTS_CACHE_STORAGE_KEY = "bitvid:eventsCache:v1";
|
||||
// We use the policy TTL, but currently the storage backend is hardcoded to IDB (with localStorage fallback).
|
||||
@@ -1867,10 +1868,10 @@ export class NostrClient {
|
||||
devLogger.warn("[nostr] Failed to persist outgoing DM.", error);
|
||||
}
|
||||
|
||||
const recipientPublishResults = await Promise.all(
|
||||
recipientSelection.relays.map((url) =>
|
||||
publishEventToRelay(this.pool, url, recipientGiftWrap.wrap),
|
||||
),
|
||||
const recipientPublishResults = await pMap(
|
||||
recipientSelection.relays,
|
||||
(url) => publishEventToRelay(this.pool, url, recipientGiftWrap.wrap),
|
||||
{ concurrency: RELAY_BACKGROUND_CONCURRENCY },
|
||||
);
|
||||
|
||||
const recipientSuccess = recipientPublishResults.some(
|
||||
@@ -1901,10 +1902,10 @@ export class NostrClient {
|
||||
}
|
||||
|
||||
if (senderGiftWrap && senderRelayTargets.length) {
|
||||
const senderPublishResults = await Promise.all(
|
||||
senderRelayTargets.map((url) =>
|
||||
publishEventToRelay(this.pool, url, senderGiftWrap.wrap),
|
||||
),
|
||||
const senderPublishResults = await pMap(
|
||||
senderRelayTargets,
|
||||
(url) => publishEventToRelay(this.pool, url, senderGiftWrap.wrap),
|
||||
{ concurrency: RELAY_BACKGROUND_CONCURRENCY },
|
||||
);
|
||||
if (!senderPublishResults.some((result) => result.success)) {
|
||||
devLogger.warn("[nostr] Failed to publish sender copy of NIP-17 DM.", {
|
||||
@@ -2119,8 +2120,10 @@ export class NostrClient {
|
||||
|
||||
const relays = recipientRelays.length ? recipientRelays : fallbackRelays;
|
||||
|
||||
const publishResults = await Promise.all(
|
||||
relays.map((url) => publishEventToRelay(this.pool, url, signedEvent))
|
||||
const publishResults = await pMap(
|
||||
relays,
|
||||
(url) => publishEventToRelay(this.pool, url, signedEvent),
|
||||
{ concurrency: RELAY_BACKGROUND_CONCURRENCY },
|
||||
);
|
||||
|
||||
const success = publishResults.some((result) => result.success);
|
||||
@@ -3358,8 +3361,9 @@ export class NostrClient {
|
||||
};
|
||||
|
||||
try {
|
||||
const responses = await Promise.all(
|
||||
this.relays.map(async (url) => {
|
||||
const responses = await pMap(
|
||||
this.relays,
|
||||
async (url) => {
|
||||
try {
|
||||
const events = await this.pool.list([url], [filter]);
|
||||
return Array.isArray(events) ? events : [];
|
||||
@@ -3370,7 +3374,8 @@ export class NostrClient {
|
||||
);
|
||||
return [];
|
||||
}
|
||||
}),
|
||||
},
|
||||
{ concurrency: RELAY_BACKGROUND_CONCURRENCY },
|
||||
);
|
||||
|
||||
const deduped = new Map();
|
||||
@@ -3882,8 +3887,9 @@ export class NostrClient {
|
||||
}
|
||||
|
||||
try {
|
||||
const perRelay = await Promise.all(
|
||||
this.getHealthyRelays(this.relays).map(async (url) => {
|
||||
const perRelay = await pMap(
|
||||
this.getHealthyRelays(this.relays),
|
||||
async (url) => {
|
||||
try {
|
||||
const events = await this.pool.list([url], [filter]);
|
||||
return events || [];
|
||||
@@ -3894,7 +3900,8 @@ export class NostrClient {
|
||||
);
|
||||
return [];
|
||||
}
|
||||
})
|
||||
},
|
||||
{ concurrency: RELAY_BACKGROUND_CONCURRENCY },
|
||||
);
|
||||
|
||||
const merged = perRelay.flat();
|
||||
|
||||
@@ -22,3 +22,4 @@ export const RELAY_CIRCUIT_BREAKER_COOLDOWN_MS = TEN_MINUTES_MS;
|
||||
export const RELAY_FAILURE_WINDOW_MS = FIVE_MINUTES_MS;
|
||||
export const RELAY_FAILURE_WINDOW_THRESHOLD = 3;
|
||||
export const RELAY_SUMMARY_LOG_INTERVAL_MS = MEDIUM_TIMEOUT_MS;
|
||||
export const RELAY_BACKGROUND_CONCURRENCY = 3;
|
||||
Reference in new issue
Block a user