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

How to make Express 2x faster #5998

Open
andrehrferreira opened this issue Sep 30, 2024 · 47 comments
Open

How to make Express 2x faster #5998

andrehrferreira opened this issue Sep 30, 2024 · 47 comments

Comments

@andrehrferreira
Copy link

Hello friends of the community, how are you?

First of all, I always like to make it clear that I simply love the Express project. I have been using it in my projects for over 8 years. Therefore, this post seeks to help improve the project even more. I recently opened an inquiry regarding the performance of Express compared to other HTTP server options available. The focus on the release of version 5 was mentioned first. I fully agree that there are several priority issues in the project. However, on my own initiative, I began to study the codes more deeply to try to understand what causes Express to have a lower performance than Fasfity or Koa, for example. So I started a reinterpretation by implementing the same Express functions in a new project. In my specific case, my focus is on integrating Vite functionalities into my HTTP server and creating other layers of abstraction such as decorators. However, using Express as a base, during the development of this project I realized that the biggest performance problem that Express faces is related to the use of 'Object.defineProperty', I'll explain why.

Both Koa and Fastify use an approach of creating a new Request and Response object, defining getters and applying the objects generated by HTTP and HTTP2 from nodejs as a 'raw', assigning them as a simple property, through the getters retrieving the necessary data such as headers, body, params and queries, Express assigns getters dynamically in both the request and response using the "defineGetter" function, I understand that the way it was done takes advantage of the entire structure of original properties and functions, but the processing cost to dynamically add these getters is very high, even using Reflect.defineProperty as an alternative there is still a delay that considerably reduces the amount of requests that Express can process per second.

To make it clearer, I'll leave a simple test comparing the two approaches:

For the test I am using my personal computer a Core i9 10980XE, with 256GB DDR4, Sansung NVME SSD, on Windows 10, using WLS from Ubuntu 22.04, Node in version 20.17.0, Autocannon in version 7.15.0

autocannon -w 8 -d 10 -c 1024 http://localhost:3000

Object Define Property

const http = require('http');

const server = http.createServer((req, res) => {
  
  Object.defineProperty(req, 'xhr', {
    configurable: true,
    enumerable: true,
    get: function () {
        var val = this.headers["X-Requested-With"] || '';
        return val.toLowerCase() === 'xmlhttprequest';
    }
  });
  
  res.writeHead(200, { 'Content-Type': 'application/json' });
  res.end(JSON.stringify({ xhr: req.xhr }));
});

server.listen(3000, () => {
  console.log('Server with Object.defineProperty is running on http://localhost:3000');
});

Result:

Stat 1% 2.5% 50% 97.5% Avg Stdev Min
Req/Sec 18,447 18,447 21,551 23,135 21,518.4 1,202.72 18,441
Bytes/Sec 3.43 MB 3.43 MB 4.01 MB 4.3 MB 4 MB 224 kB 3.43 MB

Object Create

const http = require('http');

const request = {
    get xhr() {
        var val = this.req.headers["X-Requested-With"] || '';
        return val.toLowerCase() === 'xmlhttprequest';
    }
}

const server = http.createServer((req, res) => {
    let obj = Object.create(request);
    obj.req = req;
    res.writeHead(200, { 'Content-Type': 'application/json' });
    res.end(JSON.stringify({ xhr: obj.xhr }));
});

server.listen(3001, () => {
  console.log('Server with Object.create is running on http://localhost:3001');
});

Result:

Stat 1% 2.5% 50% 97.5% Avg Stdev Min
Req/Sec 20,447 20,447 25,359 26,383 24,836.8 1,658.27 20,440
Bytes/Sec 3.8 MB 3.8 MB 4.72 MB 4.91 MB 4.62 MB 309 kB 3.8 MB

Note that in the examples above we are defining only 1 getter in the request, but this action occurs several times in both the request and the response, greatly reducing the number of requests per second that Express can serve. With this change, Express will have the same performance as Koa and Fastify. Basically, I know because I have already tested it by basically rewriting the request/response with the same functions currently present. I did not send a PR for the change because I was waiting for version 5 to be officially available to check if this point was changed. However, checking the version code I found that it is apparently the same as version 4.

I hope I have helped improve the project, and if you want my help to change the code I am available, see you later =)

@cesco69
Copy link

cesco69 commented Sep 30, 2024

@andrehrferreira
Copy link
Author

