perf: improve idgenerator (about 25-30 % faster)

[Uzlopak]

my server to run the benchmarkr

'use strict'

const Fastify = require('fastify')
const fastifySession = require('..')
const fastifyCookie = require('@fastify/cookie')

const fastify = Fastify()

fastify.register(fastifyCookie)
fastify.register(fastifySession, {
  secret: 'cNaoPYAwF60HZJzkcNaoPYAwF60HZJzk',
  cookie: { secure: false }
})

fastify.get('/', async (request, reply) => {
  reply
    .send(request.cookies.sessionId)
})

fastify.listen({ port: 3000 })

before:

autocannon  http://127.0.0.1:3000Running 10s test @ http://127.0.0.1:3000
10 connections


┌─────────┬──────┬──────┬───────┬──────┬─────────┬─────────┬───────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%  │ Avg     │ Stdev   │ Max   │
├─────────┼──────┼──────┼───────┼──────┼─────────┼─────────┼───────┤
│ Latency │ 0 ms │ 0 ms │ 0 ms  │ 1 ms │ 0.04 ms │ 0.34 ms │ 17 ms │
└─────────┴──────┴──────┴───────┴──────┴─────────┴─────────┴───────┘
┌───────────┬─────────┬─────────┬────────┬─────────┬─────────┬─────────┬─────────┐
│ Stat      │ 1%      │ 2.5%    │ 50%    │ 97.5%   │ Avg     │ Stdev   │ Min     │
├───────────┼─────────┼─────────┼────────┼─────────┼─────────┼─────────┼─────────┤
│ Req/Sec   │ 16463   │ 16463   │ 22975  │ 24671   │ 22565.1 │ 2038.33 │ 16455   │
├───────────┼─────────┼─────────┼────────┼─────────┼─────────┼─────────┼─────────┤
│ Bytes/Sec │ 4.01 MB │ 4.01 MB │ 5.6 MB │ 6.01 MB │ 5.5 MB  │ 497 kB  │ 4.01 MB │
└───────────┴─────────┴─────────┴────────┴─────────┴─────────┴─────────┴─────────┘

Req/Bytes counts sampled once per second.
# of samples: 11

248k requests in 11.01s, 60.5 MB read

after:

autocannon  http://127.0.0.1:3000
Running 10s test @ http://127.0.0.1:3000
10 connections


┌─────────┬──────┬──────┬───────┬──────┬─────────┬─────────┬───────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%  │ Avg     │ Stdev   │ Max   │
├─────────┼──────┼──────┼───────┼──────┼─────────┼─────────┼───────┤
│ Latency │ 0 ms │ 0 ms │ 0 ms  │ 1 ms │ 0.04 ms │ 0.31 ms │ 19 ms │
└─────────┴──────┴──────┴───────┴──────┴─────────┴─────────┴───────┘
┌───────────┬─────────┬─────────┬─────────┬─────────┬──────────┬─────────┬─────────┐
│ Stat      │ 1%      │ 2.5%    │ 50%     │ 97.5%   │ Avg      │ Stdev   │ Min     │
├───────────┼─────────┼─────────┼─────────┼─────────┼──────────┼─────────┼─────────┤
│ Req/Sec   │ 18351   │ 18351   │ 26319   │ 27023   │ 25586.19 │ 2393.26 │ 18343   │
├───────────┼─────────┼─────────┼─────────┼─────────┼──────────┼─────────┼─────────┤
│ Bytes/Sec │ 4.47 MB │ 4.47 MB │ 6.41 MB │ 6.58 MB │ 6.23 MB  │ 583 kB  │ 4.47 MB │
└───────────┴─────────┴─────────┴─────────┴─────────┴──────────┴─────────┴─────────┘

Req/Bytes counts sampled once per second.
# of samples: 11

281k requests in 11.01s, 68.6 MB read

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

lgtm

[climba03003]

Is the performance better than using toString('base64url') ?

import { randomBytes } from 'crypto'
randomBytes(32).toString('base64url')

[Uzlopak]

@climba03003

There are to implications with randomBytes

• randomBytes is not cached but randomInt is cached in nodejs, so perf of randomInt is significantly higher
• base64url is not returning always 32 characters, as trailing = (base64-padding) are removed

[climba03003]

randomBytes is not cached but randomInt is cached in nodejs, so perf of randomInt is significantly higher

randomInt only start out-perform after 5000, maybe computer dependent.

import crypto from 'crypto'
const randomBytes = crypto.randomBytes

function idGenerator (len) {
  const fnBody = []

  fnBody.push('const randomInt = crypto.randomInt;')
  fnBody.push('const base64url = \'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_\'.split(\'\');')
  fnBody.push('return function () {')
  const chars = []
  for (let i = 0; i < ((len) | 0); ++i) chars.push('base64url[randomInt(0, 64)]')
  fnBody.push('return ' + chars.join('+'))
  fnBody.push('}')
  return new Function('crypto', fnBody.join(''))(crypto) // eslint-disable-line no-new-func
}

