----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37877/#review98207 -----------------------------------------------------------
Ship it! 3rdparty/libprocess/include/process/metrics/counter.hpp (line 78) <https://reviews.apache.org/r/37877/#comment154505> Since we're touching this, can we use a more meaningful variable name? We use full words for variable names in Mesos. (Not yours, I know ;-)) Also pointing out the explicit specialization mentioned in a prior review. 3rdparty/libprocess/include/process/process.hpp (line 335) <https://reviews.apache.org/r/37877/#comment154503> Is this a place where we want to change the code to use a more suitable type? My thoughts are "If we're reference counting, we should be unsigned" and "If we're on a 64-bit architecture we might as well use a long to avoid chances of overflowing the ref count". Thoughts? We could do this in a subsequent review to separate the refactorings. 3rdparty/libprocess/src/latch.cpp (lines 41 - 44) <https://reviews.apache.org/r/37877/#comment154508> Please use full words for variable names. Similar below. 3rdparty/libprocess/src/process.cpp (line 430) <https://reviews.apache.org/r/37877/#comment154509> Another one where I'm curious if it would be worth increasing the range (and possibly only expressing valid values, >=0) for safety. 3rdparty/libprocess/src/process.cpp (lines 757 - 773) <https://reviews.apache.org/r/37877/#comment154521> Would you agree that what is happening here is not immediately obvious? Now that you understand it, would you mind adding a few comments around the different exit conditions and which stage of intitialization they represent? In particular I think the exit condition around 769 could use a comment. 3rdparty/libprocess/src/process.cpp (line 767) <https://reviews.apache.org/r/37877/#comment154517> Can you wrap compare_exchange_strong in backticks ``` 3rdparty/libprocess/src/process.cpp (line 768) <https://reviews.apache.org/r/37877/#comment154516> Full variable names. 3rdparty/libprocess/src/process.cpp (line 953) <https://reviews.apache.org/r/37877/#comment154519> let's use backticks ``` 3rdparty/libprocess/src/process.cpp (line 1023) <https://reviews.apache.org/r/37877/#comment154511> nice catch. 3rdparty/libprocess/src/process.cpp (line 2826) <https://reviews.apache.org/r/37877/#comment154512> nice catch. - Joris Van Remoortere On Sept. 9, 2015, 4:01 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37877/ > ----------------------------------------------------------- > > (Updated Sept. 9, 2015, 4:01 p.m.) > > > Review request for mesos, Joris Van Remoortere and switched to 'mcypark'. > > > Bugs: MESOS-3326 > https://issues.apache.org/jira/browse/MESOS-3326 > > > Repository: mesos > > > Description > ------- > > MESOS-3326. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/latch.hpp > a1a2227a9edcc31fd82c6410262aa4565fd66cb2 > 3rdparty/libprocess/include/process/metrics/counter.hpp > e51a8beb80b15dd64aa2e481036ae8ba37125640 > 3rdparty/libprocess/include/process/process.hpp > 009f7c4167fa379ac6b1c267e1b4d5fcdf28d697 > 3rdparty/libprocess/src/clock.cpp 09c60e5a5d9bd9fc5511e57f3209fad7dbf834d6 > 3rdparty/libprocess/src/latch.cpp f7d94d92e85c58878d98e13757b6fc37837ca977 > 3rdparty/libprocess/src/process.cpp > 755187c8761137cb2bf2f7295b29a63f63c68bc6 > 3rdparty/libprocess/src/process_reference.hpp > f8df4a6dcf01bb7af750c1ed9e85c64cea2042c5 > > Diff: https://reviews.apache.org/r/37877/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Neil Conway > >
