You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2014/01/02 13:38:58 UTC
Review Request 16569: Added freeMemory() to stout.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Repository: mesos-git
Description
-------
freeMemory() returns amount of free system memory. Can in
combination with memory() be used to get total used memory.
This allows future finer grained break-downs of used memory
in user memory, kernel buffers/caches and so on.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
Diff: https://reviews.apache.org/r/16569/diff/
Testing
-------
make check and functional testing.
Thanks,
Niklas Nielsen
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31053
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
<https://reviews.apache.org/r/16569/#comment59395>
s/main/free/ ?
- Ian Downes
On Jan. 2, 2014, 12:38 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 12:38 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Jan. 2, 2014, 11:05 p.m., Vinod Kone wrote:
> > Ship It!
Cool - I will go ahead and commit this.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31083
-----------------------------------------------------------
On Jan. 2, 2014, 11:04 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 11:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31083
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Jan. 2, 2014, 11:04 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 11:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Jan. 6, 2014, 7:25 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 888
> > <https://reviews.apache.org/r/16569/diff/4/?file=413787#file413787line888>
> >
> > If we're making total and free memory two separate function calls, os::totalMemory and os::freeMemory seems more consistent.
> >
> > But, what about a single os::memory that returns a simple os::Memory struct, rather than making two separate functions?
>
> Benjamin Hindman wrote:
> +1!
>
> Niklas Nielsen wrote:
> Ben H: +1 to merging the two functions into one or to change memory() into totalMemory()? :)
>
> If we consider continued breakdown of memory distribution (user, kernel buffers, caches, shared and so on), the shape of the struct would be OS dependent (think BSD's and Linux have different VM metrics) but that might be ok?
> Alternatively, we could have memory() returning the basic breakdown (total and free) in a struct and have auxiliary functions for the more specific ones?
>
> Ben M: Sorry for moving forward with this before getting your input. I will follow up with a subsequent review request for whatever we agree on here.
Having an os::Memory, just like we did for load average. Just having Memory { Bytes total; Bytes free; } is pretty universal and super duper clean.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31245
-----------------------------------------------------------
On Jan. 2, 2014, 11:04 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 11:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Jan. 6, 2014, 7:25 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 888
> > <https://reviews.apache.org/r/16569/diff/4/?file=413787#file413787line888>
> >
> > If we're making total and free memory two separate function calls, os::totalMemory and os::freeMemory seems more consistent.
> >
> > But, what about a single os::memory that returns a simple os::Memory struct, rather than making two separate functions?
+1!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31245
-----------------------------------------------------------
On Jan. 2, 2014, 11:04 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 11:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Jan. 6, 2014, 7:25 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 888
> > <https://reviews.apache.org/r/16569/diff/4/?file=413787#file413787line888>
> >
> > If we're making total and free memory two separate function calls, os::totalMemory and os::freeMemory seems more consistent.
> >
> > But, what about a single os::memory that returns a simple os::Memory struct, rather than making two separate functions?
>
> Benjamin Hindman wrote:
> +1!
>
> Niklas Nielsen wrote:
> Ben H: +1 to merging the two functions into one or to change memory() into totalMemory()? :)
>
> If we consider continued breakdown of memory distribution (user, kernel buffers, caches, shared and so on), the shape of the struct would be OS dependent (think BSD's and Linux have different VM metrics) but that might be ok?
> Alternatively, we could have memory() returning the basic breakdown (total and free) in a struct and have auxiliary functions for the more specific ones?
>
> Ben M: Sorry for moving forward with this before getting your input. I will follow up with a subsequent review request for whatever we agree on here.
>
> Benjamin Hindman wrote:
> Having an os::Memory, just like we did for load average. Just having Memory { Bytes total; Bytes free; } is pretty universal and super duper clean.
Cool - I'll get those merged today.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31245
-----------------------------------------------------------
On Jan. 2, 2014, 11:04 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 11:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Jan. 6, 2014, 7:25 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 888
> > <https://reviews.apache.org/r/16569/diff/4/?file=413787#file413787line888>
> >
> > If we're making total and free memory two separate function calls, os::totalMemory and os::freeMemory seems more consistent.
> >
> > But, what about a single os::memory that returns a simple os::Memory struct, rather than making two separate functions?
>
> Benjamin Hindman wrote:
> +1!
Ben H: +1 to merging the two functions into one or to change memory() into totalMemory()? :)
If we consider continued breakdown of memory distribution (user, kernel buffers, caches, shared and so on), the shape of the struct would be OS dependent (think BSD's and Linux have different VM metrics) but that might be ok?
Alternatively, we could have memory() returning the basic breakdown (total and free) in a struct and have auxiliary functions for the more specific ones?
Ben M: Sorry for moving forward with this before getting your input. I will follow up with a subsequent review request for whatever we agree on here.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31245
-----------------------------------------------------------
On Jan. 2, 2014, 11:04 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 11:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31245
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
<https://reviews.apache.org/r/16569/#comment59644>
If we're making total and free memory two separate function calls, os::totalMemory and os::freeMemory seems more consistent.
But, what about a single os::memory that returns a simple os::Memory struct, rather than making two separate functions?
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
<https://reviews.apache.org/r/16569/#comment59645>
If we're making total and free memory two separate function calls, os::totalMemory and os::freeMemory seems more consistent.
But, what about a single os::memory that returns a simple os::Memory struct, rather than making two separate functions?
- Ben Mahler
On Jan. 2, 2014, 11:04 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 11:04 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/
-----------------------------------------------------------
(Updated Jan. 2, 2014, 11:04 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Changes
-------
Forgot to remove test code in slave.
Repository: mesos-git
Description
-------
freeMemory() returns amount of free system memory. Can in
combination with memory() be used to get total used memory.
This allows future finer grained break-downs of used memory
in user memory, kernel buffers/caches and so on.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
Diff: https://reviews.apache.org/r/16569/diff/
Testing
-------
make check and functional testing.
Thanks,
Niklas Nielsen
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/
-----------------------------------------------------------
(Updated Jan. 2, 2014, 11:03 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Changes
-------
Addressed Vinod and Ian's comments.
Changed free page variable from uint64_t to unsigned int, according to:
"vmmeter.h:113: unsigned int v_free_count; /* number of pages free */"
Repository: mesos-git
Description
-------
freeMemory() returns amount of free system memory. Can in
combination with memory() be used to get total used memory.
This allows future finer grained break-downs of used memory
in user memory, kernel buffers/caches and so on.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
src/slave/http.cpp 1358810
Diff: https://reviews.apache.org/r/16569/diff/
Testing
-------
make check and functional testing.
Thanks,
Niklas Nielsen
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Ian Downes <ia...@gmail.com>.
> On Jan. 2, 2014, 6:51 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 901
> > <https://reviews.apache.org/r/16569/diff/2/?file=413414#file413414line901>
> >
> > can this constant be pulled from a well known header file?
There's a POSIX variable PAGESIZE available through sysconf.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31046
-----------------------------------------------------------
On Jan. 2, 2014, 12:38 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 12:38 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 16569: Added freeMemory() to stout.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16569/#review31046
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
<https://reviews.apache.org/r/16569/#comment59393>
can this constant be pulled from a well known header file?
- Vinod Kone
On Jan. 2, 2014, 12:38 p.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16569/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2014, 12:38 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> freeMemory() returns amount of free system memory. Can in
> combination with memory() be used to get total used memory.
> This allows future finer grained break-downs of used memory
> in user memory, kernel buffers/caches and so on.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7c1f6ec
>
> Diff: https://reviews.apache.org/r/16569/diff/
>
>
> Testing
> -------
>
> make check and functional testing.
>
>
> Thanks,
>
> Niklas Nielsen
>
>