These are chat archives for HdrHistogram/HdrHistogram

5th
Aug 2015
Michael Barker
@mikeb01
Aug 05 2015 04:00 UTC
@giltene I've moved over to using an index directly only the counts, fairly simple change and removes the need to track 2 fields on the iterator. I'm going to look at auto-resizing next. I'd already done the autoshifting to support doubles. Eventually I'll get the double histogram done (just need to write the double iterators).
Michael Haberler
@mhaberler
Aug 05 2015 05:40 UTC
@giltene - filed HdrHistogram/HdrHistogram_c#14
Michael Barker
@mikeb01
Aug 05 2015 07:32 UTC
@mhaberler Thanks I'll have a look. I notice you just had results from i586 and amd64. I assume that the arm7 also crashes in the same way?
Michael Haberler
@mhaberler
Aug 05 2015 08:37 UTC
@mikeb01 - yes; added the armv7l output to the issue
Michael Barker
@mikeb01
Aug 05 2015 08:39 UTC
Cheers, just got a i386 vm installed. I'll start digging. I suspect it is something to do with running on 32bit.
Michael Haberler
@mhaberler
Aug 05 2015 08:41 UTC
happy to test in my platform bestiary ;)
Michael Barker
@mikeb01
Aug 05 2015 08:50 UTC
No problem, I've replicated the issues.
Michael Haberler
@mhaberler
Aug 05 2015 08:51 UTC
excellent. Seems to be minor, pointer size or somesuch.
Michael Barker
@mikeb01
Aug 05 2015 08:51 UTC
I think there's an integer overflow in the test setup.
Michael Haberler
@mhaberler
Aug 05 2015 08:53 UTC
there are some clang flags which might help testing for that, see 'integer-overflow' in http://clang.llvm.org/docs/UsersManual.html
Michael Barker
@mikeb01
Aug 05 2015 08:54 UTC
I hadn't compiled in on 32 bit before and just now gcc spit out a bunch of warnings, so I'll fix those first and see how I get on.
Michael Haberler
@mhaberler
Aug 05 2015 08:56 UTC
oh, you're the author of disruptor? hats off!
Michael Barker
@mikeb01
Aug 05 2015 08:57 UTC
One of them.
Michael Barker
@mikeb01
Aug 05 2015 09:04 UTC
Thank you.
Michael Barker
@mikeb01
Aug 05 2015 09:31 UTC
Fixed 2 so far, just issues with the tests at the moment.
Michael Barker
@mikeb01
Aug 05 2015 09:48 UTC
@mhaberler I have all of the test passing on my 32 bit VM. I haven't tested on ARM though. If you get a chance to test it would be appreciated. Otherwise I'll dig out my Raspberry PI tomorrow.
Michael Haberler
@mhaberler
Aug 05 2015 10:34 UTC
no I can cover arm. Results added, test1 failed on armv7l/gcc4.9.2
Michael Barker
@mikeb01
Aug 05 2015 10:51 UTC
@mhaberler Have a go now. I've removed the failing test case, it's not particularly useful.
Warnings should be fixed too.
Michael Haberler
@mhaberler
Aug 05 2015 11:31 UTC
yes, works fine on several ARMv7l's now
rate: BB Black: Iteration - 1, ops/sec: 2,873,900.04
cubox-i Iteration - 1, ops/sec: 3,612,192.52
rpi2 Iteration - 1, ops/sec: 2,714,977.90
@mikeb01 - I will run this in an RT environment where malloc/free is not available but under a different name
I'll eventually post a PR to make malloc/free etc macros which can be replaced compile-time
Michael Haberler
@mhaberler
Aug 05 2015 11:44 UTC
Q on usage: I see routines to write/read histos
I would be interested in displaying live data, not snapshots, by a separate thread/python program attaching shared memory - is that feasible in principle? or maybe even an intended use case?
Michael Barker
@mikeb01
Aug 05 2015 11:55 UTC
The closes thing is the interval recorder, which is used in the hiccup.c example. It uses a double-buffer like approach where you sample the data by exchanging 2 histograms. By clearing the histogram being read after use, you can effectively stream histograms between threads/processes each representing a fixed period, e.g. once a second. You can also aggregate those histograms later to get a view over longer periods of time.
Michael Haberler
@mhaberler
Aug 05 2015 12:01 UTC
I see - well that's fine. Need to get the hang of the API but no showstoppers yet ;)
Alec
@ahothan
Aug 05 2015 16:57 UTC
@mikeb01 : trying to upgrade the wrk2 code to use the latest version of HdrHistogram_c, 2 questions.
  1. use of recorder API vs. manual resetting the histograms
  2. suggestion for hdr_histogram_log API extension