const compute1 = idGenerator(32)
const compute2 = function() {
  return randomBytes(32).toString('base64url')
}

console.time('compute1')
for(let i = 5000; i > 0; i--) {
  compute1()
}
console.timeEnd('compute1')

console.time('compute2')
for(let i = 5000; i > 0; i--) {
  compute2()
}
console.timeEnd('compute2')

base64url is not returning always 32 characters, as trailing = (base64-padding) are removed

The id should be consistence in byte length, not consistence in characters.
This plugin should be compatible with express-session store.
I am not sure if any of the store is storing the id as fixed binary length.

[Uzlopak]

I have also a cached randomBytes solution, which is also outperforming non-cached randomBytes.

[Uzlopak]

@climba03003

const crypto = require('crypto')
const randomBytes = crypto.randomBytes

const cacheSize = 24 << 7
let pos = 0
let cache = crypto.randomBytes(cacheSize)

var EQUAL_END_REGEXP = /=/
var PLUS_GLOBAL_REGEXP = /\+/g
var SLASH_GLOBAL_REGEXP = /\//g

const compute1 = Buffer.isEncoding('base64url')
? function () {
  if ((pos + 24) > cacheSize) {
    cache = crypto.randomBytes(cacheSize)
    pos = 0
  }
  const buf = Buffer.allocUnsafe(24)
  cache.copy(buf, 0, pos, (pos += 24))
  return buf.toString('base64url')
}
: function () {
  if ((pos + 24) > cacheSize) {
    cache = crypto.randomBytes(cacheSize)
    pos = 0
  }
  const buf = Buffer.allocUnsafe(24)
  cache.copy(buf, 0, pos, (pos += 24))
  return buf.toString('base64')
    .replace(EQUAL_END_REGEXP, '')
    .replace(PLUS_GLOBAL_REGEXP, '-')
    .replace(SLASH_GLOBAL_REGEXP, '_')
}

const compute2 = Buffer.isEncoding('base64url')
? function() {
  return randomBytes(32).toString('base64url')
}
: function() {
  return randomBytes(32).toString('base64')
  .replace(EQUAL_END_REGEXP, '')
  .replace(PLUS_GLOBAL_REGEXP, '-')
  .replace(SLASH_GLOBAL_REGEXP, '_')
}

console.time('compute1')
for(let i = 50000; i > 0; i--) {
  compute1()
}
console.timeEnd('compute1')

console.time('compute2')
for(let i = 50000; i > 0; i--) {
  compute2()
}
console.timeEnd('compute2')

uzlopak@Battlestation:~/workspace/session$ nvm use 12
Now using node v12.22.12 (npm v6.14.16)
uzlopak@Battlestation:~/workspace/session$ node test
compute1: 39.415ms
compute2: 80.814ms
uzlopak@Battlestation:~/workspace/session$ nvm use 14
Now using node v14.20.0 (npm v6.14.17)
uzlopak@Battlestation:~/workspace/session$ node test
compute1: 24.481ms
compute2: 69.213ms
uzlopak@Battlestation:~/workspace/session$ nvm use 18
Now using node v18.7.0 (npm v8.15.0)
uzlopak@Battlestation:~/workspace/session$ node test
compute1: 27.079ms
compute2: 114.488ms

With 5000 iterations it does not show immediatly, but it is faster in node 18

uzlopak@Battlestation:~/workspace/session$ nvm use 12
Now using node v12.22.12 (npm v6.14.16)
uzlopak@Battlestation:~/workspace/session$ node test
compute1: 10.818ms
compute2: 11.453ms
uzlopak@Battlestation:~/workspace/session$ nvm use 14
Now using node v14.20.0 (npm v6.14.17)
uzlopak@Battlestation:~/workspace/session$ node test
compute1: 9.809ms
compute2: 10.247ms
uzlopak@Battlestation:~/workspace/session$ nvm use 16
Now using node v16.16.0 (npm v8.11.0)
uzlopak@Battlestation:~/workspace/session$ node test
compute1: 7.552ms
compute2: 12.115ms
uzlopak@Battlestation:~/workspace/session$ nvm use 18
Now using node v18.7.0 (npm v8.15.0)
uzlopak@Battlestation:~/workspace/session$ node test
compute1: 6.229ms
compute2: 13.205ms

And I need to differentiate for base64url because base64url exists since node 14.18.0

[Uzlopak]

If i use cached randombytes i get with autocannon about 270k-278k ops/s, with randomInt I get constantly above 280k/ops, sometimes spiking to 290k /ops

[Uzlopak]

@kibertoad
@mcollina

How should I proceed?

[Uzlopak]

Ok. got it with cached randomBytes. It is definetly faster than randomInt. Got 277k ops/s on node 18 vs. randomInt 248k ops/s vs. uid-safe 212k ops/s

[Uzlopak]

Oh I wanted to ping @climba03003 not @kibertoad

[Uzlopak]

I guess everybody is happy :)