@cesco69 Yes, I checked earlier before posting, I will wait for the project maintainers to respond to the post to see if I implement the resolution and send it to them or if they will make the change themselves.

@cesco69
Copy link

cesco69 commented Sep 30, 2024

And also... While Object.defineProperty() provides flexibility, it adds overhead compared to directly setting properties on an object. Consider directly adding values to the request object instead of defining getters for every property, particularly if these values won't change during a request's lifecycle.

req.secure = req.protocol === 'https';

instead of

defineGetter(req, 'secure', function secure(){
  return this.protocol === 'https';
});

@tjdav
Copy link

tjdav commented Sep 30, 2024

Would it be beneficial to extend the res properties using the Object.create second argument?

var req = Object.create(http.IncomingMessage.prototype)

for example

var req = Object.create(http.IncomingMessage.prototype, {
  xhr: {
    configurable: true,
    enumerable: true,
    get () {
      var val = this.req.headers["X-Requested-With"] || '';
      return val.toLowerCase() === 'xmlhttprequest';
    }
  }
})

@nigrosimone
Copy link

nigrosimone commented Sep 30, 2024

Would it be beneficial to extend the res properties using the Object.create second argument?

var req = Object.create(http.IncomingMessage.prototype)

for example

var req = Object.create(http.IncomingMessage.prototype, {
  xhr: {
    configurable: true,
    enumerable: true,
    get () {
      var val = this.req.headers["X-Requested-With"] || '';
      return val.toLowerCase() === 'xmlhttprequest';
    }
  }
})

benchmark

defineGetter(req, 'xhr', function xhr(){
  var val = this.get('X-Requested-With') || '';
  return val.toLowerCase() === 'xmlhttprequest';
});

VS

req = Object.create(req, {
  xhr: {
    configurable: true,
    enumerable: true,
    get() {
      var val = this.req.headers["X-Requested-With"] || '';
      return val.toLowerCase() === 'xmlhttprequest';
    }
  }
});

RESULT
defineGetter: 950,260 ops/s
Object.create: 305,490 ops/s

defineGetter wins (3x FASTER)!

@andrehrferreira
Copy link
Author

@tjdav From what I've seen, the problem is precisely extending based on the request that already has many parameters/functions. Object.create and Object.defineProperty probably perform some type of validation that ends up weighing down the process. That's why the alternative used by Koa and Fastify is to use req and res as a direct property and create getters by calling the property.

@nigrosimone
Copy link

nigrosimone commented Sep 30, 2024

@tjdav & @andrehrferreira

With Object.defineProperty the engine (Node/V8) has to handle the possibility of custom getters, setters, and property descriptors, which can prevent certain optimizations that would otherwise make property access faster.

For example, consider the difference between these two methods:

Direct assignment: 4,937,680 ops/s

const obj = {};
obj.prop = 42; 

Object.defineProperty: 1,191,010 ops/s

const obj = {};
Object.defineProperty(obj, 'prop', {
    value: 42,
    writable: true,
    enumerable: true,
    configurable: true
});

what I don't understand is why some functions have been assigned to the request with direct assignment (eg.: https://github.com/expressjs/express/blob/5.0/lib/request.js#L257) while others are with Object.defineProperty

see https://humanwhocodes.com/blog/2015/11/performance-implication-object-defineproperty/ and https://v8.dev/blog/fast-properties

Using direrect assignment make all a bit faster

@cesco69
Copy link

cesco69 commented Oct 1, 2024

