Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-11 Thread Yanyan Hu 


> On July 11, 2016, 11:37 p.m., Joseph Wu wrote:
> > Content-wise, looks good.  
> > 
> > I've left some comments below on a few stylistic nits, which I'll fix 
> > before committing.
> > I also went ahead and tweaked your patch description to explain what the 
> > improvement was.

Hi, Joseph, thank you so much for helping fix these issues!


- Yanyan


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


On July 11, 2016, 7:52 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 11, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-11 Thread Joseph Wu 

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


Ship it!




Content-wise, looks good.  

I've left some comments below on a few stylistic nits, which I'll fix before 
committing.
I also went ahead and tweaked your patch description to explain what the 
improvement was.


src/common/values.cpp (lines 275 - 276)


* Missing a space after `foreach`.
* Missing curly braces.
* Indent by 2 spaces, not 4.



src/common/values.cpp (lines 288 - 292)


* Opening curly brace here stays inline.
* Rename `s` variable to `interval`.
* Indent with 2 spaces, not 3 :)



src/common/values.cpp (lines 395 - 397)


Rename to `_left`, `_right`, `left`, and `right`.



src/common/values.cpp (line 401)


Missing some spaces here.



src/tests/values_tests.cpp (line 318)


Capitalize test name.  Here and below.



src/tests/values_tests.cpp (lines 338 - 339)


This fits on one line.



src/tests/values_tests.cpp (line 366)


Typo here: `tp`.



src/tests/values_tests.cpp (lines 369 - 370)


Fits on one line.


- Joseph Wu


On July 11, 2016, 12:52 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 11, 2016, 12:52 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-11 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [48593]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 11, 2016, 7:52 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 11, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-11 Thread Yanyan Hu 

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

(Updated July 11, 2016, 7:52 a.m.)


Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
Remoortere.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/48593/diff/


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-11 Thread Yanyan Hu 


> On July 11, 2016, 7:43 a.m., Joris Van Remoortere wrote:
> > Let's use full, and meaningful words for our names.

Yes, will rename res variable to ranges as Joseph suggested. Thanks.


- Yanyan


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


On July 8, 2016, 1:42 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 8, 2016, 1:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-10 Thread Yanyan Hu 


> On July 8, 2016, 10:05 p.m., Joseph Wu wrote:
> > src/tests/values_tests.cpp, lines 362-364
> > 
> >
> > Here, you have (closed, open) bounds.
> > 
> > In the implementation, you (correctly) use (closed, closed) bounds.  
> > Consider normalizing.

Hi, Joseph, thanks a lot for your review. Will make change according to your 
comment.


- Yanyan


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


On July 8, 2016, 1:42 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 8, 2016, 1:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-08 Thread Joseph Wu 

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




src/common/values.cpp (line 271)


Make sure you use the `uint64_t` type.  

Here and below; and in tests.



src/common/values.cpp (lines 275 - 278)


There's no need to use an integer iterator here.  Do this instead:

```
foreach (const Value::Range& range, ranges.range()) {
  set += (Bound::closed(range.begin()),
  Bound::closed(range.end()));
}
```



src/common/values.cpp (line 287)


No need for this.  See below comment.



src/common/values.cpp (line 288)


Can you rename this local variable to `ranges`?



src/common/values.cpp (lines 292 - 295)


When you add an item to a protobuf array, the generated C++ code returns a 
pointer than you can further modify:

```
Value::Range* range = ranges.add_range();
range->set_begin(s.lower());
range->set_end(s.upper() - 1);
```



src/common/values.cpp (line 400)


Consider renaming to `leftSet` and `rightSet`.



src/common/values.cpp (line 402)


We should be able to forgo this `coalesce` step now.  

Before, this would normalize the input, by sorting the ranges and merging 
adjacent ranges.  Converting to `IntervalSet` should take care of this for us.



src/common/values.cpp (lines 405 - 406)


You could inline these two.



src/tests/values_tests.cpp (line 339)


Missing a comma after the second interval.



src/tests/values_tests.cpp (lines 362 - 364)


Here, you have (closed, open) bounds.

In the implementation, you (correctly) use (closed, closed) bounds.  
Consider normalizing.


- Joseph Wu


On July 7, 2016, 6:42 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 7, 2016, 6:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-08 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [48593]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 8, 2016, 1:42 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 8, 2016, 1:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-07 Thread Yanyan Hu 


