HdrHistogram / HdrHistogramJS

TypeScript port of HdrHistogram
BSD 2-Clause "Simplified" License
123 stars 21 forks source link

Add `minValue` to get the actual minimum value #11

Open rochdev opened 5 years ago

rochdev commented 5 years ago

Right now there is a maxValue and a minNonZeroValue, but no way to get the real minValue. This means the class needs to be wrapped to the real minimum value can be capture. This functionality should be built-in for cases where 0 is a valid value.

Given the lack of such a basic functionality, my guess is that there must be a good reason why this was not included, so happy to discuss if that's the case.

alexvictoor commented 4 years ago

Hello! Sorry for the very late response. HdrHistogram has been built to monitor latency on systems where performances matters. So I guess that higher values, 99 and 99.9.. percentiles are much more important for most users than lower values. Also, one think that is very important within HdrHistogram is to be as fast as possible when when recording values. So we try to keep the recording path as short as possible, to keep the overhead as low as possible.

rochdev commented 4 years ago

So I guess that higher values, 99 and 99.9.. percentiles are much more important for most users than lower values.

Fair enough, but I would argue that I've never seen a histogram implementation that does not provide this information. I feel like it should be provided, otherwise it's not equivalent with other histograms and code must be added to get the same functionality.

So we try to keep the recording path as short as possible, to keep the overhead as low as possible.

I'd be curious to see the overhead of a simple calculation such as determining the min value. I could be wrong, but I don't think it would be noticeable even at several millions of operations per second.

jberryman commented 3 years ago

lowestDiscernibleValue is already pinned to >= 1, and there seems to be bigger problems with storing small values