Skip to content

Increase the speed of request processing #253

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
Closed
shalyf wants to merge 2 commits into from
Closed

Increase the speed of request processing #253

shalyf wants to merge 2 commits into from

Conversation

@shalyf
Copy link

@shalyf shalyf commented Jul 18, 2017 •
edited
Loading

  1. Increase the speed of request processing.
    The old HttpParser.readBody reads the data very slowly, it is read byte by byte.
    The new HttpParser.readBody will try to read 1024 bytes at a time.

  2. file upload preprocess, to ensure that the transmission of large files when the memory security.
    I added a new bool property(filePreprocess) to HttpServerIO

     /// Bool representation of whether the file upload is preprocessed.
 /// `true` if the file upload requires preprocessing when `content-type` is
 /// `application/octet-stream`. `HttpParser` will create a temp file(`tempFile`) in
 /// `NSTemporaryDirectory()`, and is deleted after the request ends.
 /// Together, `body` will be empty.
 /// `false` otherwise.
 public var filePreprocess: Bool = false

    On the iPhone to deal with a large file upload, if all write to memory(HttpRequest.body), then the program is easy to receive a memory warning and crash.
    So I added the property to make it while reading from the socket, while writing to the temporary file.

    You can see the demo(/upload/logo) in DemoServer.swift.

shalyf and others added 2 commits July 17, 2017 16:39
@shalyf
readfile
931c6ab
@shalyf
Increase the speed of request processing
f0b0950 
1. Increase the speed of request processing
2. file upload preprocess, to ensure that the transmission of large files when the memory security
Macarse
Macarse reviewed Jul 25, 2018
View reviewed changes
@adamkaplan adamkaplan mentioned this pull request Jan 23, 2019
Merged
@adamkaplan
Copy link
Collaborator

adamkaplan commented Jan 28, 2019 •
edited
Loading

Part of this PR inspired #362, so the basic request processing is now faster. The remaining code in this PR adds capability to receiving large octet-stream uploads by piping them into a file. That is low priority for my use case so I only focused on really getting the request reading correct.

I think reading to file makes good sense. However I think most services are written to use small payloads in an ETL-style, where the data needs to be modified significantly prior to saving. In this case, writing to disk is not needed and a big hit to RPS.

tl;dr, I think that writing octet-streams to disk should only be done for payloads above a certain size in bytes (via Content-Length). That size should have a sensible default, like 1MB, and be configurable by users.

adamkaplan
adamkaplan reviewed Jan 28, 2019
View reviewed changes
Sources/HttpParser.swift
return
}
let isFileUpload = contentType == "application/octet-stream"
if isFileUpload && filePreprocess {
Copy link
Collaborator

@adamkaplan adamkaplan Jan 28, 2019

Choose a reason for hiding this comment

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

Instead of or in addition to filePreprocess the data should go to disk is it's pretty big, so that the server doesn't run into memory exhaustion issues. The written file should be handed back to the caller instead of just closed, and it should be memory-mapped so that it can be efficiently processed by the caller.

@shalyf
Copy link
Author

shalyf commented Jan 30, 2019

OK, filePreprocess is not very important, it's only used in my iOS projects.
Thank @adamkaplan.

@shalyf shalyf closed this Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamkaplan adamkaplan adamkaplan left review comments

+1 more reviewer

@Macarse Macarse Macarse left review comments

Reviewers whose approvals may not affect merge requirements

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

@shalyf @adamkaplan @Macarse