perf: enhance benchmark with Protocol V2 features by danielsimonjr · Pull Request #52 · danielsimonjr/workerpool

danielsimonjr · 2025-12-22T05:34:46Z

No description provided.

Add comprehensive TypeScript examples demonstrating all Protocol V2 messaging features: - errorCodes.ts: Standardized error codes with categories - protocolVersioning.ts: Protocol version validation and message headers - messagePriority.ts: Priority-based message handling and queuing - heartbeat.ts: Worker health monitoring with HeartbeatMonitor - binaryProtocol.ts: Binary message encoding/decoding - protocolV2Complete.ts: Combined example showing all features together Also export additional binary protocol utilities from full.ts.

Update benchmark.mjs to comprehensively test all new Protocol V2 features in TypeScript/WASM compared to original JavaScript: Part A: Core Pool Operations (JS vs TS+WASM vs AdvancedPool) - Pool creation, task execution, concurrent tasks, queue throughput Part B: Protocol V2 Features (TS+WASM Only) - Error codes: message lookup, classification, categories - Message priority: lookup, sorting, message creation - Heartbeat: request/response creation - Binary protocol: encode/decode, validate, roundtrip, checksum - Type guards: all guard functions performance Part C: Data Structures Performance - CircularBuffer (growable and fixed) - WorkStealingDeque - WorkerChoiceStrategyManager Part D: Advanced Pool Features - Worker choice strategy comparison - Work stealing impact - Task affinity performance - Optimized pool factory functions Results show TS+WASM is 1.49x faster than JS on both Node.js and Bun.

Copilot

Pull request overview

This PR enhances the benchmark suite to comprehensively test Protocol V2 features while maintaining existing pool operation benchmarks. The changes introduce systematic performance testing for the new protocol capabilities including error codes, message priorities, heartbeat mechanisms, and binary protocol operations.

Key Changes:

Expanded benchmark.mjs to test Protocol V2 features (error codes, priorities, heartbeat, binary protocol) alongside core pool operations
Added 6 comprehensive example files demonstrating Protocol V2 usage patterns
Modified exports in src/ts/full.ts to expose additional Protocol V2 APIs and binary protocol functions

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/ts/full.ts	Updated exports to expose Protocol V2 features; removed some export aliases while keeping others (inconsistent naming pattern)
benchmark.mjs	Significantly expanded with Protocol V2 benchmarks, added synchronous benchmark function, improved output formatting with detailed summaries
examples/typescript/protocolVersioning.ts	New comprehensive example demonstrating protocol versioning, message creation, validation, and backward compatibility
examples/typescript/protocolV2Complete.ts	New example showing all Protocol V2 features working together in a simulated worker pool scenario
examples/typescript/messagePriority.ts	New example demonstrating the message priority system with queue management and sorting
examples/typescript/heartbeat.ts	New example showing heartbeat mechanism for worker health monitoring
examples/typescript/errorCodes.ts	New example demonstrating standardized error code system with categorization and handling patterns
examples/typescript/binaryProtocol.ts	New example demonstrating binary protocol encoding/decoding with performance comparisons

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-22T05:38:39Z

src/ts/full.ts

+  getPayload,
+  getMessageType,
+  getMessageId,
  getMessagePriority as getBinaryMessagePriority,


The aliases getBinaryMessageType and getBinaryMessageId have been removed, but getBinaryMessagePriority is still kept as an alias on line 949. This is inconsistent. Either all three should retain their getBinary* aliases for backward compatibility, or all should be changed to remove the prefix. The inconsistency will confuse API consumers.

Suggested change

getMessagePriority as getBinaryMessagePriority,

getMessagePriority,

Copilot · 2025-12-22T05:38:40Z

benchmark.mjs

+/**
+ * Run a synchronous benchmark
+ */
+function benchmarkSync(name, fn, iterations = ITERATIONS) {
+  // Warmup
+  for (let i = 0; i < WARMUP; i++) {
+    fn();
+  }
+
+  const times = [];
+  for (let i = 0; i < iterations; i++) {
+    const start = performance.now();
+    fn();
+    times.push(performance.now() - start);
+  }
+
+  const avg = times.reduce((a, b) => a + b, 0) / times.length;
+  const min = Math.min(...times);
+  const max = Math.max(...times);
+  const stdDev = Math.sqrt(times.reduce((sum, t) => sum + Math.pow(t - avg, 2), 0) / times.length);
+
+  return { name, avg, min, max, stdDev, iterations };
+}


