mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 61068: Added a copy-on-write optimization for UPID 'id' field.
Date Wed, 26 Jul 2017 04:50:40 GMT

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


Fix it, then Ship it!




I guess address is much cheaper to copy? Might want to clarify that, it might not be any faster
than copying a shared_ptr?


3rdparty/libprocess/include/process/pid.hpp
Lines 108-111 (patched)
<https://reviews.apache.org/r/61068/#comment257009>

    As an aside, seems like we should also only allow the assignment operator for UPID? Rather
than exposing the fields for anyone to modify



3rdparty/libprocess/include/process/pid.hpp
Lines 112 (patched)
<https://reviews.apache.org/r/61068/#comment257006>

    Given PID and UPID, shouldn't this be ID?



3rdparty/libprocess/include/process/pid.hpp
Lines 114 (patched)
<https://reviews.apache.org/r/61068/#comment257011>

    I think you can just use `constexpr` if you want to define it here rather than in the
.cpp.



3rdparty/libprocess/include/process/pid.hpp
Lines 118-132 (patched)
<https://reviews.apache.org/r/61068/#comment257008>

    Any reason you're not utilizing the initializer list for these?



3rdparty/libprocess/include/process/pid.hpp
Lines 134-161 (patched)
<https://reviews.apache.org/r/61068/#comment257007>

    Why the partial set of comparators? Can you fill it out? It will be a little annoying
for the person who winds up having to add the missing ones



3rdparty/libprocess/include/process/pid.hpp
Lines 211-232 (patched)
<https://reviews.apache.org/r/61068/#comment257010>

    No +=? It will be a little annoying for the person who winds up having to add it


- Benjamin Mahler


On July 24, 2017, 1:57 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61068/
> -----------------------------------------------------------
> 
> (Updated July 24, 2017, 1:57 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7798
>     https://issues.apache.org/jira/browse/MESOS-7798
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a copy-on-write optimization for UPID 'id' field.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/pid.hpp 6ed936db2e45f3aab2c3765d57772844e702f914

>   3rdparty/libprocess/src/pid.cpp 634ac44337980fec03ceadaa81a7557d6a714af8 
>   3rdparty/libprocess/src/process.cpp b268cdad776a3ca2a87cbe60eb098bde2a70667c 
> 
> 
> Diff: https://reviews.apache.org/r/61068/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message