> On July 6, 2016, 8:41 a.m., Guangya Liu wrote:
> > Another comment is that you may want to add a benchmark test case for this 
> > smiliar as 
> > https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3299
> 
> Yanyan Hu wrote:
> Hi, Guangya, thanks a lot for your comments. Will revise the patch per 
> your suggestion.
> 
> Joseph Wu wrote:
> @Yanyan, have you published a revised patch?  Most of the issues you've 
> marked as fixed are not actually fixed.

Hi, Joseph, sorry forgot to publish it...


- Yanyan


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


On July 8, 2016, 1:42 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 8, 2016, 1:42 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-07 Thread Yanyan Hu 

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

(Updated July 8, 2016, 1:42 a.m.)


Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
Remoortere.


Changes
---

Fix issues mentioned by GuangYa.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/48593/diff/


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-07 Thread Joseph Wu 


> On July 6, 2016, 1:41 a.m., Guangya Liu wrote:
> > Another comment is that you may want to add a benchmark test case for this 
> > smiliar as 
> > https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3299
> 
> Yanyan Hu wrote:
> Hi, Guangya, thanks a lot for your comments. Will revise the patch per 
> your suggestion.

@Yanyan, have you published a revised patch?  Most of the issues you've marked 
as fixed are not actually fixed.


- Joseph


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


On July 6, 2016, 3:18 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 6, 2016, 3:18 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-07 Thread Yanyan Hu 


> On July 6, 2016, 8:41 a.m., Guangya Liu wrote:
> > Another comment is that you may want to add a benchmark test case for this 
> > smiliar as 
> > https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3299

Hi, Guangya, thanks a lot for your comments. Will revise the patch per your 
suggestion.


- Yanyan


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


On July 6, 2016, 10:18 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated July 6, 2016, 10:18 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Guangya Liu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-06 Thread Guangya Liu 


> On 七月 6, 2016, 8:36 a.m., Guangya Liu wrote:
> > src/tests/values_tests.cpp, line 360
> > 
> >
> > s/{[1-2), [3-5), [7-10)}/{[1,2)[3-5)[7,10)}

s/{[1-2), [3-5), [7-10)}/{[1,2)[3,5)[7,10)}


- Guangya


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


On 六月 15, 2016, 3:39 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated 六月 15, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-06 Thread Guangya Liu 

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



Another comment is that you may want to add a benchmark test case for this 
smiliar as 
https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3299

- Guangya Liu


On 六月 15, 2016, 3:39 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated 六月 15, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-07-06 Thread Guangya Liu 

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




src/common/values.cpp (line 270)


Period to the end.



src/common/values.cpp (line 276)


what about 

