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

Update body parsing and file streaming exemples #1128

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ratchetsniper2
Copy link

As a newcomer, these examples were difficult to understand and adapt to my needs.
There were missing cork() calls and unoptimized code.
So I share these complete examples that are simpler and more understandable.

@uNetworkingAB
Copy link
Contributor

Don't remove examples. Esp. not when the substitute is slower:

alexhultman@MacBookAir uSockets % ./http_load_test 256 localhost 9001 16 1
Using pipeline factor of 16
Using post with body
request size 1744
Running benchmark now...
Req/sec: 508488.000000
Req/sec: 510892.000000
Req/sec: 511276.000000
Req/sec: 513292.000000

alexhultman@MacBookAir uSockets % ./http_load_test 256 localhost 9001 16 1
Using pipeline factor of 16
Using post with body
request size 1744
Running benchmark now...
Req/sec: 482788.000000
Req/sec: 489336.000000
Req/sec: 489724.000000

It might look as "unoptimized code" to you, but it's not. Sure, there is a missing call to cork(), so you could add that call.

@ratchetsniper2
Copy link
Author

I updated the body parsing example, you should get better performance.
"unoptimized code" was maybe not the right word, I was talking about the size of the code.
As these are examples, I think it’s important that they be easily understandable.

About changes:
Rewriting "JsonPost.js" to "ParseJsonOrFormBody.js" and add URL-encoded form body parsing example.
Merging and rewriting "VideoStreamer.js" and "VideoStreamerSync.js" to "FileStreaming.js".

@uNetworkingAB
Copy link
Contributor

I was talking about the size of the code.
As these are examples, I think it’s important that they be easily understandable.

We don't optimize for size, we optimize for speed. And examples should show the best way for speed. If you want to optimize for size, use Express.

@ratchetsniper2
Copy link
Author

We don't optimize for size, we optimize for speed. And examples should show the best way for speed. If you want to optimize for size, use Express.

I see, but it's not really newcomer friendly, the body parsing example was the one that makes me doubt to use uWS.js, it makes it look so hard just to parse the body.
In any case, the updated examples should have the same performances while being simpler.

If performance is realy the main point and as parsing request body is an important feature, we could add a parseBody or onParsedBody function directly on the HttpResponse c++ wrapper.
So Node.js would only have to process the final fulfilled ArrayBuffer.
And we use the onData function for uploading or client side streaming.

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.

2 participants