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

Fun & Games With Endianness #2

Closed
Julian opened this issue Apr 27, 2017 · 6 comments
Closed

Fun & Games With Endianness #2

Julian opened this issue Apr 27, 2017 · 6 comments

Comments

@Julian
Copy link

Julian commented Apr 27, 2017

Hello!

I'm unfortunately not quite sure where this bug belongs, because I don't know the details of the spec used to encode HDRHistograms, but someone's got some little endian in my big endian :) -- if this belongs in the other repo, lemme know.

The following is a trivial HDRHistogram encoded via HdrHistogram/HdrHistogram_Py (via from hdrh.histogram import HdrHistogram; print HdrHistogram(1, 2, 3).encode():

FJOEHBgAAAB4nBOe3CLDAAXMQMwIZTNBaRgfADG9AU4=

But decoding that with HdrHistogramJS raises an error saying that only V2 encoding is supported. Reading the code in HdrHistogram_Py shows it's definitely trying to do V2, whatever that is, and stepping through HdrHistogramJS' code, it's pretty clear that's what happening is that it's interpreting the byte buffer as big endian, but HdrHistogram_Py's output is little endian, so it blows up calculating even just the histogram's header.

Who's right :)?

@Julian
Copy link
Author

Julian commented Apr 27, 2017

CC @ahothan

@ahothan
Copy link

ahothan commented Apr 28, 2017

Could you try to decode with JS another of the pre-built base64 histograms generated by Java or C version?
You could use any of those in this log for example:
https://github.com/HdrHistogram/HdrHistogram/blob/master/src/test/resources/org/HdrHistogram/jHiccup-2.0.7S.logV2.hlog

@Julian
Copy link
Author

Julian commented Apr 28, 2017

@ahothan selecting a few random ones, they all decode successfully with HdrHistogramJS but not with HdrHistogram_Py.

Also just visually inspecting the outputted b64, all of those start with HIST, but histograms from HdrHistogram_Py do not.

Seeming pretty likely that this bug belongs on HdrHistogram_Py then?

@ahothan
Copy link

ahothan commented Apr 29, 2017

Did not notice your output was missing the starting 'HIST'
What version of HdrHistogram_py do you have?

I don't see any issue with latest.

`(vhdr)AHOTHAN-M-M03U:HdrHistogram_py ahothan$ python
Python 2.7.10 (default, Feb 6 2017, 23:53:20)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

from hdrh.histogram import HdrHistogram
histogram = HdrHistogram(1,1000,3)
histogram.encode()
'HISTFAAAABd4nJNpmSzMgADMUJoRyn0B4wMAOdgCNw=='
hh=HdrHistogram(1,2,3)
hh.encode()
'HISTFAAAABZ4nJNpmSzMgADMUJoRSjPB+AAxpAFO'
`

@Julian
Copy link
Author

Julian commented Apr 29, 2017

A ha. Thanks, that points us in the right direction. This is going to be something to do with CPython vs PyPy, there's either some implementation specific behavior in HdrHistogram_Py or PyPy has a bug in something HdrHistogram_Py is using. I did see there's some ctypes code in HdrHistogram_Py which I was looking to remove for other reasons, but will have to read through the code a bit more carefully. (I can confirm that if I use CPython I get your output).

@Julian
Copy link
Author

Julian commented Apr 30, 2017

Moving this over to HdrHistogram/HdrHistogram_py#13

@Julian Julian closed this as completed Apr 30, 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