The new function benchmarkSync is never awaited but the variable is declared with const in an async context. While this works, the return value should be returned directly without being assigned to a variable since it's synchronous. Also, the JSDoc comment states "Run a synchronous benchmark" but doesn't explain why this separate function exists or when to use it versus the async benchmark function.

Copilot · 2025-12-22T05:38:40Z

benchmark.mjs

+
+  return { faster, ratio, speedup };


The function compareResults signature changed to add optional parameters jsLabel and tsLabel with defaults, but it now returns an object { faster, ratio, speedup } that is stored in poolCreateComp but never used. This suggests either the return value should be used somewhere or the function shouldn't return anything to avoid confusion.

Suggested change

return { faster, ratio, speedup };

Copilot · 2025-12-22T05:38:40Z

benchmark.mjs

  printResults(advPoolCreate);

-  compareResults(jsPoolCreate, tsPoolCreate);
+  const poolCreateComp = compareResults(jsPoolCreate, tsPoolCreate);


Variable poolCreateComp is assigned the return value of compareResults but is never used afterwards. Either use this value or remove the variable assignment to keep the code clean.

Suggested change

const poolCreateComp = compareResults(jsPoolCreate, tsPoolCreate);

compareResults(jsPoolCreate, tsPoolCreate);

Copilot · 2025-12-22T05:38:41Z

benchmark.mjs

+  }
+
+  console.log('  ├─────────────────────────────────────────────────────────────────────┤');
+  console.log(`  │  Wins: JS=${jsWins}, TS+WASM=${tsWins}, AdvancedPool=${advWins}${' '.repeat(33)}│`);


Similar padding issue exists here where 33 is used as a fixed padding value. If the wins summary string length exceeds expectations, this could result in negative padding values being passed to String.prototype.repeat(), which would cause an error.

Copilot · 2025-12-22T05:38:41Z

benchmark.mjs

 */

 import { performance } from 'perf_hooks';
+import { execSync } from 'child_process';


The import execSync from 'child_process' is added but never used in the benchmark file. This unused import should be removed to keep the code clean.

Suggested change

import { execSync } from 'child_process';

Copilot · 2025-12-22T05:38:42Z

