Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

2019-08-23 Thread Benjamin Mahler

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


Fix it, then Ship it!





src/common/resource_quantities.cpp
Lines 155-157 (original), 150-153 (patched)


Is there a cleaner way to add const-ness?

E.g.

```
const auto& self = *this;
return self.begin();
```


- Benjamin Mahler


On Aug. 23, 2019, 7:44 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 23, 2019, 7:44 p.m.)
> 
> 
> Review request for mesos 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 16.831380548secs
> Made 0 allocation in 15.102885644secs
> 
> Master + previous patch + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 16.307044003secs
> Made 0 allocation in 14.948262599secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>



Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

2019-08-23 Thread Meng Zhu

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

(Updated Aug. 23, 2019, 12:44 p.m.)


Review request for mesos and Benjamin Mahler.


Bugs: MESOS-9806
https://issues.apache.org/jira/browse/MESOS-9806


Repository: mesos


Description (updated)
---

Master + previous patch
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 16.831380548secs
Made 0 allocation in 15.102885644secs

Master + previous patch + this patch:
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 16.307044003secs
Made 0 allocation in 14.948262599secs


Diffs (updated)
-

  include/mesos/resource_quantities.hpp 
cdb34271868ab5931d7e35273af1219824d4d5b9 
  src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 


Diff: https://reviews.apache.org/r/71355/diff/3/

Changes: https://reviews.apache.org/r/71355/diff/2-3/


Testing
---

make check
benchmark result in description


Thanks,

Meng Zhu



Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

2019-08-23 Thread Meng Zhu


> On Aug. 23, 2019, 11:38 a.m., Benjamin Mahler wrote:
> > include/mesos/resource_quantities.hpp
> > Lines 168-169 (patched)
> > 
> >
> > How will we know to update this if we have more first class resources?
> > 
> > 5 seems ok if we think we'll to update this when we add another 
> > resource name

Made it 7 and add a comment. Since we don't a native concept of first class 
resources, I don't see a way to automate this or even raise a warning.


- Meng


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


On Aug. 23, 2019, 12:44 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 23, 2019, 12:44 p.m.)
> 
> 
> Review request for mesos 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 16.831380548secs
> Made 0 allocation in 15.102885644secs
> 
> Master + previous patch + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 16.307044003secs
> Made 0 allocation in 14.948262599secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>



Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

2019-08-23 Thread Benjamin Mahler

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




include/mesos/resource_quantities.hpp
Lines 168-169 (patched)


How will we know to update this if we have more first class resources?

5 seems ok if we think we'll to update this when we add another resource 
name


- Benjamin Mahler


On Aug. 22, 2019, 11:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 22, 2019, 11:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 23.37 secs
> Made 0 allocation in 19.72 secs
> 
> Master + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 22.065721878secs
> Made 0 allocation in 19.467366742secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>



Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

2019-08-23 Thread Benjamin Mahler


> On Aug. 23, 2019, 6:24 p.m., Benjamin Mahler wrote:
> > include/mesos/resource_quantities.hpp
> > Lines 121-124 (original), 122-127 (patched)
> > 
> >
> > "small_vector is convertible to small_vector_base > Allocator> that is independent from the preallocated element capacity, so 
> > client code does not need to be templated on that N argument."
> > 
> > Is it possible to expose the non-sized version to the clients? i.e. 
> > 
> > ```
> > boost::container::small_vector > Value::Scalar>>::const_iterator
> > ```

Whoops, small_vector -> small_vector_base


- Benjamin


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


On Aug. 22, 2019, 11:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 22, 2019, 11:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 23.37 secs
> Made 0 allocation in 19.72 secs
> 
> Master + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 22.065721878secs
> Made 0 allocation in 19.467366742secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>



Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

2019-08-23 Thread Benjamin Mahler

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




include/mesos/resource_quantities.hpp
Lines 121-124 (original), 122-127 (patched)


"small_vector is convertible to small_vector_base that is independent from the preallocated element capacity, so 
client code does not need to be templated on that N argument."

Is it possible to expose the non-sized version to the clients? i.e. 

```
boost::container::small_vector>::const_iterator
```


- Benjamin Mahler


On Aug. 22, 2019, 11:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 22, 2019, 11:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 23.37 secs
> Made 0 allocation in 19.72 secs
> 
> Master + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 22.065721878secs
> Made 0 allocation in 19.467366742secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>



Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

2019-08-22 Thread Meng Zhu

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

(Updated Aug. 22, 2019, 4:58 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Splited out the boost library change.


Bugs: MESOS-9806
https://issues.apache.org/jira/browse/MESOS-9806


Repository: mesos


Description
---

Master
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 23.37 secs
Made 0 allocation in 19.72 secs

Master + this patch:
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 22.065721878secs
Made 0 allocation in 19.467366742secs


Diffs (updated)
-

  include/mesos/resource_quantities.hpp 
cdb34271868ab5931d7e35273af1219824d4d5b9 
  src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 


Diff: https://reviews.apache.org/r/71355/diff/2/

Changes: https://reviews.apache.org/r/71355/diff/1-2/


Testing
---

make check
benchmark result in description


Thanks,

Meng Zhu