4
\$\begingroup\$

I have users uploading files sometimes as large as 100+ GB to a local web server. The upload process works well and chunks come in at 50MB. The problem seems to be after the file is uploaded, when the web server assembles the files and the server (24GB RAM), despite not showing any graphical signs of memory pressure, gets very sluggish. I want to make sure my code isn't causing any unnecessary slow-downs.

Please suggest any more efficient way to do it. If it is already ok, then I'll know to look at other aspects of the process.

# open the temp file to write into
with open(temp_filename, 'wb') as temp_file:
    # loop over the chunks
    for i in range(total_chunks):
        with open(os.path.join(get_chunk_filename(chunk_identifier, i + 1)), 'rb') as chunk_file:
            # write the chunk to the temp file
            temp_file.write(chunk_file.read())
\$\endgroup\$
0
6
\$\begingroup\$

I suggest you use existing library functions, e.g. shutil.copyfileobj to do the copying. Edit to clarify, as Gareth said: Use shutil.copyfileobj(chunk_file, temp_file) instead of temp_file.write(chunk_file.read()).

Other than that (allocating and reading into Python objects via chunk_file.read()) there's no obvious flaws with the code, but I/O in Python is to be avoided on that scale anyway. I'd even say you could try using a shell script with cat $FILES > $OUTPUT and it could perform better.

\$\endgroup\$
5
  • \$\begingroup\$ Hi, thanks. I'm not sure how I can use shutil.copyfileobj to assemble chunks. Obviously, I cannot avoid chunking the file on the browser side, so the server has to assemble the chunks somehow. So would you mind explaining what you mean? \$\endgroup\$ – MFB Oct 21 '15 at 22:42
  • 2
    \$\begingroup\$ ferada means that where you have temp_file.write(chunk_file.read()), try shutil.copyfileobj(chunk_file, temp_file) instead. (Take a look at the implementation — all it does is copy in 16 kilobyte blocks.) \$\endgroup\$ – Gareth Rees Oct 21 '15 at 22:44
  • \$\begingroup\$ Ok, but the chunk_file needs to be appended to the temp_file. shutil.copyfileobj(chunk_file, temp_file) will overwrite, no? \$\endgroup\$ – MFB Oct 22 '15 at 0:48
  • 1
    \$\begingroup\$ @MFB, no, it basically does the chunked loop and there's no indication that the file offset is reset anywhere, c.f. shutil.py, so it will use whatever previous position was there, i.e. it will append. \$\endgroup\$ – ferada Oct 22 '15 at 8:58
  • \$\begingroup\$ @ferada great, make sense \$\endgroup\$ – MFB Oct 22 '15 at 10:25
3
\$\begingroup\$

Your comments are pretty unnecessary. Python is a readable language so with open(temp_filename, 'wb') as temp_file is quite readable as English. You should use comments for giving more context to why you're doing something, explain the abstract intent behind code or clarify complicated and hard to stand things. Some stuff is less readable, like 'wb' being binary writeable mode. Or what the code as a whole does. Don't try to explain every single line, instead read over your code and think about what info is missing when you read bare code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.