Validate if a rewrite in TS has any performance penalty

[StarpTech]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Maintaining types is hard and error-prone. I opened this issue to make an experiment if a rewrite of this module in TS has any performance penalty. If this experiment succeeds, we can simplify contributions and don't have to maintain TS types separately.

• Rewrite in TS without breaking changes
• Validate the performance of JS and TS versions.

@kibertoad

[Eomm]

we can simplify contributions and don't have to maintain TS types separately.

is this the truth?

I'm not a blocker here, but I want to say that I'm out of all the packages written in TS

[Uzlopak]

There is a use case for everything. I dont see typescript here as having any benefit. Its like a project on purpose uses hand crafted assembler to ensure high performance and somebody suggests C++ because it is more modern.

I am personally a big fan of typescript and use it in projects regularly. But I am openly against using typescript in any of our packages. And tbh. I am not convinced, that the overhead of controlling if the output of our code is what we wanted in the first place is worth it. Also after an update of typescript, we would need to check again if the code changed in a significant way.

And I am not convinced that rewriting fastify-plugin will solve the original issue in #217 . You have still the same issue because you have to do the same work.

[StarpTech]

Let's allow the experiment to take place before labeling it as a failure. No long-term decision was made here. @kibertoad wants to give it a shot.

[Uzlopak]

I asked kibertoad regarding your assignment of this task to him and your claim that he wanted to "give it a shot". Actually i asked him, if there is maybe a misunderstanding, because he just wrote that he would test and benchmark if there is a solution in typescript.

I will remove the assignment of kibertoad to this task, because i think it is a misunderstanding. Also i want to avoid the impression, that if he doesnt supply a PR or a fork with the desired typescript implementation that you think we kind of block this feature request from being assessed.

You are free to assign this issue to yourself and implement a typescript rewrite so that kibertoad and others can review it.

Also kibertoad is of course free to pick this task if he wants to do the heavy lifting.

[kibertoad]

I am happy to write benchmark for this task, but not sure when I'll find time for it

[StarpTech]

I believe it may not be worthwhile to pursue further investigation if the majority is opposed to it. My rationale for assigning @kibertoad to this issue was based on their comment in #217 (comment). However, it is possible that I misunderstood their perspective. My motivation has waned, and I will close this issue.

[kibertoad]

I am generally curious where this might go and especially curious to see perf implications of this change. let's not close it just yet.
I'll contribute what I can

[Eomm]

I believe it may not be worthwhile to pursue further investigation if the majority is opposed to it.

I must say that a fastify-plugin-ts could make everyone happy:

• the experimentation will take place
• the data and users' feedback will show if it works better

This package is quite feature-complete, so it does not change so often (the last feature is almost 1 year old)

[Uzlopak]

I on the other hand expect alot of ts-ignore.... :D

[andersonjoseph]

For the sake of the experiment I would like to give it a shot to this with a fastify-plugin-ts

[andersonjoseph]

Hey! I made a fork with my attempt on migrating the plugin to typescript (still a wip) if you would like to give it a review would be great.

@StarpTech @kibertoad

[metcoder95]

Based on the experiment (great work @andersonjoseph btw 🚀 ) I do not foresee too many performance regressions, but I'm still curious. Did you have the chance to run any benchmarks?

For this, I'm more about the DX 😅

[Uzlopak]

How do you want to benchmark something which is basically run a limitted amount of times at startup

[metcoder95]

I was thinking something simpler as just loading X arbitrary plugins to fastify apps, and compare (not necessarily statistically valid).

e.g. I did this just as a quick experiment:

const fp = require('fastify-plugin')
const fpts = require('fastify-plugin-ts')
const Benchmark = require('benchmark')
const fastify = require('fastify')

const suite = new Benchmark.Suite()

suite
  .add(
    'Fastify Plugin',
    async function () {
      const app = fastify()

      for (let i; i < 10; ++i) {
        app.register(
          fp((instnace, opts, done) => done(), {
            name: i
          })
        )
      }

      await app.ready()
      await app.close()
    },
    {
      minSamples: 100
    }
  )
  .add(
    'Fastify Plugin TS',
    async function () {
      const app = fastify()

      for (let i; i < 10; ++i) {
        app.register(
          fpts((instnace, opts, done) => done(), {
            name: i
          })
        )
      }

      await app.ready()
      await app.close()
    },
    {
      minSamples: 100
    }
  )
  .on('cycle', function (event) {
    console.log(String(event.target))
  })
  .on('complete', function () {
    console.log('Fastest is ' + this.filter('fastest').map('name'))
  })
  .run({
    async: true
  })

Too much room for improvement, for sure. We are interested in the ready being called, but sadly we need to pay the bill for the close one due to a memory leak in http module from Node.

So far, the numbers looks similar:

Fastify Plugin x 26,146 ops/sec ±1.57% (185 runs sampled)
Fastify Plugin TS x 26,015 ops/sec ±1.67% (185 runs sampled)

But the script is really naive, many optimizations can be done to fully target what we want to really measure

[andersonjoseph]

Did you have the chance to run any benchmarks

I haven’t run any benchmarks yet, but I was thinking of doing something similar to your script, @metcoder95. But I wasn't sure if that’s the best way to measure performance

[Uzlopak]

If you do async true then set delay 0

[metcoder95]

But I wasn't sure if that’s the best way to measure performance

It should test what we are looking for, which is the internals of the fastify-plugin, which are constituted of mostly the callback part. That's why app.ready is the key one here.

Although, the script can be optimized, especially finding a way to reset the app so a new app and its plugins are loaded so the suite just waits for the app to be ready with app.ready.
Please feel free to adapt it and play with it as you need 👍

[Eomm]

The comparison should consider the fp's options imho:

• check fastify version
• check required dependancies
• check required decorators

[andersonjoseph]

I added some benchmarks to the repo, I tried to optimize the script to only measure app.ready and also added benchmarks for fp's options. Here are the results on my machine:

Benchmark for simple case
Fastify Plugin x 98,947 ops/sec ±2.54% (79 runs sampled)
Fastify Plugin Typescript x 98,539 ops/sec ±1.91% (85 runs sampled)

Benchmark for fastify version checking
Fastify Plugin x 96,928 ops/sec ±2.53% (78 runs sampled)
Fastify Plugin Typescript x 97,231 ops/sec ±1.88% (82 runs sampled)

Benchmark for fastify dependencies checking
Fastify Plugin x 100,273 ops/sec ±1.89% (80 runs sampled)
Fastify Plugin Typescript x 99,840 ops/sec ±1.68% (83 runs sampled)

Benchmark for fastify decorators checking
Fastify Plugin x 98,176 ops/sec ±2.01% (80 runs sampled)
Fastify Plugin Typescript x 99,913 ops/sec ±1.75% (84 runs sampled)

---

here's the PR: fastify-plugin-ts/#2