nickva commented on code in PR #4672:
URL: https://github.com/apache/couchdb/pull/4672#discussion_r1266200005


##########
src/couch_stats/src/couch_stats_histogram.erl:
##########
@@ -0,0 +1,449 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+%   http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+% This module implements windowed base-2 histograms using Erlang counters [1].
+%
+% Base-2 histograms use power of 2 exponentially increasing bin widths. This
+% allows capturing a range of values from microseconds to hours with a
+% relatively small number of bins. The same principle is used when encoding
+% floating point numbers [2]. In fact, our histograms rely on the ease of
+% constructing and mainpululating binary representations of 64 bit floats in
+% Erlang to do all of its heavy lifting.
+%
+% As a refresher, the standard (IEEE 754) 64 bit floating point representations
+% looks something like:
+%
+%  sign  exponent    mantissa
+%  [s64] [e63...e53] [m52...m1]
+%  <-1-> <---11----> <---52--->
+%
+%
+% The simplest scheme migth be to use the exponent to select the histogram bin
+% and throw away the mantissa bits. However, in that case bin sizes end up
+% growing a bit too fast and we lose resolution quickly. To increase the
+% resolution, use a few most significant bit from the mantissa. For example,
+% use 3 more bits mantissa bits for a total of 14 bits: [e63...e53] + [m52,
+% m51, m50]:
+%
+%  sign  exponent    mantissa
+%  [s64] [e63...e53] [m52, m51, m50, m49...m1]
+%  <-1-> <-----------14------------> <--49--->
+%        ^^^^^^^ bin index ^^^^^^^^^
+%
+% With Erlang's wonderful binary matching capabilities this becomes a
+% one-liner:
+%
+%    <<0:1, BinIndex:14, _/bitstring>> = <<Val/float>>
+%
+% The internal implementation is a tuple of counter:new(?BIN_COUNT)
+% elements. The tuple size is determined by the time window parameter when the
+% histogram is created. After a histogram object is created, its Erlang term
+% structure is static so it's suitable to be stored in a persistent term [3].
+% The structure looks something like:
+%
+%  {
+%     1          = [1, 2, ..., ?BIN_COUNT]
+%     2          = [1, 2, ..., ?BIN_COUNT]
+%     ...
+%     TimeWindow = [1, 2, ..., ?BIN_COUNT]
+%  }
+%
+% The representation can also be regarded as a table with rows as abstract time
+% units: 1 = 1st second, 2 = 2nd second, etc, and the columns as histogram
+% bins. The update/3 function takes a value and a current time as parameters.
+% The time is used to select which row to update, and the value picks which
+% bin to increment.
+%
+% In practice, the time window would be used as a circular buffer. The time
+% parameter to the update/3 function might be the system monotonic clock time
+% and then the histogram time index is compuated as `Time rem TimeWindow`. So,
+% as the monotonic time is advancing forward, the histogram time index will
+% loop around. This comes with a minor annoynance of having to allocate a
+% larger time window to accomodate some process which cleans stale (expired)
+% histogram entries, possibly with some extra buffers to ensure the currently
+% updated interval and the interval ready to be cleaned would not overlap.
+%
+% Reading a histogram can be done via the read/3 function. The function takes a
+% start time and an interval. All the histogram entries in the interval will be
+% summed together, bin by bin, and returned as a new counters object. This
+% functionality can be used to gather and merge multiple histogram together.
+%
+% To get a stats summary across a time window use the stats/3 function. Just
+% like the read/3 function, it takes a start time and a time interval over
+% which to summarize the data.
+%
+% In addition to the new/1, update/3, read/3, stats/3 there are simple
+% functions which default to using WindowSize = 1. The intent is they would be
+% used when a simple histogram is needed without the time window functionality.
+%
+% [1] https://www.erlang.org/doc/man/counters.html
+% [2] https://en.wikipedia.org/wiki/IEEE_754
+% [3] https://www.erlang.org/doc/man/persistent_term.html
+
+-module(couch_stats_histogram).
+
+-export([
+    new/0,
+    new/1,
+
+    update/2,
+    update/3,
+
+    stats/1,
+    stats/3,
+
+    read/1,
+    read/3,
+
+    clear/1,
+    clear/3,
+
+    bin_min/1,
+    bin_max/1,
+    bin_middle/1,
+
+    calc_bin_offset/0,
+    calc_bin_count/0,
+    get_bin_boundaries/0
+]).
+
+-on_load(check_constants/0).

Review Comment:
   Ah good point about having both `-on_load(...)` handler and `calc_*` 
functions. I added `-on_load(...)` later but didn't consider that it would 
prevent using the `calc_*` functions.
   
   Wonder if we should just rely on eunit tests and skip `-on_load(...)` or 
maybe just add a comment by `-on_load(...)` specifying to comment it out when 
updating constants?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to