for(int i = 0; i < ranges.range_size(); i++) {

and remove L273



src/common/values.cpp (line 285)


Period to the end.



src/tests/values_tests.cpp (lines 33 - 34)


a new line here



src/tests/values_tests.cpp (line 316)


s/Unit test/Test



src/tests/values_tests.cpp (line 338)


s/{[1,2), [3-6), [7,9)}/{[1,2)[3,6)[7,9)}



src/tests/values_tests.cpp (line 341)


s/1U/1

ditto for others



src/tests/values_tests.cpp (line 354)


s/Unit test/Test



src/tests/values_tests.cpp (line 360)


s/{[1-2), [3-5), [7-10)}/{[1,2)[3-5)[7,10)}


- Guangya Liu


On 六月 15, 2016, 3:39 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated 六月 15, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-21 Thread Yanyan Hu 


> On June 13, 2016, 12:02 p.m., Joris Van Remoortere wrote:
> >
> 
> Yanyan Hu wrote:
> Hi, Joris, so sorry for this late reply. I have made some further 
> benchmarking and no negative impact was found when evaluating this change on 
> other existing test cases, especially those for resource and allocator. And I 
> have proposed another follow-up JIRA to state this is just an interim fix and 
> we expect c++ based resource definition as permanent solution. Thank you so 
> much.

Link of new JIRA: https://issues.apache.org/jira/browse/MESOS-5681


- Yanyan


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


On June 15, 2016, 3:39 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 15, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-21 Thread Yanyan Hu 


> On June 13, 2016, 12:02 p.m., Joris Van Remoortere wrote:
> >

Hi, Joris, so sorry for this late reply. I have made some further benchmarking 
and no negative impact was found when evaluating this change on other existing 
test cases, especially those for resource and allocator. And I have proposed 
another follow-up JIRA to state this is just an interim fix and we expect c++ 
based resource definition as permanent solution. Thank you so much.


- Yanyan


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


On June 15, 2016, 3:39 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 15, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-15 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [48593]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 15, 2016, 3:39 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 15, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-14 Thread Yanyan Hu 

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

(Updated June 15, 2016, 3:39 a.m.)


Review request for mesos, Guangya Liu and Joris Van Remoortere.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/48593/diff/


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-14 Thread Yanyan Hu 

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

(Updated June 15, 2016, 3:36 a.m.)


Review request for mesos, Guangya Liu and Joris Van Remoortere.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs (updated)
-

  3rdparty/Makefile.am 485ed550c37b092763dc5d616a8b69cdff5a3acb 
  3rdparty/libprocess/src/pid.cpp f9313cde006dd067be265343eed60412ad6b0b95 
  3rdparty/stout/include/stout/flags/flags.hpp 
dd9362772d1fbd32638fc7e70126fd49d4a03c68 
  CHANGELOG 0df0d4ccd76379ede6492e595999711e5462f79b 
  docs/agent-recovery.md 6d1b402ae5a3e26832d4fa665ca3666d4c096bc2 
  docs/allocation-module.md 6cb1392aff9eeb5cb4a8f14c91df50f29d25948c 
  docs/authorization.md dcf2160424771c513579063911cc14792f464821 
  docs/documentation-guide.md fd9f5eeecb70381ae80d70ae74866278587a9078 
  docs/executor-http-api.md 17ba6f6a0ecbd271e6b1739ffaf92fba83c0fac1 
  docs/external-containerizer.md eece9e73d47c13cc50074ef77235147f09cd380a 
  docs/high-availability.md e467a0e1d592fa1aa8d8746f59cb753f28fc30e2 
  docs/markdown-style-guide.md 667d6dfbcf6c0864085cdfb19f4e53fb49a49c44 
  docs/mesos-containerizer.md 8dcf1af819741118604af876ad612737b8ff7b32 
  docs/monitoring.md 3cfdaf5de149c5a1a7b3d64ba2870887a092ffc6 
  docs/network-monitoring.md ee999ebbb61c8bb1f38599a0391885cdadff989f 
  docs/operational-guide.md ad858e359bd94b2c5eec397ecbad3f15e0066f5e 
  docs/persistent-volume.md a9775b187637bd6d2d90ab03613b6bb6dc1454bf 
  docs/reservation.md 6408646cd42408514566e323beff548b69b6af41 
  docs/scheduler-http-api.md 4be961214c63e4e3b25c5c350b2c4f0e66863817 
  docs/upgrades.md afc76f7a0e74042bd095b925f8119bfa5daa489c 
  docs/versioning.md da874143b0faed47e388d61aa6a3580711053aea 
  include/mesos/docker/spec.hpp 92367cf942c7ee4c23b1265b659f01d427212f62 
  include/mesos/docker/spec.proto 734395cbde81bc4ff99f1310fc4b913beeb5b11f 
  site/source/blog/2016-06-13-mesos-0-28-2-released.md 
64733af9efd3d0c1d05229582f31fca15158c13c 
  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/docker/spec.cpp 2711578626dd1847f73cbf7bd3771f36e6755a99 
  src/examples/balloon_executor.cpp 7343ee72c1fc8d6e58527809ffb74fc5dd09ee0e 
  src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
  src/files/files.cpp 20bc6fa0c22ab017c4e23a745c313a3caf0aec36 
  src/master/http.cpp db625f0d656f207a89fcc14b18ae2fc31d30e673 
  src/master/master.hpp a0944ddccd3a4b33458cd2489bb5fcdbbdc55720 
  src/master/master.cpp 1971e9bf3875ceb117130b84533dc37873ca60df 
  src/slave/containerizer/docker.cpp bd4f5930684208d532f4ad1721aae353d309c0a7 
  src/slave/containerizer/mesos/containerizer.cpp 
915e4cdf4fed345515bffbf7cc5147d161640ede 
  src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
78ba4207c78fe51000e1fef487a57a1ab272d43a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
9014fc80e326e585a4f85eb0104de87156195f6d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
ad56cfad823fcd8244e9d9f7bb8b801228c1f06e 
  src/slave/http.cpp 93c23a0b31193b17acd09337337530e984803b48 
  src/slave/slave.hpp ef171188afb7169e7f386547af9fa334a65374af 
  src/slave/slave.cpp 0af04d6fe53f92e03905fb7b3bec72b09d5e8e57 
  src/tests/balloon_framework_test.sh e0b0b04c9e86614cd3696f34d1de2b9f981da840 
  src/tests/containerizer/docker_spec_tests.cpp 
1c6473aa9b9c2e56d32d10dc88d9bc66f563d91e 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
8d24583fef5a1f99763ece26ba55d3b389011a48 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
  src/uri/fetchers/docker.hpp 6cb57be97d9494ea59ce9759e3d52e37b19d6c43 
  src/uri/fetchers/docker.cpp 211be6fdf416621c467faa5592c9e52a49b450ca 

Diff: https://reviews.apache.org/r/48593/diff/


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-14 Thread Yanyan Hu 


> On June 13, 2016, 12:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, lines 409-420
> > 
> >
> > Thanks for digging into this issue further.
> > Please do read my response on your e-mail thread for further context.
> > 
> > This patch looks like it fixes a particular bottleneck by converting to 
> > intervalset inline.
> > I understand why you did this, as my e-mail outlines that changing the 
> > stored data-type is more complicated.
> > 
> > I can see how this improves performance; however, is the resource 
> > validation of ranges also not a problem?
> > 
> > Your benchmarks are specific to this operation. Have you tried this 
> > patch your example 10K task / 20 node cluster to identify if the validation 
> > is still a bottleneck?
> > 
> > If we run all the benchmarks in the test suite, are all of them 
> > positively impacted? Or are some negatively?
> > 
> > If this is purely a win, then we can consider this approach as a 
> > temporary fix assuming the following:
> > - Simple patch (yes! :-D )
> > - A JIRA to follow up with changing the stored data-type so we don't 
> > treat this as a permanent solution

Hi, Joris, thank you so much for your comment! Understand your consideration 
and totally agree that refactoring the resource definition should be the 
permanent solution. I have made a test by running 10K tasks in 20 nodes and 
didn't found performance bottleneck caused by resource validation. Mesos 
allocator now works very efficiently in that scale. And I will make further 
tests to evalute the impact on other test cases to see whether there is new 
performance regression induced.

BTW, is there any approach to analyze the benchmark result(time consumption) 
automatically or I need to compare the difference manually?


- Yanyan


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


On June 13, 2016, 12:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 13, 2016, 12:03 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-13 Thread Joris Van Remoortere 

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




src/common/values.cpp (lines 399 - 410)


Thanks for digging into this issue further.
Please do read my response on your e-mail thread for further context.

This patch looks like it fixes a particular bottleneck by converting to 
intervalset inline.
I understand why you did this, as my e-mail outlines that changing the 
stored data-type is more complicated.

I can see how this improves performance; however, is the resource 
validation of ranges also not a problem?

Your benchmarks are specific to this operation. Have you tried this patch 
your example 10K task / 20 node cluster to identify if the validation is still 
a bottleneck?

If we run all the benchmarks in the test suite, are all of them positively 
impacted? Or are some negatively?

If this is purely a win, then we can consider this approach as a temporary 
fix assuming the following:
- Simple patch (yes! :-D )
- A JIRA to follow up with changing the stored data-type so we don't treat 
this as a permanent solution


- Joris Van Remoortere


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-13 Thread Yanyan Hu 


> On June 13, 2016, 3:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > 
> >
> > Any data on performance improvement?
> 
> Yanyan Hu wrote:
> Hi, Klaus, performance data is listed here: 
> https://issues.apache.org/jira/browse/MESOS-5425# thanks.
> 
> Klaus Ma wrote:
> Thanks; according to the data. It seems worse performance when # < 200, 
> but better performance when # > 200. I'm not sure which is general case in 
> production :).
> 
> Yanyan Hu wrote:
> Hi, if you refer to the latest result, the performance is always better 
> than current implemenation :)
> 
> https://issues.apache.org/jira/browse/MESOS-5425?focusedCommentId=15303641=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15303641
> 
> Klaus Ma wrote:
> Great! would you add some benchmark test for this patch? I think current 
> test cases has covered the general feature, but not benchmark for it :).