Hi, I have tried (in PR #6004) to extend the request

class Request extends http.IncomingMessage { 

}

ad set the getter without Object.defineProperty, eg.:

class Request extends http.IncomingMessage { 
  get query() {
    var queryparse = this.app.get('query parser fn');

    if (!queryparse) {
      // parsing is disabled
      return Object.create(null);
    }

    var querystring = parse(this).query;

    return queryparse(querystring);
  }
}

all 1227 tests passing

I'm on Windows and the express benchmark works only on linux. Can someone run this for me and show me if my PR improves performance?

@faulpeltz
Copy link

Run on Linux Mint 22, on a Ryzen 3900X, with Node 22.9.0
The default benchmark only runs for 3sec per run which gave very inconsistent results, this is with 30sec/run.
Compared versions:
left 3 columns: vanilla express from expressjs/express:master
right 2 columns: your patch commit 8e3c005

Unfortunately there were no significant changes:
express_perf1

Flame chart using flame with default settings and one 30sec run with 10 middleware, 100 conn
express_flamechart1

@andrehrferreira
Copy link
Author

I'm testing my project using the same parameters and functions that exist in Express and the result is well balanced. I'll leave it here for reference. Obviously, it needs to be adapted to the reality of Express, which uses pure JavaScript.

https://github.com/andrehrferreira/cmmv-server/blob/main/packages/server/lib/request.ts

https://github.com/andrehrferreira/cmmv-server/blob/main/packages/server/lib/response.ts

It is not yet 100% implemented but preliminary results have been:

(index) Framework Reqs/s Total Reqs Transfer/s Transfer Total Latency
0 http 24635 246341 4.18 MB 41.82 MB 123 ms
1 fastify 23451 234531 3.58 MB 35.79 MB 110 ms
2 cmmv 21877 218783 3.34 MB 33.38 MB 116 ms
3 koa 21823 218223 3.64 MB 36.42 MB 111 ms
4 hapi 16244 162432 3.42 MB 34.23 MB 88 ms
5 express 6668 66677 1.51 MB 15.13 MB 83 ms

https://github.com/andrehrferreira/cmmv-server/blob/main/tools/benchmarks/benchmarks-allservers.js

@cesco69
Copy link

cesco69 commented Oct 1, 2024

@andrehrferreira Wow! cmmv is really close to fastify!
@faulpeltz thanks!

@andrehrferreira
Copy link
Author

@cesco69 I honestly want Express to solve the performance problem so I don't have to use my project, it's a lot of work, but since my focus is SEO and keeping the application's TTFB low, I need the request latency to be as low as possible.

@IamLizu
Copy link
Member

IamLizu commented Oct 1, 2024

Interesting, I was looking at router module the other day and I think using something smarter than a liner search approach for route matching might make it a little bit fast.

Maybe it could improve the benchmarks for requests handled per second but I am skeptical.

cc: @wesleytodd

@andrehrferreira
Copy link
Author

@IamLizu Have you taken a look at https://www.npmjs.com/package/find-my-way ?

@nigrosimone
Copy link

@andrehrferreira congratulations, your server is very interesting. I wonder if you think you can make a pull request on express and get the same performance without breaking changes?

@IamLizu
Copy link
Member

IamLizu commented Oct 1, 2024

@andrehrferreira yes, I have.

That said, I am patiently waiting to see what others think about this. Folks have a plan to make Express faster and compare the benchmarks with its own earlier version.

@andrehrferreira
Copy link
Author

@nigrosimone I'm thinking about doing this, but it's a big change so I'd prefer the maintainers to signal whether they prefer to do it or want me to do it.

@andrehrferreira
Copy link
Author

@IamLizu I'm using it in my project and it's more efficient than the Express router, and the same one used in Fastify.

@cesco69
Copy link

cesco69 commented Oct 2, 2024

@IamLizu perhaps it would be useful to have a backlog of ideas to implement to improve performance, so those who want to help can work on something

@LancerComet
Copy link

@tjdav & @andrehrferreira

With Object.defineProperty the engine (Node/V8) has to handle the possibility of custom getters, setters, and property descriptors, which can prevent certain optimizations that would otherwise make property access faster.

For example, consider the difference between these two methods:

Direct assignment: 4,937,680 ops/s

const obj = {};
obj.prop = 42; 

Object.defineProperty: 1,191,010 ops/s

const obj = {};
Object.defineProperty(obj, 'prop', {
    value: 42,
    writable: true,
    enumerable: true,
    configurable: true
});

what I don't understand is why some functions have been assigned to the request with direct assignment (eg.: https://github.com/expressjs/express/blob/5.0/lib/request.js#L257) while others are with Object.defineProperty

see https://humanwhocodes.com/blog/2015/11/performance-implication-object-defineproperty/ and https://v8.dev/blog/fast-properties

Using direrect assignment make all a bit faster

I speculate that Express uses getters to lazily compute property values, thus avoiding calculating properties that users do not access. However, for optimizations (fast mode) it might be better to define “get” directly on the object.

@nigrosimone
Copy link

I speculate that Express uses getters to lazily compute property values, thus avoiding calculating properties that users do not access. However, for optimizations (fast mode) it might be better to define “get” directly on the object.

yep, sure, but seems it's more slower add getter instead of compute the properties.

@cesco69
Copy link

cesco69 commented Oct 3, 2024

I speculate that Express uses getters to lazily compute property values, thus avoiding calculating properties that users do not access. However, for optimizations (fast mode) it might be better to define “get” directly on the object.

yep, sure, but seems it's more slower add getter instead of compute the properties.

Some properties can be static, eg. protocol and secure in a real world scenario doesn't change per request, but I believe that making them static would be a disruptive change..

defineGetter(req, 'protocol', function protocol(){
  var proto = this.connection.encrypted
    ? 'https'
    : 'http';
  var trust = this.app.get('trust proxy fn');

  if (!trust(this.connection.remoteAddress, 0)) {
    return proto;
  }

  // Note: X-Forwarded-Proto is normally only ever a
  //       single value, but this is to be safe.
  var header = this.get('X-Forwarded-Proto') || proto
  var index = header.indexOf(',')

  return index !== -1
    ? header.substring(0, index).trim()
    : header.trim()
});

defineGetter(req, 'secure', function secure(){
  return this.protocol === 'https';
});

@zhang-wenchao
Copy link

Switch to Deno at runtime and obtain 3 times performance improvement.

@andrehrferreira
Copy link
Author

@zhang-wenchao Sorry but I don't trust projects guided by Ryan Dahl after all the history that happened with node

@cesco69
Copy link

cesco69 commented Oct 9, 2024

@andrehrferreira
if I wanted to modify the request and response of express, and applying the HTTP and HTTP2 generated objects from nodejs as 'raw', do you know any tricks?

eg.:

const express = require('express');
const http = require('http');

express['request'] = {
    // ...
    raw: http.IncomingMessage.prototype
};

express['response'] = {
   // ...
   raw: http.ServerResponse.prototype
};

const app = express()

app.get('/', function (req, res) {
  res.send('Hello World')
})

app.listen(3000)

@andrehrferreira
Copy link
Author

@cesco69 It's a very big change, it's not as simple as it seems, especially since this change needs to ensure that all the project's existing tests pass.

@cesco69
Copy link

cesco69 commented Oct 9, 2024

I believe the primary challenge lies in the custom middleware that interacts with the properties of the HTTP module. One potential solution to avoid breaking existing functionality is to introduce this change behind an opt-in configuration in a future 6.0.0 branch.

Here's a suggested implementation:

const express = require('express');

const app = express({ raw: true }); // Opt-in raw property for request/response

app.get('/', function (req, res) {
  res.send('Hello World');
});

app.listen(3000);

This approach allows users to test the changes and verify that all middleware remains compatible. Additionally, custom middleware developers can check for the existence of this property and adjust its behavior accordingly (if need to access to the http request/response):

app.use((req, res, next) => {
  if (req.raw) {
    // Handle the raw request
    req.raw.xyz
  } else {
    // Handle the standard request
    req.xyz
  }
});

By implementing this feature as opt-in, we can ensure that users have the flexibility to adopt it at their own pace while maintaining backward compatibility.

@andrehrferreira
Copy link
Author

@cesco69 It is possible to change it while maintaining exactly the same behavior that already exists today by creating gates that directly call raw as an abstraction, and just not merging the classes that works well, it takes a little work to do, there is no need to do anything palliative.

@nigrosimone
Copy link

nigrosimone commented Oct 9, 2024

@andrehrferreira You could try doing this PR as a POC. I think the Express team isn't very interested, but perhaps, seeing the results, they might change their minds. I can help you test it on some of my projects.

@andrehrferreira
Copy link
Author

andrehrferreira commented Oct 9, 2024

@nigrosimone In my personal project I have already made this change, I will leave the results below, its name is CMMV (https://github.com/andrehrferreira/cmmv-server)

  • https://github.com/fastify/benchmarks
  • Machine: linux x64 | 32 vCPUs | 256.6GB Mem
  • Node: v20.17.0
  • Run: Thu Oct 02 2024 15:23:41 GMT+0000 (Coordinated Universal Time)
  • Method: autocannon -c 100 -d 40 -p 10 localhost:3000
Version Router Requests/s Latency (ms) Throughput/Mb
bare v20.17.0 45270.4 21.62 8.07
micro 10.0.1 44705.8 21.93 7.97
fastify 5.0.0 44547.8 22.01 7.99
connect 3.7.0 44174.4 22.18 7.88
polka 0.5.2 43791.2 22.37 7.81
rayo 1.4.6 43731.8 22.41 7.80
server-base-router 7.1.32 43117.6 22.72 7.69
server-base 7.1.32 42169.4 23.24 7.52
micro-route 2.5.0 41600.0 23.55 7.42
connect-router 1.3.8 41163.3 23.85 7.34
cmmv 0.4.0 40995.2 23.92 7.35
hono 4.6.3 39738.6 24.68 7.09
polkadot 1.0.0 37472.8 26.20 6.68
koa 2.15.3 37181.4 26.42 6.63
0http 3.5.3 37101.6 26.47 6.62
take-five 2.0.0 35171.4 27.95 12.65
h3 1.13.0 34667.4 28.35 6.18
koa-isomorphic-router 1.0.1 34542.7 28.46 6.16
h3-router 1.13.0 33551.0 29.31 5.98
restana 4.9.9 33532.8 29.36 5.98
koa-router 12.0.1 33426.2 29.46 5.96
microrouter 3.1.3 30049.2 32.79 5.36
hapi 21.3.10 30014.8 32.82 5.35
restify 11.1.0 28548.0 34.55 5.15
fastify-big-json 5.0.0 11675.6 85.19 134.34
express 5.0.1 10058.2 98.82 1.79
express-with-middlewares 5.0.1 8826.8 112.63 3.28
trpc-router 10.45.2 N/A N/A N/A

@nigrosimone
Copy link

Yep! I have see your awesome projects! But i have Legacy project with Express to Speed up 🥲

@andrehrferreira
Copy link
Author

@nigrosimone It's a big change, it will take a few days to make it 100% and pass all the tests, my concern is wasting time doing this PR and it not being in the interest of the maintainers to change the base structure needed to improve performance, since version 5 is basically the same code as version 4.

@nigrosimone
Copy link

@nigrosimone It's a big change, it will take a few days to make it 100% and pass all the tests, my concern is wasting time doing this PR and it not being in the interest of the maintainers to change the base structure needed to improve performance, since version 5 is basically the same code as version 4.

I understand and you're right, but if the team doesn't care about your idea, you could fork "Express" and call it for example "Fastpress" as a drop to replace Express and anyone who uses Express could switch to yours and a new community could be born around it. You are working on your own server anyway, so I believe you have the motivation and skills to carry out a large project independently.

@andrehrferreira
Copy link
Author

@nigrosimone It is not my goal to honestly fork Express, because if I were to do that I would just have to adapt my project to have the same features and pass all the Express tests, what I want is to not have to maintain independent code and solve Express' problems to use it again in my products, it is a lot of work to maintain the code and I have no way of dedicating myself to that.

@cesco69
Copy link

cesco69 commented Oct 10, 2024

@dougwilson @wesleytodd give instructions to @andrehrferreira, whether he should work or we can abandon this idea.

@dougwilson
Copy link
Contributor

Hi @cesco69 I'm sure you have put in a lot of work, but apologies I am no longer involved in Express these days. There are a lot of nice people who are, check out the README for the list 👍

@bjohansebas
Copy link
Member

Maybe @UlisesGascon can help you

@nigrosimone
Copy link

nigrosimone commented Oct 12, 2024

Guys, leave a +1 for keep more attention on this isuue on chromium/V8/Turbofan: Object.defineProperty is slow

image

@UlisesGascon
Copy link
Member

Thanks for the ping @bjohansebas!

Currently, in the @expressjs/express-tc, we are discussing priorities for 2025 and what to focus on after the v5 release. 100% performance will be part of it, and I am glad to see that the community is heavily interested in helping with this matter ❤️

In a few weeks, we can organize a solid backlog and start focusing our energies on making Express faster and faster with each release ⚡

@cesco69

This comment was marked as off-topic.

@andrehrferreira

This comment was marked as off-topic.

@cesco69

This comment was marked as off-topic.

@bjohansebas

This comment was marked as off-topic.

@cesco69

This comment was marked as off-topic.

@nigrosimone

This comment was marked as off-topic.

@andrehrferreira

This comment was marked as off-topic.

@UlisesGascon
Copy link
Member

While I love this discussion a lot, I marked some comments as "off-topic" (as requested) to keep the discussion around the main idea. As mentioned in expressjs/discussions#173.

I will strongly suggest to avoid comparing Express with other frameworks, just to avoid the typical social media flame/drama. We can focus on comparing new changes against previous versions (as Node.js does) or even compare Express against Node.js core libraries to see how our deviation increase/decrease

That said, I think that is great to learn from others and see how much of this ideas and improvements we can adapt to Express without sacrificing stability, security, etc...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.