search

dimdenGD / ultimate-express

The Ultimate Express. Fastest http server with full Express compatibility, based on µWebSockets.
Apache License 2.0
518 stars 15 forks source link

Bugfix/48 #49

Closed Pho3niX90 closed 1 week ago

Pho3niX90 commented 1 week ago

closes #48

pkg-pr-new[bot] commented 1 week ago

Open in Stackblitz

npm i https://pkg.pr.new/dimdenGD/ultimate-express@49

commit: b938cc4

Pho3niX90 commented 1 week ago

@dimdenGD I am finished with this now if you want to review. I have added a benchmark workflow as well, super basic, and can post to the PR, I think you just need to enable it in your settings. Would give a nice indication if a PR is slowing the code down etc.

dimdenGD commented 1 week ago

Why do {...new NullObject()}? It's like creating a fast object and then creating a slow one and putting it inside. Would be faster to just have {} at this point

Pho3niX90 commented 1 week ago

Why do {...new NullObject()}? It's like creating a fast object and then creating a slow one and putting it inside. Would be faster to just have {} at this point

My initial thoughts. But ran some benchmarks, and seems it's faster like this. See if you get the same results from it?

dimdenGD commented 1 week ago

I don't see how it's possible for it to be faster that way. Microbenchmarks are often deceptive

Pho3niX90 commented 1 week ago

So these are the results (tests/sample/object_creation.js):

Min is not the lowest, but more consistent than the spread.

image

But I am fine with either. Should I change it back to {}

dimdenGD commented 1 week ago

You should probably test some reads+writes too. Also there's no point in using these spreads for internal stuff only used by library, only for public things like req.query and req.params