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

Unbuffered reading / writing #2909

Closed
benjaoming opened this Issue Jan 29, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@benjaoming
Member

benjaoming commented Jan 29, 2015

Incidentally, I just merged this in django-dbbackup... it's about writing in chunks: https://bitbucket.org/mjs7231/django-dbbackup/commits/c7e0588c3cc5acd3a96bf7708871edb025b859bc

The problem is what seems to be a common pattern in kalite: Doing read() calls without any particular concern of filesize. This operation will load a whole file into memory. But the thing is that we don't know the memory of the target system. In case of Sandisks, Arduinos, Raspberries etc.. it can become problematic.

I suggest having a look at all occurrences of read\(.*\). I haven't seen examples of the same with write(unbuffered_data) but it could be a good idea to investigate, too.

@aronasorman

This comment has been minimized.

Show comment
Hide comment
@aronasorman

aronasorman Jan 29, 2015

Member

I agree. Since we're gonna do a whole bunch of replacements to solve this issue, the first step would be to make a function that accepts a file-like object and returns a generator for the chunks of each file.

Member

aronasorman commented Jan 29, 2015

I agree. Since we're gonna do a whole bunch of replacements to solve this issue, the first step would be to make a function that accepts a file-like object and returns a generator for the chunks of each file.

@aronasorman aronasorman self-assigned this Jan 29, 2015

@aronasorman aronasorman added this to the 0.14.x milestone Jan 29, 2015

@benjaoming

This comment has been minimized.

Show comment
Hide comment
@benjaoming

benjaoming Jan 29, 2015

Member

That's a good idea. That way, depending on the device, we can have a global setting for buffer size.

Member

benjaoming commented Jan 29, 2015

That's a good idea. That way, depending on the device, we can have a global setting for buffer size.

@aronasorman aronasorman modified the milestones: 0.15.0, 0.14.0 May 26, 2015

@aronasorman aronasorman modified the milestones: 0.16.0, 0.15.0 Sep 1, 2015

@benjaoming benjaoming removed the performance label Oct 2, 2015

@rtibbles

This comment has been minimized.

Show comment
Hide comment
@rtibbles

rtibbles Feb 1, 2016

Member

This should be less of an issue with content packs now. Punting.

Member

rtibbles commented Feb 1, 2016

This should be less of an issue with content packs now. Punting.

@rtibbles rtibbles modified the milestones: 0.17.0, 0.16.0 Feb 1, 2016

@shreab373

This comment has been minimized.

Show comment
Hide comment
@shreab373

shreab373 Jan 28, 2017

Hi, can I take this? Could you tell me what exactly needs to be done to solve this?

shreab373 commented Jan 28, 2017

Hi, can I take this? Could you tell me what exactly needs to be done to solve this?

@benjaoming benjaoming modified the milestones: 0.17.0, 0.17.1 Mar 19, 2017

@benjaoming

This comment has been minimized.

Show comment
Hide comment
@benjaoming

benjaoming Apr 7, 2017

Member

retrievecontentpack only uses calls to shutil.copyfileobj and zipfile.extractall

Our video download writes in chunks in fle_utils.internet.download.download_file

The only problematic thing remaining could be this check on file size from download_video in fle_utils.video:

not len(open(filepath, "rb").read()) == int(response.headers['content-length']))
Member

benjaoming commented Apr 7, 2017

retrievecontentpack only uses calls to shutil.copyfileobj and zipfile.extractall

Our video download writes in chunks in fle_utils.internet.download.download_file

The only problematic thing remaining could be this check on file size from download_video in fle_utils.video:

not len(open(filepath, "rb").read()) == int(response.headers['content-length']))
@benjaoming

This comment has been minimized.

Show comment
Hide comment
@benjaoming

benjaoming Apr 10, 2017

Member

Fixed in #5436

Member

benjaoming commented Apr 10, 2017

Fixed in #5436

@benjaoming benjaoming closed this Apr 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment