Skip to content

improve codebase and performance #41

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.

Sign up for GitHub

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

Jump to bottom
Merged
mcollina merged 8 commits into from
Jul 7, 2022
Merged

improve codebase and performance #41

mcollina merged 8 commits into from
Jul 7, 2022

Conversation

@Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jul 6, 2022 •
edited
Loading

before:

  no header   x 9,918,143 ops/sec ±0.41% (193 runs sampled)
  1 address   x 6,509,341 ops/sec ±0.59% (190 runs sampled)
  2 addresses x 3,776,782 ops/sec ±1.24% (188 runs sampled)
  5 addresses x 2,228,356 ops/sec ±0.28% (181 runs sampled)

after:

no header   x 1,246,115,625 ops/sec ±0.24% (196 runs sampled)
  1 address   x    66,139,899 ops/sec ±0.91% (193 runs sampled)
  2 addresses x    10,107,089 ops/sec ±0.34% (195 runs sampled)
  5 addresses x     4,374,716 ops/sec ±0.48% (194 runs sampled)

Uzlopak
Uzlopak commented Jul 6, 2022
View reviewed changes
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 6, 2022

I was actually investigating if i could improve the original package of dougwilson. Then i saw you already created a fork. I hope you like it...

@Uzlopak Uzlopak changed the title improve performance improve codebase and performance Jul 6, 2022
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 6, 2022

@Fdawgs
@mcollina
@Eomm
@kibertoad

Fdawgs
Fdawgs approved these changes Jul 7, 2022
View reviewed changes
Copy link
Member

@Fdawgs Fdawgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disgusting, lgtm

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 7, 2022

@Fdawgs

Thank you xD. I added some unit tests for some edge cases, just to be on the safe side. Please just run the workflows again ;)

I even had a faster implementation, but that would have been a security risk, as it would use recursion. And Forwarded-For is a security header, so I didnt wanted that somebody could raise a MaxStackCall Error and poitentially crashing the service.

@mcollina mcollina merged commit 8d2224e into fastify:master Jul 7, 2022
@Uzlopak Uzlopak deleted the high-perf branch July 7, 2022 11:02
@Uzlopak Uzlopak mentioned this pull request Sep 11, 2023
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fdawgs Fdawgs Fdawgs approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Uzlopak @Fdawgs @mcollina