Review Request 26578: Memory cleanup: libprocess gc finalize

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26578/
---

Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Repository: mesos-git


Description
---

Implement the finalize() virtual override for Libprocess' garbage collector.
This terminates all processes registered with the garbage collector when the 
garbage collector terminates.


Diffs
-

  3rdparty/libprocess/include/process/gc.hpp e83c636 

Diff: https://reviews.apache.org/r/26578/diff/


Testing
---

make check
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 26578: Memory cleanup: libprocess gc finalize

[Dominic Hamon]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26578/#review56264
---



3rdparty/libprocess/include/process/gc.hpp


std::map> processes?


- Dominic Hamon


On Oct. 10, 2014, 3:13 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26578/
> ---
> 
> (Updated Oct. 10, 2014, 3:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Implement the finalize() virtual override for Libprocess' garbage collector.
> This terminates all processes registered with the garbage collector when the 
> garbage collector terminates.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gc.hpp e83c636 
> 
> Diff: https://reviews.apache.org/r/26578/diff/
> 
> 
> Testing
> ---
> 
> make check
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 26578: Memory cleanup: libprocess gc finalize

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26578/
---

(Updated Oct. 13, 2014, 5:29 p.m.)


Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Changes
---

Use std::unqiue_ptr in libprocess gc.
Rewrite exited() logic to use find() erase to reduce map lookups.


Repository: mesos-git


Description
---

Implement the finalize() virtual override for Libprocess' garbage collector.
This terminates all processes registered with the garbage collector when the 
garbage collector terminates.


Diffs (updated)
-

  3rdparty/libprocess/include/process/gc.hpp e83c636 

Diff: https://reviews.apache.org/r/26578/diff/


Testing
---

make check
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 26578: Memory cleanup: libprocess gc finalize

[Niklas Nielsen]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26578/#review56803
---



3rdparty/libprocess/include/process/gc.hpp


How about the include for unique ptr?



3rdparty/libprocess/include/process/gc.hpp


Let 'iter' spell out what it is for (and expand to a full word).


- Niklas Nielsen


On Oct. 13, 2014, 10:29 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26578/
> ---
> 
> (Updated Oct. 13, 2014, 10:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Implement the finalize() virtual override for Libprocess' garbage collector.
> This terminates all processes registered with the garbage collector when the 
> garbage collector terminates.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gc.hpp e83c636 
> 
> Diff: https://reviews.apache.org/r/26578/diff/
> 
> 
> Testing
> ---
> 
> make check
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 26578: Memory cleanup: libprocess gc finalize

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26578/
---

(Updated Oct. 15, 2014, 9:37 p.m.)


Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Changes
---

fix style issue.


Repository: mesos-git


Description
---

Implement the finalize() virtual override for Libprocess' garbage collector.
This terminates all processes registered with the garbage collector when the 
garbage collector terminates.


Diffs (updated)
-

  3rdparty/libprocess/include/process/gc.hpp e83c636 

Diff: https://reviews.apache.org/r/26578/diff/


Testing
---

make check
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 26578: Memory cleanup: libprocess gc finalize

[Benjamin Hindman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26578/#review58919
---



3rdparty/libprocess/include/process/gc.hpp


This, IMHO, is a misleading/confusing use of unique_ptr, and a perfect 
opportunity to discuss how, and when, we want to use it in the code base.

The reason this case is misleading is because 'process' is in no way 
"unique", in the sense that this pointer will be stored in lots of different 
structures throughout the libprocess library. What you're using unique_ptr for 
here is "automatic" memory management (although it's not so automatic because 
you're still calling 'reset') but that might give some people the illusion that 
they can use this pointer in such a way that nobody else can manipulate the 
object that is being pointed to because nobody else knows it.

In this case it's easy to just use 'delete' where you're doing 'reset', but 
in other cases we should be very explicit about our use of unique_ptr and the 
semantics we want to convey to other readers of the code.



3rdparty/libprocess/include/process/gc.hpp


I'm not convinced these are a good semantics. The point of the garbage 
collector is to reclaim memory when a process terminates, but now we've also 
termianted processes when the garbage collector gets terminated! Making sure 
all processes are garbage (i.e., terminated) should be outside the scope of the 
responsibilities of the GarbageCollector.


- Benjamin Hindman


On Oct. 15, 2014, 9:37 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26578/
> ---
> 
> (Updated Oct. 15, 2014, 9:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Implement the finalize() virtual override for Libprocess' garbage collector.
> This terminates all processes registered with the garbage collector when the 
> garbage collector terminates.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gc.hpp e83c636 
> 
> Diff: https://reviews.apache.org/r/26578/diff/
> 
> 
> Testing
> ---
> 
> make check
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>