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,

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

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:

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

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

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,

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)

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:

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

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

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

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

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

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

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)

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

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

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:

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

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

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

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)

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: >

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: >

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: >

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: >

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 > >

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:

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)

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

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:

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.

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

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.

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)

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