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.

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

small optimization of BytesIOPayload.size #2129

Merged
merged 1 commit into from
Jul 26, 2017

Conversation

arthurdarcet
Copy link
Contributor

@arthurdarcet arthurdarcet commented Jul 25, 2017

When posting a large body that you have as a bytes object, as per #2127 , the body should be passed as a io.BytesIO object.

But when this BytesIO is initialised on a bytes object that might be used somewhere else, calling getbuffer needs to copy the underlying data to return a writable memoryview.

python -m timeit -s "import io; a = b'0' * 2 ** 30" 'io.BytesIO(a).getbuffer()'
10 loops, best of 3: 627 msec per loop
python -m timeit -s "import io; a = b'0' * 2 ** 30" 'io.BytesIO(a).getvalue()'
1000000 loops, best of 3: 0.247 usec per loop
python -m timeit -s "import io; a = b'0' * 2 ** 30" 'io.BytesIO(a)'
10000000 loops, best of 3: 0.186 usec per loop

(the instantiating the BytesIO in the loop to avoid the caching done by BytesIO)

This is problematic beyond wasting 500ms when making a request: the event loop is actually locked during this memory copy, so an underlying aiohttp server won't be able to process any other pending requests.

But, calling getvalue instead of getbuffer will make some (weird) use-cases way worse than now:

python -m timeit -s "import io; d = io.BytesIO(b'0' * 2 ** 30)" i = d.getbuffer()" 'd.getvalue()'
10 loops, best of 3: 624 msec per loop

Calling seek(0, SEEK_END) avoid all this

@codecov-io
Copy link

codecov-io commented Jul 25, 2017

Codecov Report

Merging #2129 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2129      +/-   ##
==========================================
+ Coverage   97.07%   97.07%   +<.01%     
==========================================
  Files          38       38              
  Lines        7720     7723       +3     
  Branches     1346     1346              
==========================================
+ Hits         7494     7497       +3     
  Misses        102      102              
  Partials      124      124
Impacted Files Coverage Δ
aiohttp/payload.py 98.86% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 153197f...f3b3f17. Read the comment docs.

@fafhrd91
Copy link
Member

lgtm

@asvetlov comments?

@asvetlov
Copy link
Member

looks good

@asvetlov asvetlov merged commit 91dc5b7 into aio-libs:master Jul 26, 2017
@asvetlov
Copy link
Member

Thank you

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants