Skip to content

Use native bytearray truncation - #120

Closed
lovelydinosaur wants to merge 6 commits into
python-hyper:masterfrom
lovelydinosaur:native-bytearray-truncation
Closed

Use native bytearray truncation#120
lovelydinosaur wants to merge 6 commits into
python-hyper:masterfrom
lovelydinosaur:native-bytearray-truncation

Conversation

@lovelydinosaur

Copy link
Copy Markdown
Contributor

Switching from .compress() to using native buffer truncation.

I figure despite some related work on #115 it's worth looking at this PR in isolation.

We probably want this to be blocked on #116.

Benchmarking on Python 3.7...

Before:

$ PYTHONPATH=. venv/bin/pythonbench/benchmarks/benchmarks.py6901.9requests/sec7055.0requests/sec7084.2requests/sec7070.6requests/sec7107.9requests/sec7075.8requests/sec7079.4requests/sec
$ PYTHONPATH=. venv/bin/pythonbench/benchmarks/benchmarks.py6972.9requests/sec6996.9requests/sec6947.3requests/sec6999.9requests/sec7040.6requests/sec7030.0requests/sec6997.6requests/sec

After:

$ PYTHONPATH=. venv/bin/pythonbench/benchmarks/benchmarks.py7054.6requests/sec7133.3requests/sec7148.3requests/sec7116.4requests/sec7121.0requests/sec7128.2requests/sec7156.5requests/sec
$ PYTHONPATH=. venv/bin/pythonbench/benchmarks/benchmarks.py7018.9requests/sec7074.7requests/sec7075.0requests/sec7070.6requests/sec7028.2requests/sec7118.3requests/sec7131.2requests/sec

@lovelydinosaur

Copy link
Copy Markdown
ContributorAuthor

We also ought to follow up on the action in the comment "make sure PyPy also has the optimization", but I'm not sure where to start with that.

Comment threadh11/_receivebuffer.py
# bytearray is amortized O(n), thanks to some excellent work by Antoine
# Martin:
#
# https://bugs.python.org/issue19087

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be helpful to keep the paragraph above.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, yes. I've addressed it as an inline comment.

Comment threadh11/_receivebuffer.py Outdated
if not out:
return None
self._start += len(out)
self._data[:count] = b""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a del would be a little clearer but maybe it's just me. (Also below).

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, yup!

Comment threadh11/_receivebuffer.py Outdated
@bluetech

Copy link
Copy Markdown
Contributor

We also ought to follow up on the action in the comment "make sure PyPy also has the optimization", but I'm not sure where to start with that.

With this code:

size=50*2**20b=bytearray(size)
foriinrange(size):
delb[0]

Results:

python3.8: 4.257s
pypy3 version 7.3.2: 0.326s
pypy2 version 7.3.2: 0.326s
python2.7: DNF

So it's either that pypy has this optimization, or it's doing some magic JIT handling of this specific code. I tried some tricks to foil any such optimization and it seems legit.

If we're willing to have a timing-based unit test for this, a size = 3 * 2**20 with a timeout of 5s would do the trick IMO, even with a super fast CPU running the quadratic case or a very slow CPU running the linear case. But it will probably be flaky anyway...

Comment threadh11/_receivebuffer.py Outdated
Co-authored-by: Ran Benita <ran@unusedvar.com>
@njsmith

Copy link
Copy Markdown
Member

I think this has been superseded by #115

@pgjones

Copy link
Copy Markdown
Member

Agree with #115 superseding this. (Please reopen if not).

@pgjonespgjones closed this Dec 26, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@lovelydinosaur@bluetech@njsmith@pgjones