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

Include pure-python versions of encode/decode #12

Closed
Julian opened this issue Apr 30, 2017 · 4 comments
Closed

Include pure-python versions of encode/decode #12

Julian opened this issue Apr 30, 2017 · 4 comments

Comments

@Julian
Copy link

Julian commented Apr 30, 2017

https://github.com/HdrHistogram/HdrHistogram_py/blob/master/src/python-codec.c includes a comment that pure python versions are too slow, but on PyPy, they'd very likely be preferred, and faster.

@ahothan
Copy link
Contributor

ahothan commented May 1, 2017

Julian, I had a pure python version of the codec, just need to dig it out from git history.
Reason I moved to C extensions is that python version was pretty slow, but perhaps with Pypy it will be good enough.
One possible hurdle is that the pure python codec only supported the V1 encoding (as the support for V2 happened around the time C was added), so might not be straightforward.

I'm really busy with other stuff so can't commit on when Pypy could be supported.
Perhaps somebody else can contribute.

@Julian
Copy link
Author

Julian commented May 1, 2017

Ah, awesome!

Definitely understood on being busy :)

I'm going to put some time in here one way or another, because we already use HdrHistogram_Py in production, so now I need to fix #13 one way or another (either by fixing BigEndianStructure in PyPy, removing it here, or stripping out ctypes entirely)

Seeing the old pure-python code will definitely help I think -- hopefully V1 and V2 are close enough to each other that it's not all wasted code?

@ahothan
Copy link
Contributor

ahothan commented May 1, 2017

Fix PyPy would be fantastic ;-)

@ahothan
Copy link
Contributor

ahothan commented Oct 27, 2017

Closing this as there is no more immediate need.

@ahothan ahothan closed this as completed Oct 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants