You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2019/08/23 04:56:23 UTC
Review Request 71360: Optimized shrinkResources() by filtering before
copying.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71360/
-----------------------------------------------------------
Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
Bugs: MESOS-9806
https://issues.apache.org/jira/browse/MESOS-9806
Repository: mesos
Description
-------
Master + previous patch:
HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 15.635489759secs
Made 0 allocation in 14.291803907secs
Master + previous patch + this patch:
HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 15.361303511secs
Made 0 allocation in 14.323725559secs
Diffs
-----
include/mesos/resource_quantities.hpp cdb34271868ab5931d7e35273af1219824d4d5b9
src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6
src/common/resources_utils.cpp cbdad4b045ad957b2ab1fd09a6b9fb12bb4fb523
Diff: https://reviews.apache.org/r/71360/diff/1/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 71360: Optimized shrinkResources() by filtering
before copying.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Aug. 23, 2019, 7:25 p.m., Benjamin Mahler wrote:
> > src/common/resources_utils.cpp
> > Line 915 (original), 915-917 (patched)
> > <https://reviews.apache.org/r/71360/diff/1/?file=2162750#file2162750line915>
> >
> > It's not clear to me that this is less expensive, if every name is in the target it's strictly more expensive?
> >
> > How about adding an overload of the `RepeatedPtrField<Resource>` operator that works off of an rvalue of the object? That way we can actually move out rather than copy out?
Oh.. I guess it won't help since every Resource will have a shared count > 1, so can't move it out.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71360/#review217410
-----------------------------------------------------------
On Aug. 23, 2019, 4:56 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71360/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2019, 4:56 a.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
>
>
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master + previous patch:
>
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.635489759secs
> Made 0 allocation in 14.291803907secs
>
> Master + previous patch + this patch:
>
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.361303511secs
> Made 0 allocation in 14.323725559secs
>
>
> Diffs
> -----
>
> include/mesos/resource_quantities.hpp cdb34271868ab5931d7e35273af1219824d4d5b9
> src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6
> src/common/resources_utils.cpp cbdad4b045ad957b2ab1fd09a6b9fb12bb4fb523
>
>
> Diff: https://reviews.apache.org/r/71360/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 71360: Optimized shrinkResources() by filtering
before copying.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71360/#review217410
-----------------------------------------------------------
src/common/resources_utils.cpp
Line 915 (original), 915-917 (patched)
<https://reviews.apache.org/r/71360/#comment304691>
It's not clear to me that this is less expensive, if every name is in the target it's strictly more expensive?
How about adding an overload of the `RepeatedPtrField<Resource>` operator that works off of an rvalue of the object? That way we can actually move out rather than copy out?
- Benjamin Mahler
On Aug. 23, 2019, 4:56 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71360/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2019, 4:56 a.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
>
>
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master + previous patch:
>
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.635489759secs
> Made 0 allocation in 14.291803907secs
>
> Master + previous patch + this patch:
>
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.361303511secs
> Made 0 allocation in 14.323725559secs
>
>
> Diffs
> -----
>
> include/mesos/resource_quantities.hpp cdb34271868ab5931d7e35273af1219824d4d5b9
> src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6
> src/common/resources_utils.cpp cbdad4b045ad957b2ab1fd09a6b9fb12bb4fb523
>
>
> Diff: https://reviews.apache.org/r/71360/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 71360: Optimized shrinkResources() by filtering
before copying.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71360/#review217409
-----------------------------------------------------------
src/common/resource_quantities.cpp
Lines 183-197 (patched)
<https://reviews.apache.org/r/71360/#comment304690>
Do we need the new function? Can't we just:
```
Value::Scalar zero;
google::protobuf::RepeatedPtrField<Resource> resourceVector =
resources.filter(
[&target](const Resource& r) { return target.get(r.name()) != zero; });
```
- Benjamin Mahler
On Aug. 23, 2019, 4:56 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71360/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2019, 4:56 a.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
>
>
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master + previous patch:
>
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.635489759secs
> Made 0 allocation in 14.291803907secs
>
> Master + previous patch + this patch:
>
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.361303511secs
> Made 0 allocation in 14.323725559secs
>
>
> Diffs
> -----
>
> include/mesos/resource_quantities.hpp cdb34271868ab5931d7e35273af1219824d4d5b9
> src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6
> src/common/resources_utils.cpp cbdad4b045ad957b2ab1fd09a6b9fb12bb4fb523
>
>
> Diff: https://reviews.apache.org/r/71360/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>