src/ts/full.ts

 // Binary protocol for WASM (TypeScript stubs for testing)
 export {
-  MAGIC as BINARY_MAGIC,
+  MAGIC,


The alias BINARY_MAGIC has been removed in favor of exporting just MAGIC. This is an inconsistent naming change - other binary protocol constants like BINARY_VERSION at line 916 still retain the BINARY_ prefix for clarity. Consider either keeping MAGIC as BINARY_MAGIC for consistency with BINARY_VERSION, or renaming all binary protocol exports to remove the prefix consistently.

Suggested change

MAGIC,

MAGIC,

MAGIC as BINARY_MAGIC,

Copilot · 2025-12-22T05:38:42Z

benchmark.mjs

+  console.log('  │  OVERALL RESULT                                                     │');
+  console.log('  ├─────────────────────────────────────────────────────────────────────┤');
+  console.log(`  │    TS+WASM is ${overallSpeedup.toFixed(2)}x ${overallSpeedup > 1 ? 'faster' : 'slower'} than JS on average${' '.repeat(28)}│`);
+  console.log(`  │    Runtime: ${runtime}${' '.repeat(55 - runtime.length)}│`);


The string formatting for the summary tables uses hardcoded padding calculations that may break if the runtime string length varies significantly. For example, on line 991, the padding calculation 55 - runtime.length could become negative if the runtime string exceeds 55 characters, breaking the table layout. Consider adding a Math.max(0, ...) or validating the string length.

Suggested change

console.log(` │ Runtime: ${runtime}${' '.repeat(55 - runtime.length)}│`);

console.log(` │ Runtime: ${runtime}${' '.repeat(Math.max(0, 55 - runtime.length))}│`);

Copilot · 2025-12-22T05:38:42Z

benchmark.mjs

+  console.log('\n  ┌─────────────────────────────────────────────────────────────────────┐');
+  console.log('  │  OVERALL RESULT                                                     │');
+  console.log('  ├─────────────────────────────────────────────────────────────────────┤');
+  console.log(`  │    TS+WASM is ${overallSpeedup.toFixed(2)}x ${overallSpeedup > 1 ? 'faster' : 'slower'} than JS on average${' '.repeat(28)}│`);


The same padding calculation issue exists here where 28 spaces are hardcoded for the "TS+WASM is X.XXx faster/slower" message.

Suggested change

console.log(` │ TS+WASM is ${overallSpeedup.toFixed(2)}x ${overallSpeedup > 1 ? 'faster' : 'slower'} than JS on average${' '.repeat(28)}│`);

const overallInnerWidth = 68;

const overallMessage = `TS+WASM is ${overallSpeedup.toFixed(2)}x ${overallSpeedup > 1 ? 'faster' : 'slower'} than JS on average`;

const overallPadding = Math.max(0, overallInnerWidth - 4 - overallMessage.length);

console.log(` │ ${overallMessage}${' '.repeat(overallPadding)}│`);

Copilot · 2025-12-22T05:38:43Z

examples/typescript/protocolV2Complete.ts

+    // Find an idle worker using Array.from to avoid Map iterator issues
+    let idleWorker: Worker | null = null;
+    const workerList = Array.from(this.workers.values());
+    for (let i = 0; i < workerList.length; i++) {
+      if (workerList[i].status === 'idle') {
+        idleWorker = workerList[i];


The comment "Find an idle worker using Array.from to avoid Map iterator issues" suggests there were iterator issues with Map, but using Array.from(this.workers.values()) followed by a traditional for loop is unnecessarily verbose. A simpler approach would be to use for (const worker of this.workers.values()) which is the idiomatic way to iterate over Map values without any issues. The current implementation creates an unnecessary intermediate array.

Suggested change

// Find an idle worker using Array.from to avoid Map iterator issues

let idleWorker: Worker | null = null;

const workerList = Array.from(this.workers.values());

for (let i = 0; i < workerList.length; i++) {

if (workerList[i].status === 'idle') {

idleWorker = workerList[i];

// Find an idle worker by iterating over the worker map values

let idleWorker: Worker | null = null;

for (const worker of this.workers.values()) {

if (worker.status === 'idle') {

idleWorker = worker;

claude added 2 commits December 22, 2025 00:43

Copilot AI review requested due to automatic review settings December 22, 2025 05:34

danielsimonjr merged commit f70cb7c into master Dec 22, 2025
2 of 8 checks passed

Copilot started reviewing on behalf of danielsimonjr December 22, 2025 05:35 View session

Copilot AI reviewed Dec 22, 2025

View reviewed changes

danielsimonjr deleted the claude/optimize-typescript-wasm-K0cfU branch January 7, 2026 02:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: enhance benchmark with Protocol V2 features#52

perf: enhance benchmark with Protocol V2 features#52
danielsimonjr merged 2 commits intomasterfrom
claude/optimize-typescript-wasm-K0cfU

danielsimonjr commented Dec 22, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	getMessagePriority as getBinaryMessagePriority,
	getMessagePriority,

	const poolCreateComp = compareResults(jsPoolCreate, tsPoolCreate);
	compareResults(jsPoolCreate, tsPoolCreate);

	console.log(` │ Runtime: ${runtime}${' '.repeat(55 - runtime.length)}│`);
	console.log(` │ Runtime: ${runtime}${' '.repeat(Math.max(0, 55 - runtime.length))}│`);

-  console.log(`  │    TS+WASM is ${overallSpeedup.toFixed(2)}x ${overallSpeedup > 1 ? 'faster' : 'slower'} than JS on average${' '.repeat(28)}│`);
+  const overallInnerWidth = 68;
+  const overallMessage = `TS+WASM is ${overallSpeedup.toFixed(2)}x ${overallSpeedup > 1 ? 'faster' : 'slower'} than JS on average`;
+  const overallPadding = Math.max(0, overallInnerWidth - 4 - overallMessage.length);
+  console.log(`  │    ${overallMessage}${' '.repeat(overallPadding)}│`);

Conversation

danielsimonjr commented Dec 22, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants