> On Feb. 4, 2015, 11:47 p.m., Dominic Hamon wrote: > > src/linux/cgroups.hpp, line 602 > > <https://reviews.apache.org/r/30545/diff/2/?file=848406#file848406line602> > > > > why is there a trailing underscore?
there is a function counter(); c++ doesn't allow a member function and a field to have the same name. > On Feb. 4, 2015, 11:47 p.m., Dominic Hamon wrote: > > src/linux/cgroups.hpp, line 568 > > <https://reviews.apache.org/r/30545/diff/2/?file=848406#file848406line568> > > > > there's a static method that creates a PressureListener, and a > > constructor to create a Process, and they take similar arguments. > > > > this is a confusing API. > > > > as an end user of this system, what should I be calling? should the > > PressureListener be the public class and the PressureListenerProcess be a > > private implementation detail? The constructor should be private. > On Feb. 4, 2015, 11:47 p.m., Dominic Hamon wrote: > > src/linux/cgroups.hpp, line 563 > > <https://reviews.apache.org/r/30545/diff/2/?file=848406#file848406line563> > > > > could this return a Try<Owned<PressureListener>> instead? Replying here about returning a Try<PressureListener*> vs a Try<Owned<PressureListener>> and the memory leak comment previously. It is a common pattern for a function like cgroups::listen to create a libprocess, install some callbacks, spawn if off and return. Since spawn takes a raw libprocess pointer, when the function returns, the owned pointer frees the memory of the libprocess running. Try::get returns a const reference, so even 'releasing' the raw pointer out of the owned pointer isn't possible. Freeing the memory in a callback doesn't work because the callback is run in the context of the libprocess. These are my findings so far; would love to know if I have missed anything. - Chi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30545/#review71078 ----------------------------------------------------------- On Feb. 12, 2015, 8:08 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30545/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2015, 8:08 p.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Bugs: MESOS-2136 > https://issues.apache.org/jira/browse/MESOS-2136 > > > Repository: mesos > > > Description > ------- > > cgroups: added support to listen on memory pressures. > > > Diffs > ----- > > src/Makefile.am 39ce858fd009f8aef388bd3d6b17aa4c885ff9c9 > src/linux/cgroups.hpp bf8efee05b995a37d4e6e7679a493b2d5238aa0b > src/linux/cgroups.cpp b6f75b14dea7609b90627eccdd33ef891ed9d21f > src/tests/cgroups_tests.cpp 9d50a47c939f5f136c85fdcc555459512c6f2259 > src/tests/memory_test_helper.hpp PRE-CREATION > src/tests/memory_test_helper.cpp PRE-CREATION > src/tests/memory_test_helper_main.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/30545/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
