discussions icon indicating copy to clipboard operation
discussions copied to clipboard

feat(adr): establish a performance working group

Open wesleytodd opened this issue 1 year ago • 6 comments

This proposal would create a Performance Working Group. See the RFC content for details.

NOTE: this WG would operate under the STF proposal we have made. More details on that to come as we work through the process with the German Govt.

wesleytodd avatar Nov 14 '24 15:11 wesleytodd

Hey guys, how are you?

I took some time off to take a year-end break, I recently performed some tests using my integration tool with the Node.js inspector, I'll leave the link to the test repository, https://github.com/andrehrferreira/express-benchmarks, the test is very simple, I uploaded an instance of express without any middleware and performed the monitoring using the inspector, then I used autocannon to trigger multiple requests in this application using the command autocannon -c 100 -d 40 -p 10 localhost:3000

As a result, my system creates a cpuprofile file that I analyze using the application https://discoveryjs.github.io/cpupro, I'll leave the profile file in the repository for deeper analysis, but basically, 37.5% of the processing time was from internal express functions, with 13.8% from response.js and 9% from the router, which in my opinion are the main ones bottlenecks of the current version, 7.3% in middleware/init.js, looking deeper the most worrying is in fact the send function for response, followed by the header and contentType, as I had already commented before in my point of view the main problem is in the use of Object.defineProperty, however I will continue analyzing other important points to be optimized

andrehrferreira avatar Jan 22 '25 05:01 andrehrferreira

Hey @andrehrferreira, thanks for the post. These kinds of things are great, but for the project the larger problem is two fold:

  1. Tracking performance as we change and improve the libraries (also taking into account runtime improvements)
  2. Testing real application performance where the framework is impactful

It has been my experience that many engineers will spend a lot of time on optimizing some small subsystem that has little to no impact on the real world performance of their applications. My goal with setting up this working group is to level up everyone who wants to do perf work and to give ourselves the tools to do the right analysis for the right type of changes. Please keep finding things, and preferably open PRs with a change, a before and after benchmark result, and a description of how that impacts real applications (is it hot path, startup, etc), but also lets post that in the applicable repos instead of here where we are bootstraping the WG. Thanks!


Onto the topic of the WG, I am going to be just starting the work on this next week. I am going to update this proposal soon to include some more detailed goals/outcomes so we can be clear on what this is for.

wesleytodd avatar Feb 04 '25 15:02 wesleytodd

@wesleytodd, How are you?

When I opened this discussion a few months ago, I was at the beginning of a project I'm working on. Due to the understandable long period that Express requires to upload updates of this size, I ended up rewriting all the functions in parallel in a separate project that I'm already using in the application. I'll leave the repository here https://github.com/cmmvio/cmmv-server

In addition to the Express codes, I used part of Fastify and Koa for this version. What I realized in this process, following what you're saying, is that Express currently has 3 serious problems. The first is related to the way the system packages the req/res using Object.defineProperty. This in itself is practically 90% of the performance problem. The second problem is related to the Router, which is slow compared to other methods. In my project, I'm using https://www.npmjs.com/package/find-my-way

The third, relatively minor problem is the middleware processing lifecycle, which Fastify solved it using Hooks, and that was the path I followed as well. Below I will leave the numbers from the last performance test I did:

Benchmarks

  • https://github.com/fastify/benchmarks
  • Machine: linux x64 | 32 vCPUs | 128.0GB Mem
  • Node: v20.17.0
  • Run: Wed Jan 22 2025 05:33:16 GMT+0000 (Coordinated Universal Time)
  • Method: autocannon -c 100 -d 40 -p 10 localhost:3000
Framework Version Router Requests/s Latency (ms) Throughput/Mb
bare v20.17.0 91084.8 10.51 16.24
fastify 5.2.1 87495.9 10.94 15.69
connect-router 1.3.8 86404.8 11.05 15.41
micro-route 2.5.0 86115.2 11.10 15.36
cmmv 0.8.0 85812.8 11.15 15.39
adonisjs 7.4.0 83334.4 11.52 14.86
koa 2.15.3 78946.0 12.17 14.08
koa-isomorphic-router 1.0.1 71901.6 13.42 12.82
koa-router 13.1.0 70117.2 13.77 12.50
microrouter 3.1.3 63030.8 15.37 11.24
hapi 21.3.12 60592.8 16.00 10.81
restify 11.1.0 58288.0 16.65 10.51
fastify-big-json 5.2.1 21667.6 45.64 249.29
express 5.0.1 21299.2 46.42 3.80
express-with-middlewares 5.0.1 18680.4 53.00 6.95

andrehrferreira avatar Feb 04 '25 18:02 andrehrferreira

I just pushed some updates to the proposal. Adding the TC for a review now that it has more details.

wesleytodd avatar Feb 04 '25 18:02 wesleytodd

Just wanted to post this here since it came up on our WG call: https://github.com/openjs-foundation/infrastructure/issues/5

wesleytodd avatar Feb 12 '25 21:02 wesleytodd

A PR to track for this, just wanted to not loose the link again: https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2437

wesleytodd avatar Mar 05 '25 17:03 wesleytodd

Btw, i want to join this group.

bjohansebas avatar Apr 17 '25 18:04 bjohansebas

@bjohansebas can you open a PR adding yourself in the new repo?

wesleytodd avatar Apr 23 '25 15:04 wesleytodd