> On July 28, 2016, 6:43 p.m., Guangya Liu wrote:
> > src/tests/resources_tests.cpp, lines 24802487
> > <https://reviews.apache.org/r/50205/diff/1/?file=1447686#file1447686line2480>
> >
> > I prefer that you adjust the position of `shared` to the place where it was
used.
> >
> > ```
> > // Test a typical vector of scalars which include shared resources
> > // (viz, shared persistent volumes).
> > Resource disk = createDiskResource(
> > "256", "test", "persistentId", "/volume", None(), true);
> >
> > Parameter shared;
> > shared.resources = Resources::parse("cpus:1;mem:128").get() + disk;
> > shared.totalOperations = 50000;
> > ```
>
> Anindya Sinha wrote:
> Both are equivalent, but I will change it so as to match the flow of existing code
in this function.
I don't think we need to do this.
It's very clear as it is that `Parameter shared;` starts the section that prepares the variable.
The rest of the section fills in the variable's members. The rest of the method is similar
in this regard. The fact that `disk` is defined separately is just for cosmetic reasons (we
could have inlined it but the indentation would look a bit awkward.
This is different from the antipattern that defines the variable far away from where it's
first used (which results in poor readability).
 Jiang Yan

This is an automatically generated email. To reply, visit:
https://reviews.apache.org/r/50205/#review144047

On July 30, 2016, 12:03 a.m., Anindya Sinha wrote:
>
> 
> This is an automatically generated email. To reply, visit:
> https://reviews.apache.org/r/50205/
> 
>
> (Updated July 30, 2016, 12:03 a.m.)
>
>
> Review request for mesos, Klaus Ma and Jiang Yan Xu.
>
>
> Bugs: MESOS4892
> https://issues.apache.org/jira/browse/MESOS4892
>
>
> Repository: mesos
>
>
> Description
> 
>
> Enhanced benchmark test for resources to include shared resources.
>
>
> Diffs
> 
>
> src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388
>
> Diff: https://reviews.apache.org/r/50205/diff/
>
>
> Testing
> 
>
> Tests passed. Results for resources benchmark is as follows:
>
> Minimal impact seen in Resources arithmetic with the Resources refactor changes to incorporate
shared resources.
>
> With shared resources patch (note that 4th test below is for shared resources for scalars)
>
> [] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (806 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17032 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (1048 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3 (940 ms)
> [] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test (19826 ms total)
>
> HEAD
>
> [] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (726 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17413 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (1026 ms)
> [] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (19165 ms total)
>
> Output from stdout is:
>
> [==========] Running 4 tests from 1 test case.
> [] Global test environment setup.
> [] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> Took 72635us to perform 50000 'total += r' operations on cpus(*):1; gpus(*):1; mem(*):128;
disk(*):256
> Took 116027us to perform 50000 'total = r' operations on cpus(*):1; gpus(*):1; mem(*):128;
disk(*):256
> Took 283213us to perform 50000 'total = total + r' operations on cpus(*):1; gpus(*):1;
mem(*):128; disk(*):256
> Took 333641us to perform 50000 'total = total  r' operations on cpus(*):1; gpus(*):1;
mem(*):128; disk(*):256
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (806 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> Took 4.103766secs to perform 10 'total += r' operations on cpus(0, principal_0, {key_0:
value_0}):1; gpus(...
> Took 4.310104secs to perform 10 'total = r' operations on cpus(0, principal_0, {key_0:
value_0}):1; gpus(...
> Took 4186ms to perform 10 'total = total + r' operations on cpus(0, principal_0, {key_0:
value_0}):1; gpus(...
> Took 4.380353secs to perform 10 'total = total  r' operations on cpus(0, principal_0,
{key_0: value_0}):1; gpus(...
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17032 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> Took 483119us to perform 1000 'total += r' operations on ports(*):[12, 45, 78, 1011,
1314, 1617, 1...
> Took 2271us to perform 1000 'total = r' operations on ports(*):[12, 45, 78, 1011,
1314, 1617, 1...
> Took 559844us to perform 1000 'total = total + r' operations on ports(*):[12, 45, 78,
1011, 1314, 1617, 1...
> Took 2423us to perform 1000 'total = total  r' operations on ports(*):[12, 45, 78,
1011, 1314, 1617, 1...
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (1048 ms)
> [ RUN ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3
> Took 61629us to perform 50000 'total += r' operations on cpus(*):1; mem(*):128; disk(test)[persistentId:...
> Took 98664us to perform 50000 'total = r' operations on cpus(*):1; mem(*):128; disk(test)[persistentId:...
> Took 348041us to perform 50000 'total = total + r' operations on cpus(*):1; mem(*):128;
disk(test)[persistentId:...
> Took 431057us to perform 50000 'total = total  r' operations on cpus(*):1; mem(*):128;
disk(test)[persistentId:...
> [ OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3 (940 ms)
> [] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test (19826 ms total)
>
> [] Global test environment teardown
> [==========] 4 tests from 1 test case ran. (19836 ms total)
> [ PASSED ] 4 tests.
>
>
> Thanks,
>
> Anindya Sinha
>
>
