> On May 30, 2013, 6:24 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/bandwidth_tests.cpp, lines 98-101 > > <https://reviews.apache.org/r/11542/diff/1/?file=299033#file299033line98> > > > > woah. this is hard to comprehend. can we simpler numbers here?
I added some comments here. The numbers don't really mean much but but basically I just wanted to show/check what the outputs of multiple bandwidths of different units added together look like. (15 digits of full precision, no trailing zeros, etc). > On May 30, 2013, 6:24 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/bandwidth_tests.cpp, line 32 > > <https://reviews.apache.org/r/11542/diff/1/?file=299033#file299033line32> > > > > can we test a good constructor too? Added two EXPECT_NO_THROW cases. Good constructors are used throughout the test. > On May 30, 2013, 6:24 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/bandwidth.hpp, lines > > 126-149 > > <https://reviews.apache.org/r/11542/diff/1/?file=299032#file299032line126> > > > > how about calling these functions bps(), kbps() etc? > > Jiang Yan Xu wrote: > I'd love to, but see the explanation above. Keeping the method names in the longer form to avoid ambiguity. > On May 30, 2013, 6:24 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/bandwidth.hpp, lines 39-49 > > <https://reviews.apache.org/r/11542/diff/1/?file=299032#file299032line39> > > > > how about "bps", "kbps" etc too? > > > > also kbit, mbit etc doesn't make sense for bandwidth no? > > Jiang Yan Xu wrote: > The units are very confusing... > > According to BenH, we want to comply with the units used in "tc". > In tc http://linux.die.net/man/8/tc > "mbit" means "Megabits per second" while "mbps" means "Megabytes per > second" > > This contradicts how the unit "mbps" is often used... it is however, not > a standard. > > The standard says we should be "Mibit/s" to mean 2^20 bits per second and > "Mbit/s" to mean 10^6 bits per second but every programmer seems to use > "Mbit/s" to mean "2^20 bits per second"... > http://en.wikipedia.org/wiki/Bit_rate > > I'd prefer to use kbps etc to mean "kilobits per second" and elaborate on > this in the documentation if there's no objection. Improved documentation to explain 'mbit' is 'tc' style. Changed 'kbit/s' to 'Kbit/s'. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11542/#review21198 ----------------------------------------------------------- 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 > >