Alec
@ahothan
Aug 05 2015 17:08 UTC
  1. the current modified wrk2 code implements multiple threads, each with its own histogram, and a service thread kicks in every interval to do this for each histogram: lock the mutex associated to histogram, take a snapshot (encode the histogram), reset the histogram, unlock (this mutex is used to protect every record_value())
    Normally with the recorder API this would not be necessary (use hdr_interval_recorder_sample() to get the new inactive histogram then encode that histogram). Is there any reason to use the recorder instead of the current crude but simple implementation?
Alec
@ahothan
Aug 05 2015 17:17 UTC
  1. int hdr_log_write(
    struct hdr_log_writer writer,
    FILE
    file,
    const struct timespec start_timestamp,
    const struct timespec
    end_timestamp,
    struct hdr_histogram* histogram);
    In my case I'd need to write the base64 encoded string in a different place than a FILE (I was thinking using another wrapper that could be JSON as I need to add other fields specific to wrk2).
    Would it be possible for example to add a function that returns only the encoded data in V1 format given a histogram ptr and one to free it when the caller is done with it? If you're ok I can help and do a pull request.
Michael Barker
@mikeb01
Aug 05 2015 20:30 UTC
@ahothan I believe the integration in wrk2 is the way it is because @giltene did the implementation before I had ported the interval recorder. It would be a nice update to move to the interval recorder in wrk2.
Have a look at the hdr_log_encode, hdr_log_decode methods in hdr_histogram_log.h. These will allow you to encode/decode to a char*.
Gil Tene
@giltene
Aug 05 2015 21:33 UTC
@mikeb01 [re: rewrite in your future] Based on some movies I've watched recently, Katanas seem to be good for efficiently separating heads from bodies. ;-)
Michael Barker
@mikeb01
Aug 05 2015 21:36 UTC
@giltene Sounds like a plan. On a side note, if I use realloc to shrink some allocated memory, will it guarantee that the memory won't be moved?
Todd L. Montgomery
@tmontgomery
Aug 05 2015 21:36 UTC
no. It can be moved
Michael Barker
@mikeb01
Aug 05 2015 21:36 UTC
Damn, there goes my cunning plan.
Todd L. Montgomery
@tmontgomery
Aug 05 2015 21:37 UTC
it often isn't. However, some allocators take the opportunity to collalesce on a collapsing realloc()
Gil Tene
@giltene
Aug 05 2015 21:38 UTC
Were you going to auto-downsize?
Michael Barker
@mikeb01
Aug 05 2015 21:40 UTC
I was going to allocate a contiguous block on the first allocation so that if you didn't have a resize you'd still get the flat layout, then on resize realloc to shrink the memory down to just include the header then allocate a new larger body. I'll probably just keep it simple and do 2 allocations.
Gil Tene
@giltene
Aug 05 2015 21:44 UTC
Ahhh.. Sneaky. But you'd still have either a de-ref or a conditional even when you get the flat version, right? Have you thought of some way around that one?
Michael Barker
@mikeb01
Aug 05 2015 21:46 UTC
Nope, was just going to suck up the cost of a de-ref, but the address would be pointing to memory directly adjacent to it.
Gil Tene
@giltene
Aug 05 2015 21:49 UTC
That probably wouldn't be much of a win, since the likelihood of hitting the same cache line as the ref is very low (only on recording into the first handful of sub-buckets). All other value recordings will be de-reffing and missing the same way whether you were flat or not (as long as code has to deal with both being a possibility on the same struct).
Michael Barker
@mikeb01
Aug 05 2015 21:51 UTC
I think you're right, but it would of been fun to get working.
Alec
@ahothan
Aug 05 2015 22:09 UTC
@mikeb01 ok somehow I missed hdr_log_encode( ), it is exactly what I need.
Regarding wrk2, my version will simply require a pre-install of HdrHistogram_c to build (I think that is better than copying over the source code as it is done in wrk2 today). It is working on MacOSX (cmake/make install) and I will test on Ubuntu as well.
Actually the original wrk2 does not do interval reporting of latencies, this is a wrk2 feature my team mate has added. We have not requested a pull for this work but we'd be happy to if there is interest from @giltene.
Michael Barker
@mikeb01
Aug 05 2015 22:14 UTC
@ahothan The build will generate both shared and static libraries, so you could use those instead of instead of a pre-install. I haven't started work on packaging (RPM, deb, etc) getting it pre-installed may be difficult.
Alec
@ahothan
Aug 05 2015 22:26 UTC
@mikeb01 standard distro package (rpm, debian, mac dmg) would be ideal but not available yet. I thought cmake could handle those?
cmake + "make install" on mac will install under /usr/local/lib and include, which works fine with the wrk2 build, why do you say it may be difficult?
Michael Barker
@mikeb01
Aug 05 2015 22:29 UTC
Think about a user having to install wrk2, they would have to download, build and install hdr_histogram first. They wouldn't know if the versions would match up. I.e. which versions of wrk2 have been tested with which versions of HdrHistogram. Copying in the library into the project, while a little bit clunky does solve a bunch of these types of issues.
Alec
@ahothan
Aug 05 2015 22:32 UTC
I see your point, but I don't like the idea of committing binaries in git. Are you expecting backward compatibility problems moving forward? The API looks pretty stable to me. In the unlikely case where there is a compatibility break we can always address this by specifying a version requirement?
In my case, I will build+install hdrhistogram and wrk2 in a Ubuntu based VM that will subsequently be used to create an image, which can then run in the cloud. So no problem of version mismatch there.
Michael Barker
@mikeb01
Aug 05 2015 22:33 UTC
I don't want to commit to a fixed API yet.
Alec
@ahothan
Aug 05 2015 22:35 UTC
fair enough, libraries in the wrk2 project will be very hard to maintain (not mentioning you need a version for each target OS), you might as well copy the source code if it comes to that
Todd L. Montgomery
@tmontgomery
Aug 05 2015 22:37 UTC
if using CMake, you can bring in HdrHistogram_c as an external project via git if you wish
Alec
@ahothan
Aug 05 2015 22:40 UTC
@tmontgomery wrk2 is a straight 100-line Makefile while HdrHistogram_c uses cmake, are you suggesting to convert wrk2 to cmake?
If there was a versioning/tagging in HdrHistogram_c that can be used for version compatibility that could be a first step...
Todd L. Montgomery
@tmontgomery
Aug 05 2015 22:41 UTC
wrk2 would probably be a shorter CMakeLists.txt file then. But dependency on cmake might be undesirable to some.
in terms of tagging, I think you might be able to do it based on commit hash...
Alec
@ahothan
Aug 05 2015 22:43 UTC
Best is to use git tags, as everybody does, and use standard version formats 0.0 or 0.0.0
from that point it is easier to say "use repo X with version <= 2.0.0"
Todd L. Montgomery
@tmontgomery
Aug 05 2015 22:44 UTC
indeed. But until @mikeb01 tags it as a release....
Michael Barker
@mikeb01
Aug 05 2015 22:45 UTC
I would consider tagging it as a 0.1.
Alec
@ahothan
Aug 05 2015 22:45 UTC
yep ;-) I see the java version has 20 releases (!)
Michael Barker
@mikeb01
Aug 05 2015 22:46 UTC
Gil's a bit of a machine...
Pushed HdrHistogram_c-0.0.1.
Todd L. Montgomery
@tmontgomery
Aug 05 2015 22:49 UTC
cool! thanks, @mikeb01
Alec
@ahothan
Aug 05 2015 22:49 UTC
need to celebrate this milestone ;-) !
Todd L. Montgomery
@tmontgomery
Aug 05 2015 22:49 UTC
now I can update Aeron to use that as well
Gil Tene
@giltene
Aug 05 2015 22:50 UTC
@mikeb01 I'd look to move it to 1.0.1 pretty quickly. IMO the C code has been pretty good for a while now. Improvement and API changes under newer version tags are not a big problem as long as people can get to a stable older tag.
Michael Barker
@mikeb01
Aug 05 2015 22:51 UTC
@giltene My only problem is that people would assume that it is "complete", at the moment the double histogram is only halfway implemented.
And I'm not sure that I'm ready to fix the API yet.
I want to get the double histogram iterators in place, which should let me know if I need to change anything significant.
Gil Tene
@giltene
Aug 05 2015 22:53 UTC
DoubleHistogram wasn't added to the Java versions until 2.x
I think your current state is already ahead of where Java was at 1.0.01
Michael Barker
@mikeb01
Aug 05 2015 22:56 UTC
I'm hoping to implement the double iterators on top of the current ones. If I can get some of that done to the point that I know that I won't need to change the API for it happen, I'll release 1.0 (without a full double histogram implementation).
Gil Tene
@giltene
Aug 05 2015 22:56 UTC
:-)
Michael Barker
@mikeb01
Aug 05 2015 22:57 UTC
I think that auto-resizing will be post 1.0 as well.