> On June 3, 2013, 10:39 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/bandwidth.hpp, line 10
> > <https://reviews.apache.org/r/11542/diff/2/?file=299690#file299690line10>
> >
> > I'm not sold on the benefit of using boost rational here, were there
> > other motivations besides the bad_rational handling?
> >
> > We generally do not use exceptions in libstout, but I see why you may
> > have wanted to given the constructor can fail. But what are the failure
> > scenarios you would like to handle? It seems a resulting SIGFPE from a
> > duration of 0 is sufficient, no?
>
> Jiang Yan Xu wrote:
> I think it is intuitive because it semantically has the two components
> that we measure as integers: the numerator (# of bits) and the denominator
> (duration) but if we store them as one 'double', we lose precision in cases
> such as "10/3: 10 bits per 3 seconds".
> Alternatively we can store the numerator and the denominator separately
> as two integer fields but in operators such as '+' and '==' we need to do all
> the math (LCM/GCD) that rational class already does for us.
>
> =====
>
> If we stick with rational then I am only passing the bad_rational
> exception up to the caller.
>
> Ben Mahler wrote:
> I brought this up as we generally try to avoid using boost libraries
> where not needed.
>
> In this case I don't think the precision loss is severe, especially
> considering we are dealing with discrete values under the hood (bits) with a
> discrete (ns) denominator. Can you convince me otherwise?
>
> I get the sense that 95% of the Bandwidth users be using 1 second as the
> denominator. If so, then I'm in favor of a simpler implementation at the cost
> of potential precision loss in certain use-cases. What are these cases?
My first implementation used 'double' type (not submitted for review)...
The case we are concerned about not being able to precisely represent repeating
decimals is when determining whether resources are fully allocated: 0.333... +
0.666... = 0.999... (!= 1) while 1/3 + 2/3 == 1
There are certainly techniques to deal with this but in general I think we are
preferring to use more precise representations ('double' -> 'int64_t' in
Duration).
If what makes this case special is that we are depending on the boost library
class I think this is a general design issue and I am OK with either of the two
approaches.
==
I think there are two cases in general
1) we measure periodically (not necessarily per 1 second but can be normalized
to 1 second)
2) we measure when an event is fired, which could be any Duration
With the rational class, the math (normalization) is done automatically. You
just give it a Bytes and a Duration object without calculating the bandwidth
yourself.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11542/#review21363
-----------------------------------------------------------
On May 31, 2013, 9:20 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11542/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 9:20 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Vinod Kone, and Ben Mahler.
>
>
> Description
> -------
>
> - This implementation uses Boost::rational<uint64_t> to store both the bits
> and the nanoseconds as integers to preserve precision.
> - It requires the boost lib to be repackaged to include Boost::rational.
> - The usage 'rational' handles avoid overflow cases better (it doesn't) than
> simply multiply the denominators in various arithmetic operations.
> - Bandwidth always >= 0
> - Bandwidth constructor passes 'boost::bad_rational' up when the denominator
> is zero.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am
> 7a9ede62145e3150f7af6675d4384feafd9c0a88
> 3rdparty/libprocess/3rdparty/boost-1.53.0.tar.gz
> 770d837aaba23d031b04ad77658f339587174aae
> 3rdparty/libprocess/3rdparty/stout/Makefile.am
> 84062a0e1dfe4ec04bac7cac5ebaac4b945eb66e
> 3rdparty/libprocess/3rdparty/stout/include/stout/bandwidth.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/bandwidth_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/11542/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