Yes, I'm working on it. Will post it later :)


- Yanyan


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


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-13 Thread Klaus Ma 


> On June 13, 2016, 11:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > 
> >
> > Any data on performance improvement?
> 
> Yanyan Hu wrote:
> Hi, Klaus, performance data is listed here: 
> https://issues.apache.org/jira/browse/MESOS-5425# thanks.
> 
> Klaus Ma wrote:
> Thanks; according to the data. It seems worse performance when # < 200, 
> but better performance when # > 200. I'm not sure which is general case in 
> production :).
> 
> Yanyan Hu wrote:
> Hi, if you refer to the latest result, the performance is always better 
> than current implemenation :)
> 
> https://issues.apache.org/jira/browse/MESOS-5425?focusedCommentId=15303641=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15303641

Great! would you add some benchmark test for this patch? I think current test 
cases has covered the general feature, but not benchmark for it :).


- Klaus


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


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-13 Thread Yanyan Hu 


> On June 13, 2016, 3:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > 
> >
> > Any data on performance improvement?
> 
> Yanyan Hu wrote:
> Hi, Klaus, performance data is listed here: 
> https://issues.apache.org/jira/browse/MESOS-5425# thanks.
> 
> Klaus Ma wrote:
> Thanks; according to the data. It seems worse performance when # < 200, 
> but better performance when # > 200. I'm not sure which is general case in 
> production :).

