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.

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

Upload file huge memory consumption #103

Open
delemercier opened this issue Nov 7, 2020 · 9 comments
Open

Upload file huge memory consumption #103

delemercier opened this issue Nov 7, 2020 · 9 comments

Comments

@delemercier
Copy link

Hello,

This web framework is amazing, we have to push it further to be prod ready !

I used the blog example to complete it with file uploading / downloading.

I made it work with the code at the bottom, but had issue with memory consumption. When uploading a 170MB zip file, the memory went up to 2.4GB for the app process (on windows). It was 10MB stable before.

It seems like the Context.getUploadFile() write the body as string. Is there a way to use a write stream to avoid using lots of memory?

The code used:

karax:

proc uploadSection*(ctx: Context, error: string = ""): VNode =
  result = buildHtml(main(class = "content")):
    h3: text "Upload"
    if error.len > 0:
      tdiv(class = "alert"):
        text error
    form(`method` = "post", enctype = "multipart/form-data"):
      label(`for` = "file"): text "Enter file"
      input(`type`= "file", name = "file")
      input(`type` = "submit")

Prologue view:

proc upload*(ctx: Context) {.async.} =
  if ctx.session.getOrDefault("userId").len != 0:
    if ctx.request.reqMethod == HttpGet:
      resp htmlResponse(uploadPage(ctx, "Upload"))
    elif ctx.request.reqMethod == HttpPost:
      let file = ctx.getUploadFile("file")
      file.save("uploaded")
      resp fmt"<html><h1>{file.filename} uploaded</h1></html>"

Thank you guys :)
@ringabout
Copy link
Member

First you could switch to -d:usestd in windows to reduce memory usage.

For the time being, both HTTP servers that Prologue supports don't provide streaming body.

nim-lang/Nim#13147 has tried to solve the problem of asynchttpserver, but has been reverted already.

And

ringabout/httpx#3

I will switch to https://github.com/iocrate/netkit in the future and hope to solve this problem.

@delemercier
Copy link
Author

Here are some tests for uploading a 105MB file:

  • nim c -r -d:release app.nim => 5 sec 1.4GB memory
  • nim c -r app.nim => 1min30sec 1.4GB memory
  • nim c -r -d:release -d:usestd => never ends with 12MB file

I added some timestamp:

    elif ctx.request.reqMethod == HttpPost:
      echo now()
      let file = ctx.getUploadFile("file")
      echo now()
      file.save(ctx.getSettings("uploadPath").getStr())
      echo now()
      resp fmt"<html><h1>{file.filename} uploaded</h1></html>"

Actually, the time spent is before the first echo now(). It seems the time is spent building the request, maybe encoding data into string.

@ITwrx
Copy link

ITwrx commented Nov 19, 2020

is this the same issue i'm seeing when serving any file from disk, such as an image via img tag or video via html5 video tag?

If i serve a 92k image in an html page, and load the page 5 times, the ram usage climbs from 3.4MB to 4.1MB.
5 more page loads and it has climbed to 5MB.
I'm seeing this in other nim web frameworks too, but i thought prologue was immune, b/c i didn't see any rise in mem usage with the hello world example. Serving files does the trick though.

Do i need to open another issue or is the issue i'm seeing caused by this body streaming issue?
thanks

@ringabout
Copy link
Member

ringabout commented Nov 19, 2020 •
edited
Loading

May be related to this issue. If serving same file, I think you could use some memory cache tables instead of reading files.
Or you could use nginx to serve static files which is more suitable for production.

@bung87
Copy link
Contributor

bung87 commented Nov 19, 2020

please use directory path match serve static file like common web application does, like public directory,media, static

@bung87
Copy link
Contributor

bung87 commented Nov 20, 2020

here is simple example https://github.com/bung87/nginx_supervisor_tornado/blob/master/etc/nginx/nginx.conf

@ITwrx
Copy link

ITwrx commented Nov 20, 2020

Thanks, i opened #106

@ITwrx
Copy link

ITwrx commented Dec 11, 2020

@xflywind so since nim-lang/Nim#13147 got reverted and they were concerned with breaking changes being introduced does that mean that nim web development will not have usable file upload capability until nim 2.0, or maybe until prologue moves to netkit? i'm trying to decide if i can use nim for some upcoming projects.

@jivank
Copy link
Contributor

jivank commented Feb 8, 2021

When using asynchttpserver, the problem is most likely this line of code:
https://github.com/nim-lang/Nim/blob/devel/lib/pure/asynchttpserver.nim#L288

It saves the entire file to the body string variable / in memory. It will be hard to fix this without breaking all the frameworks. Perhaps nim-lang/Nim#13147 where the socket doesn't read anything and defers to the framework can be changed to not break anything by default (by using a flag like -d:asynchttpserver2).

Another possibility that I think would be most backwards compatible is letting asynchttpserver parse the form if a multipart is detected, save any files to disk and replace the body content for the boundary with the filepath saved. This isn't ideal and is very nonstandard but since we have passed v1.0 it might suffice until a better solution is found.

@ITwrx ITwrx mentioned this issue Jun 5, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@ITwrx @bung87 @jivank @delemercier @ringabout