perf: remove intermediate list creation when constructing the error list

[gurgunday]

Removes the usage of an intermediate list and .map when creating the error list, which should make it faster

I created a small benchmark with Claude and it does indeed seem faster (node 22.17):

Verifying results are equivalent...
Old result errors: 5
New result errors: 5
Results match: true

Running 100,000 iterations each...

Old approach (spread + map):
  Total time: 244.55ms
  Average per operation: 0.0024ms
  Operations per second: 408,913

New approach (for loop + push):
  Total time: 214.47ms
  Average per operation: 0.0021ms
  Operations per second: 466,262

Benchmark:

import { TypeCompiler } from '@sinclair/typebox/compiler'
import { Type } from '@sinclair/typebox'
import { Value } from '@sinclair/typebox/value'

const schema = Type.Object({
  name: Type.String({ minLength: 3 }),
  age: Type.Number({ minimum: 0, maximum: 120 }),
  email: Type.String({ format: 'email' }),
  address: Type.Object({
    street: Type.String(),
    city: Type.String(),
    zipCode: Type.String({ pattern: '^[0-9]{5}$' })
  })
})

// Invalid data that will trigger multiple validation errors
const invalidData = {
  name: 'ab', // too short
  age: -5, // negative
  email: 'invalid-email', // invalid format
  address: {
    street: '', // empty
    city: 123, // wrong type
    zipCode: 'invalid' // wrong pattern
  }
}

const typeCheck = TypeCompiler.Compile(schema)

// Old approach (using spread and map)
function oldErrorConstruction () {
  const errors = [...typeCheck.Errors(invalidData)]
  return {
    error: errors.map((error) => ({
      message: `${error.message}`,
      instancePath: error.path
    }))
  }
}

// New approach (using for loop and push)
function newErrorConstruction () {
  const errors = []
  for (const error of typeCheck.Errors(invalidData)) {
    errors.push({
      message: error.message,
      instancePath: error.path
    })
  }
  return { error: errors }
}

// Benchmark function
function benchmark (name, fn, iterations = 100000) {
  // Warm up
  for (let i = 0; i < 1000; i++) {
    fn()
  }
  
  const start = process.hrtime.bigint()
  for (let i = 0; i < iterations; i++) {
    fn()
  }
  const end = process.hrtime.bigint()
  
  const duration = Number(end - start) / 1000000 // Convert to milliseconds
  const opsPerSecond = Math.round(iterations / (duration / 1000))
  
  console.log(`${name}:`)
  console.log(`  Total time: ${duration.toFixed(2)}ms`)
  console.log(`  Average per operation: ${(duration / iterations).toFixed(4)}ms`)
  console.log(`  Operations per second: ${opsPerSecond.toLocaleString()}`)
  console.log()
  
  return duration
}

console.log('TypeBox Error Construction Benchmark')
console.log('====================================')
console.log()

// Verify both functions produce the same result
const oldResult = oldErrorConstruction()
const newResult = newErrorConstruction()

console.log('Verifying results are equivalent...')
console.log('Old result errors:', oldResult.error.length)
console.log('New result errors:', newResult.error.length)
console.log('Results match:', JSON.stringify(oldResult) === JSON.stringify(newResult))
console.log()

// Run benchmarks
const iterations = 100000
console.log(`Running ${iterations.toLocaleString()} iterations each...`)
console.log()

const oldTime = benchmark('Old approach (spread + map)', oldErrorConstruction, iterations)
const newTime = benchmark('New approach (for loop + push)', newErrorConstruction, iterations)

// Calculate improvement
const improvement = ((oldTime - newTime) / oldTime) * 100
console.log('Performance Summary:')
console.log('===================')
if (improvement > 0) {
  console.log(`New approach is ${improvement.toFixed(1)}% faster`)
} else {
  console.log(`Old approach is ${Math.abs(improvement).toFixed(1)}% faster`)
}
console.log(`Speed ratio: ${(oldTime / newTime).toFixed(2)}x`)

[Fdawgs]

Would a basic for loop do any better?
https://github.com/kibertoad/nodejs-benchmark-tournament/blob/master/loops/_results/results.md

[jean-michelet]

There's a great talk on how arrays are handled in V8 here: https://www.youtube.com/watch?v=m9cTaYI95Zc. But maybe a bit old?

Because it doesn't hurt readability, I am +1.
But in general, I suspect from what I read that the performance gains from optimizing array usage (e.g., using index assignment instead of .push()) are very marginal - especially in cases like this. Given that the errors list is typically quite small.

[gurgunday]

Would a basic for loop do any better? https://github.com/kibertoad/nodejs-benchmark-tournament/blob/master/loops/_results/results.md

Like @jean-michelet said, this might not yield the desired improvements anymore when dealing with small-to-mid-sized arrays. Besides, typeCheck.Errors returns an iterable anyway, so we can't use a normal loop

Here, I just wanted to remove the redundant array creation

[Uzlopak]

LGTM

[mcollina]

lgtm