Hi, if you refer to the latest result, the performance is always better than 
current implemenation :)
https://issues.apache.org/jira/browse/MESOS-5425?focusedCommentId=15303641=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15303641


- Yanyan


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


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-13 Thread Klaus Ma 


> On June 13, 2016, 11:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > 
> >
> > Any data on performance improvement?
> 
> Yanyan Hu wrote:
> Hi, Klaus, performance data is listed here: 
> https://issues.apache.org/jira/browse/MESOS-5425# thanks.

Thanks; according to the data. It seems worse performance when # < 200, but 
better performance when # > 200. I'm not sure which is general case in 
production :).


- Klaus


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


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-12 Thread Yanyan Hu 


> On June 12, 2016, 9:32 a.m., Guangya Liu wrote:
> > Hi Yanyan, can you please add some test cases for your code chage? You can 
> > refer to 
> > https://github.com/apache/mesos/blob/master/src/tests/values_tests.cpp for 
> > detail.
> > 
> > You can take a look at 
> > https://github.com/apache/mesos/blob/master/docs/newbie-guide.md#making-changes
> >  for how to run a specified unit test case.

Thanks, Guangya, will add some test cases for this change.


- Yanyan


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


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-12 Thread Yanyan Hu 


> On June 13, 2016, 3:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > 
> >
> > Any data on performance improvement?

Hi, Klaus, performance data is listed here: 
https://issues.apache.org/jira/browse/MESOS-5425# thanks.


- Yanyan


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


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-12 Thread Klaus Ma 

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




src/common/values.cpp (lines 401 - 409)


Any data on performance improvement?


- Klaus Ma


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-12 Thread Guangya Liu 

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



Hi Yanyan, can you please add some test cases for your code chage? You can 
refer to https://github.com/apache/mesos/blob/master/src/tests/values_tests.cpp 
for detail.

You can take a look at 
https://github.com/apache/mesos/blob/master/docs/newbie-guide.md#making-changes 
for how to run a specified unit test case.

- Guangya Liu


On 六月 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated 六月 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-12 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [48593]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-12 Thread Yanyan Hu 

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

(Updated June 12, 2016, 8:03 a.m.)


Review request for mesos and Guangya Liu.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 

Diff: https://reviews.apache.org/r/48593/diff/


Testing
---

make check


Thanks,

Yanyan Hu

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-12 Thread Yanyan Hu 


> On June 12, 2016, 3:35 a.m., haosdent huang wrote:
> > src/common/values.cpp, line 310
> > 
> >
> > Usually we perfer to return `Value::Ranges` directly.
> > ```
> > Value::Ranges IntervalSet2Ranges(const IntervalSet& set)
> > ```
> > 
> > In additional, `convertToRanges` maybe a better name

Yes, that is better. Will change.


> On June 12, 2016, 3:35 a.m., haosdent huang wrote:
> > src/common/values.cpp, line 313
> > 
> >
> > I think we could use foreach here?

Thanks, will change.


- Yanyan


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


On June 12, 2016, 3:28 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-11 Thread Mesos ReviewBot 

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On June 12, 2016, 3:28 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

2016-06-11 Thread haosdent huang 

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




src/common/values.cpp (line 282)


Usually we perfer to return `Value::Ranges` directly.
```
Value::Ranges IntervalSet2Ranges(const IntervalSet& set)
```

In additional, `convertToRanges` maybe a better name



src/common/values.cpp (line 285)


I think we could use foreach here?


- haosdent huang


On June 12, 2016, 3:28 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Review Request 48593: Refactor Ranges Subtraction.

2016-06-11 Thread Yanyan Hu 

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

Review request for mesos.


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


Repository: mesos


Description
---

This patch reimplement Ranges subtraction using
IntervalSet data type: Ranges values will be
converted to IntervalSet values for subtraction
and the result will be converted back to Ranges
after subtraction is done. This change is for
fixing jira MESOS-5425.


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 

Diff: https://reviews.apache.org/r/48593/diff/


Testing
---

make check


Thanks,

Yanyan Hu