Upload file huge memory consumption

delemercier commented 3 years ago

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 commented 3 years ago

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.

https://github.com/nim-lang/Nim/pull/13147 has tried to solve the problem of asynchttpserver, but has been reverted already.

And

https://github.com/xflywind/httpx/issues/3

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

delemercier commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

bung87 commented 3 years ago

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

ITwrx commented 3 years ago

Thanks, i opened #106

ITwrx commented 3 years ago

@xflywind so since https://github.com/nim-lang/Nim/pull/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 commented 3 years ago

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 https://github.com/nim-lang/Nim/pull/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.

planety / prologue

Upload file huge memory consumption #103