Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Improve performance by caching verifier #360

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

SeanReece
Copy link
Contributor

@SeanReece SeanReece commented Jan 7, 2025

Description

Closes: #358

This PR fixes an issue causing fast-jwt verifier to be recreated on every request, which means the fast-jwt cache was not being used. By caching the fast-jwt verifier based on the options passed in, an existing verifier instance can be reused to verify incoming request JWTs instead of having to recreate a new verifier, and then the fast-jwt cache can be utilized (if enabled).

Benchmarks

Tested using a simple fastify server with autocannon used to create load

No auth: 77,978 req/s

Changes (no cache): 53,008 req/s
Changes (cache): 60,880 req/s

Main (no cache): 46,302 req/s
Main (cache): 34,669.1 req/s

image

Very odd behaviour on current main where enabling cache has a performance impact, I believe this is caused by fast-jwt having to recreate cache on every request.

Config used:

fastify.register(require('@fastify/jwt'), {
  secret: 'supersecret',
  decode: { complete: true },
  verify: {
     cache: true, // disabled for "no cache" tests
  },
})

Checklist

@kibertoad
Copy link
Member

@SeanReece ping! do you need any help with this?

@SeanReece
Copy link
Contributor Author

@kibertoad Thanks for reaching out - work has just been busy lately. I've been spending a bit more time getting familiar with the fastify-jwt source and I think I have a better solution instead of caching verifier instances. Hoping to have something today.

function decode (token, options) {
assert(token, 'missing token')

let selectedDecoder = decoder

if (options && typeof options !== 'function') {
if (options && options !== decodeOptions && typeof options !== 'function') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a new decoder was also being created on every request (see line 478), this ensures we will use the global decoder if the default/global settings are used.


if (typeof options === 'function' && !next) {
if (next === undefined) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moving this from below to preserve options passed in even when using promises.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@kibertoad kibertoad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job!

@kibertoad kibertoad merged commit 5bb333a into fastify:master Jan 22, 2025
11 checks passed
@SeanReece SeanReece deleted the cache-verifier branch January 22, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

performance issue: createVerifier called for every request and cache not used
4 participants