@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).
@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
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?
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.
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?
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.
@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*.
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.
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).
@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.
@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.
@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?
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.
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.
@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...
@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.
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).