> On March 11, 2015, 7 p.m., Ben Mahler wrote: > > I'm curious, what prompted this change? > > Dominic Hamon wrote: > a comment on the original version in a review that it wasn't the best way > of counting bits. it seems like a generally useful thing to do. > > Evelina Dumitrescu wrote: > another benefit: IPv6 netmasks, where there are 128 bits.
I don't disagree with this change at all. I'm just hoping someone actually put some thought into why this matters: Are we optimizing with an understanding of where the bottlenecks are, and how this avoids them? For example, when is this bit counting occurring? How often? Is it in a critical path of the code? These are the kinds of questions its very important to be asking ourselves when we decide to optimize things, otherwise we start to introduce complexity in the wrong places. > On March 11, 2015, 7 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/bits.hpp, line 25 > > <https://reviews.apache.org/r/31677/diff/5/?file=890606#file890606line25> > > > > Can we return a size_t to capture that this is a count and that > > negatives are not possible? > > > > Is `bits::count` not a sufficient name? > > Dominic Hamon wrote: > as per the style guide, > http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Integer_Types, > "You should not use the unsigned integer types such as uint32_t, unless > there is a valid reason such as representing a bit pattern rather than a > number, or you need defined overflow modulo 2^N. In particular, do not use > unsigned types to say a number will never be negative. Instead, use > assertions for this." > > there may be a countUnsetBits in the future. Maybe countSet, but we could > bikeshed all day :) >From that same section... *When appropriate, you are welcome to use standard types like **size_t** and ptrdiff_t.* A grep through the code shows we use "size_t" heavily. In the case of "size_t", we're using the type system to provide valuable information to the caller. The caller understands that a size is being returned and does not need to think about what it means for this to return -1. I'd like to understand *why* you think we shouldn't use size_t since we're already leveraging it heavily. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31677/#review76105 ----------------------------------------------------------- On March 10, 2015, 7:59 p.m., Evelina Dumitrescu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31677/ > ----------------------------------------------------------- > > (Updated March 10, 2015, 7:59 p.m.) > > > Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van > Remoortere, and Niklas Nielsen. > > > Repository: mesos > > > Description > ------- > > see summary > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/Makefile.am > a5224554f6851930aa97cadc5da3d010829d87dc > 3rdparty/libprocess/3rdparty/stout/include/Makefile.am > ac2bbed6fe86623fb51cac3613d79d7b1372df9d > 3rdparty/libprocess/3rdparty/stout/include/stout/bits.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp > e4e86deb8a42a1d91f4d4ac210eae26f8f57ee17 > 3rdparty/libprocess/3rdparty/stout/tests/bits_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/31677/diff/ > > > Testing > ------- > > > Thanks, > > Evelina Dumitrescu > >
