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

remove redundant isNaN check and add fast path for empty options #264

Merged
merged 14 commits into from
Dec 7, 2023

Conversation

gurgunday
Copy link
Member

@gurgunday gurgunday commented Nov 24, 2023

isFinite already covers NaN

Also added a fast path for the case where options is undefined

Benchmark benchmark/cookie

Before:

Running 10s test @ http://127.0.0.1:5001
10 connections


┌─────────┬──────┬──────┬───────┬──────┬─────────┬─────────┬───────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%  │ Avg     │ Stdev   │ Max   │
├─────────┼──────┼──────┼───────┼──────┼─────────┼─────────┼───────┤
│ Latency │ 0 ms │ 0 ms │ 0 ms  │ 0 ms │ 0.01 ms │ 0.12 ms │ 20 ms │
└─────────┴──────┴──────┴───────┴──────┴─────────┴─────────┴───────┘
┌───────────┬─────────┬─────────┬─────────┬─────────┬──────────┬──────────┬─────────┐
│ Stat      │ 1%      │ 2.5%    │ 50%     │ 97.5%   │ Avg      │ Stdev    │ Min     │
├───────────┼─────────┼─────────┼─────────┼─────────┼──────────┼──────────┼─────────┤
│ Req/Sec   │ 35,807  │ 35,807  │ 49,311  │ 52,735  │ 47,676.8 │ 5,391.76 │ 35,785  │
├───────────┼─────────┼─────────┼─────────┼─────────┼──────────┼──────────┼─────────┤
│ Bytes/Sec │ 7.77 MB │ 7.77 MB │ 10.7 MB │ 11.4 MB │ 10.3 MB  │ 1.17 MB  │ 7.77 MB │
└───────────┴─────────┴─────────┴─────────┴─────────┴──────────┴──────────┴─────────┘

Req/Bytes counts sampled once per second.
# of samples: 10

477k requests in 10.01s, 103 MB read

Now:

Running 10s test @ http://127.0.0.1:5001
10 connections


┌─────────┬──────┬──────┬───────┬──────┬─────────┬─────────┬───────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%  │ Avg     │ Stdev   │ Max   │
├─────────┼──────┼──────┼───────┼──────┼─────────┼─────────┼───────┤
│ Latency │ 0 ms │ 0 ms │ 0 ms  │ 0 ms │ 0.01 ms │ 0.09 ms │ 20 ms │
└─────────┴──────┴──────┴───────┴──────┴─────────┴─────────┴───────┘
┌───────────┬────────┬────────┬─────────┬─────────┬───────────┬─────────┬────────┐
│ Stat      │ 1%     │ 2.5%   │ 50%     │ 97.5%   │ Avg       │ Stdev   │ Min    │
├───────────┼────────┼────────┼─────────┼─────────┼───────────┼─────────┼────────┤
│ Req/Sec   │ 43,327 │ 43,327 │ 49,823  │ 53,279  │ 49,444.37 │ 2,829.3 │ 43,320 │
├───────────┼────────┼────────┼─────────┼─────────┼───────────┼─────────┼────────┤
│ Bytes/Sec │ 9.4 MB │ 9.4 MB │ 10.8 MB │ 11.6 MB │ 10.7 MB   │ 615 kB  │ 9.4 MB │
└───────────┴────────┴────────┴─────────┴─────────┴───────────┴─────────┴────────┘

Req/Bytes counts sampled once per second.
# of samples: 11

544k requests in 11.01s, 118 MB read

cookie.js Outdated Show resolved Hide resolved
cookie.js Show resolved Hide resolved
@gurgunday
Copy link
Member Author

gurgunday commented Dec 7, 2023

Can you take a look?

Changes:

  • Don't create empty objects
  • Call regex tests only when string is not empty
  • Don't call isNaN when isFinite is called
  • Fast path for when options is undefined|null
  • Math.trunc is a little faster than .floor
  • +var has better bytecode than var - 0

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

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.

3 participants