You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2019/07/03 16:07:19 UTC

Re: Review Request 70884: Added optional 'host' string member to UPID.

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


Ship it!




Was the benchmark done with an optimized build?
Can you expand on the performance implications of this change in the description? (e.g. compare the min, 25th percentile, median, 75th percentile, max of at least 10 runs).


3rdparty/libprocess/include/process/pid.hpp
Lines 204 (patched)
<https://reviews.apache.org/r/70884/#comment303525>

    Hm.. hostname or host?


- Benjamin Mahler


On June 26, 2019, 1:37 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70884/
> -----------------------------------------------------------
> 
> (Updated June 26, 2019, 1:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9858
>     https://issues.apache.org/jira/browse/MESOS-9858
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows client code to access the original hostname
> that was used to specify a libprocess address.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/pid.hpp 9f09ab46fa3ceaeac09b0fbf9f532728c4ed2d7a 
>   3rdparty/libprocess/src/pid.cpp fdc61b5ab6c75b33ce33de7edd11e9302550f300 
> 
> 
> Diff: https://reviews.apache.org/r/70884/diff/4/
> 
> 
> Testing
> -------
> 
> Results from running the `Process_BENCHMARK_ThroughputPerformance` with `--gtest_repeat=10`. There seems to be a slight degradation of performance on average, but the benchmark runtime seems to be dominated by other factors, as indicated by the large variance and presence of outliers on both sides.
> 
> 
> # Master
> 2519083.849973
> 2401163.451671
> 2322734.358626
> 2285763.292828
> 1883268.801707
> 2179436.004374
> 2071825.301714
> 2105588.897964
> 2067228.706673
> 
> 
> # Including UPID changes
> 2305142.571762
> 2194902.761484
> 2139113.533129
> 2024036.049603
> 1765327.993699
> 2172592.534808
> 2053217.096855
> 2103817.232509
> 1480080.596818
> 1808977.015807
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 70884: Added optional 'host' string member to UPID.

Posted by Benno Evers <be...@mesosphere.com>.

> On July 3, 2019, 4:07 p.m., Benjamin Mahler wrote:
> > Was the benchmark done with an optimized build?
> > Can you expand on the performance implications of this change in the description? (e.g. compare the min, 25th percentile, median, 75th percentile, max of at least 10 runs).

Sorry, I did not see this comment before committing. The benchmark above was done on my laptop, I think using an optimized build but I can't prove it anymore. I can repeat the benchmark on one of our build machines and record the results in some JIRA instead.


> On July 3, 2019, 4:07 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/pid.hpp
> > Lines 204 (patched)
> > <https://reviews.apache.org/r/70884/diff/4/?file=2153428#file2153428line204>
> >
> >     Hm.. hostname or host?

I opted for `host` here because that's the name that was already used in the PID-parsing code. However, since this is an internal field there should be no problem updating the name if we decide something else would be more appropriate.


- Benno


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


On June 26, 2019, 1:37 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70884/
> -----------------------------------------------------------
> 
> (Updated June 26, 2019, 1:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9858
>     https://issues.apache.org/jira/browse/MESOS-9858
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This allows client code to access the original hostname
> that was used to specify a libprocess address.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/pid.hpp 9f09ab46fa3ceaeac09b0fbf9f532728c4ed2d7a 
>   3rdparty/libprocess/src/pid.cpp fdc61b5ab6c75b33ce33de7edd11e9302550f300 
> 
> 
> Diff: https://reviews.apache.org/r/70884/diff/4/
> 
> 
> Testing
> -------
> 
> Results from running the `Process_BENCHMARK_ThroughputPerformance` with `--gtest_repeat=10`. There seems to be a slight degradation of performance on average, but the benchmark runtime seems to be dominated by other factors, as indicated by the large variance and presence of outliers on both sides.
> 
> 
> # Master
> 2519083.849973
> 2401163.451671
> 2322734.358626
> 2285763.292828
> 1883268.801707
> 2179436.004374
> 2071825.301714
> 2105588.897964
> 2067228.706673
> 
> 
> # Including UPID changes
> 2305142.571762
> 2194902.761484
> 2139113.533129
> 2024036.049603
> 1765327.993699
> 2172592.534808
> 2053217.096855
> 2103817.232509
> 1480080.596818
> 1808977.015807
> 
> 
> Thanks,
> 
> Benno Evers
> 
>