> On Sept. 6, 2018, 5:37 p.m., James Peach wrote: > > 3rdparty/stout/include/stout/os/posix/lsof.hpp > > Lines 29 (patched) > > <https://reviews.apache.org/r/68642/diff/1/?file=2082825#file2082825line29> > > > > Why hashset rather than vector? By definition there aren't any > > duplicates, so we don't need the overhead of hashset here? > > > > /cc @bmahler > > Qian Zhang wrote: > If we make this method return a vector, then in the subsequent patches > when we remove whitelist fds from the vector, we need to use `std::find` to > get the position of the to-be-deleted fd and then call `std::vector::erase` > to remove it which is inefficient, especially the implementation of > `std::vector::erase` may need to relocate all the elements > (http://www.cplusplus.com/reference/vector/vector/erase/) . So I think we can > use `std::list` like what we did for `os::ls`.
The malloc overhead of using a list is way more expensive than the memmove needed for the vector erase. In the one place that does this, we can also just skip over closing the stdio file descriptors if there is a performance concern. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68642/#review208411 ----------------------------------------------------------- On Oct. 11, 2018, 1:57 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68642/ > ----------------------------------------------------------- > > (Updated Oct. 11, 2018, 1:57 p.m.) > > > Review request for mesos, Gilbert Song and James Peach. > > > Bugs: MESOS-9152 > https://issues.apache.org/jira/browse/MESOS-9152 > > > Repository: mesos > > > Description > ------- > > Added `lsof()` into stout. > > > Diffs > ----- > > 3rdparty/stout/include/Makefile.am a9c61fcba253a811494cdcdb0afb3d3a018f4585 > 3rdparty/stout/include/stout/os.hpp > aee041891b7e7ff93a0b1ac31019a7a3d4eae962 > 3rdparty/stout/include/stout/os/lsof.hpp PRE-CREATION > 3rdparty/stout/include/stout/os/posix/lsof.hpp PRE-CREATION > 3rdparty/stout/include/stout/os/windows/lsof.hpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/68642/diff/2/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >
