You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Dmitry Zhuk <dz...@twopensource.com> on 2017/10/11 17:35:46 UTC
Review Request 62901: Used protobuf arenas for creating messages in
ProtobufProcess.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62901/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-6971
https://issues.apache.org/jira/browse/MESOS-6971
Repository: mesos
Description
-------
This changes `ProtobufProcess` to use arenas for creating messages.
Diffs
-----
3rdparty/libprocess/include/process/protobuf.hpp 2b6b623e04c6d629647287e4bdf617a6f9e8d813
Diff: https://reviews.apache.org/r/62901/diff/1/
Testing
-------
Ran benchmark
```
./benchmarks --gtest_filter=ProcessTest.Process_BENCHMARK_MessagePassing
```
before and after changes with the following results:
```
Without arenas
[ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
Size: 4, elapsed: 12.638082ms
Size: 28, elapsed: 38.13611ms
Size: 124, elapsed: 136.028145ms
Size: 508, elapsed: 514.481138ms
Size: 2048, elapsed: 2.009934749secs
Size: 8208, elapsed: 8.027707078secs
Size: 32848, elapsed: 32.188444623secs
[ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (42936 ms)
[ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
Size: 4, elapsed: 13.574658ms
Size: 28, elapsed: 38.735425ms
Size: 124, elapsed: 135.356633ms
Size: 508, elapsed: 513.202006ms
Size: 2048, elapsed: 2.016349277secs
Size: 8208, elapsed: 8.148115463secs
Size: 32848, elapsed: 32.266659276secs
[ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (43139 ms)
With arenas
[ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
Size: 4, elapsed: 17.66253ms
Size: 28, elapsed: 42.242613ms
Size: 124, elapsed: 133.215018ms
Size: 508, elapsed: 478.285954ms
Size: 2048, elapsed: 1.846806816secs
Size: 8208, elapsed: 7.807271474secs
Size: 32848, elapsed: 32.344994264secs
[ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (42678 ms)
[----------] 1 test from ProcessTest (42678 ms total)
[ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
Size: 4, elapsed: 17.651803ms
Size: 28, elapsed: 42.907844ms
Size: 124, elapsed: 134.527995ms
Size: 508, elapsed: 479.715174ms
Size: 2048, elapsed: 1.839642098secs
Size: 8208, elapsed: 7.847239368secs
Size: 32848, elapsed: 32.519693203secs
[ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (42888 ms)
[----------] 1 test from ProcessTest (42888 ms total)
```
Thanks,
Dmitry Zhuk
Re: Review Request 62901: Used protobuf arenas for creating messages
in ProtobufProcess.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62901/#review187727
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 62898.
Failed command: `python.exe .\support\apply-reviews.py -n -r 62898`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62901
Relevant logs:
- [apply-review-62898-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62901/logs/apply-review-62898-stdout.log):
```
error: patch failed: 3rdparty/libprocess/include/process/protobuf.hpp:128
error: 3rdparty/libprocess/include/process/protobuf.hpp: patch does not apply
```
- Mesos Reviewbot Windows
On Oct. 11, 2017, 10:35 a.m., Dmitry Zhuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62901/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2017, 10:35 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-6971
> https://issues.apache.org/jira/browse/MESOS-6971
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This changes `ProtobufProcess` to use arenas for creating messages.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/protobuf.hpp 2b6b623e04c6d629647287e4bdf617a6f9e8d813
>
>
> Diff: https://reviews.apache.org/r/62901/diff/1/
>
>
> Testing
> -------
>
> Ran benchmark
> ```
> ./benchmarks --gtest_filter=ProcessTest.Process_BENCHMARK_MessagePassing
> ```
> before and after changes with the following results:
> ```
> Without arenas
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 12.638082ms
> Size: 28, elapsed: 38.13611ms
> Size: 124, elapsed: 136.028145ms
> Size: 508, elapsed: 514.481138ms
> Size: 2048, elapsed: 2.009934749secs
> Size: 8208, elapsed: 8.027707078secs
> Size: 32848, elapsed: 32.188444623secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (42936 ms)
>
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 13.574658ms
> Size: 28, elapsed: 38.735425ms
> Size: 124, elapsed: 135.356633ms
> Size: 508, elapsed: 513.202006ms
> Size: 2048, elapsed: 2.016349277secs
> Size: 8208, elapsed: 8.148115463secs
> Size: 32848, elapsed: 32.266659276secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (43139 ms)
>
>
> With arenas
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 17.66253ms
> Size: 28, elapsed: 42.242613ms
> Size: 124, elapsed: 133.215018ms
> Size: 508, elapsed: 478.285954ms
> Size: 2048, elapsed: 1.846806816secs
> Size: 8208, elapsed: 7.807271474secs
> Size: 32848, elapsed: 32.344994264secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (42678 ms)
> [----------] 1 test from ProcessTest (42678 ms total)
>
>
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 17.651803ms
> Size: 28, elapsed: 42.907844ms
> Size: 124, elapsed: 134.527995ms
> Size: 508, elapsed: 479.715174ms
> Size: 2048, elapsed: 1.839642098secs
> Size: 8208, elapsed: 7.847239368secs
> Size: 32848, elapsed: 32.519693203secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (42888 ms)
> [----------] 1 test from ProcessTest (42888 ms total)
>
> ```
>
>
> Thanks,
>
> Dmitry Zhuk
>
>
Re: Review Request 62901: Used protobuf arenas for creating messages
in ProtobufProcess.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62901/#review187877
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/include/process/protobuf.hpp
Lines 236-237 (original), 237-238 (patched)
<https://reviews.apache.org/r/62901/#comment264910>
How about an explicit CHECK_NOTNULL?
```
M* m = CHECK_NOTNULL(google::protobuf::Arena::CreateMessage<M>(&arena));
```
- Benjamin Mahler
On Oct. 12, 2017, 1:10 p.m., Dmitry Zhuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62901/
> -----------------------------------------------------------
>
> (Updated Oct. 12, 2017, 1:10 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-6971
> https://issues.apache.org/jira/browse/MESOS-6971
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This changes `ProtobufProcess` to use arenas for creating messages.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/protobuf.hpp e46a076b19de711900f9023ae5c53db45951b547
>
>
> Diff: https://reviews.apache.org/r/62901/diff/2/
>
>
> Testing
> -------
>
> Ran benchmark
> ```
> ./benchmarks --gtest_filter=ProcessTest.Process_BENCHMARK_MessagePassing
> ```
> before and after changes with the following results:
> ```
> Without arenas
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 14.800662ms
> Size: 10, elapsed: 23.607056ms
> Size: 34, elapsed: 53.234174ms
> Size: 64, elapsed: 86.6773ms
> Size: 154, elapsed: 191.359858ms
> Size: 304, elapsed: 330.10146ms
> Size: 605, elapsed: 612.198306ms
> Size: 1507, elapsed: 1.515456025secs
> Size: 3011, elapsed: 3.006522946secs
> Size: 4515, elapsed: 4.532226307secs
> Size: 6019, elapsed: 6.045134458secs
> Size: 9027, elapsed: 9.077230263secs
> Size: 12034, elapsed: 12.156748523secs
> Size: 15042, elapsed: 15.199843166secs
> Size: 30081, elapsed: 30.394480817secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (83252 ms)
>
>
> With arenas
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 18.910807ms
> Size: 10, elapsed: 28.725021ms
> Size: 34, elapsed: 53.235732ms
> Size: 64, elapsed: 86.577933ms
> Size: 154, elapsed: 157.530026ms
> Size: 304, elapsed: 284.589934ms
> Size: 605, elapsed: 541.077226ms
> Size: 1507, elapsed: 1.311257359secs
> Size: 3011, elapsed: 2.591459371secs
> Size: 4515, elapsed: 3.879464983secs
> Size: 6019, elapsed: 5.168277919secs
> Size: 9027, elapsed: 8.265426958secs
> Size: 12034, elapsed: 11.087320011secs
> Size: 15042, elapsed: 13.854052262secs
> Size: 30081, elapsed: 28.361437857secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (75702 ms)
> ```
>
>
> Thanks,
>
> Dmitry Zhuk
>
>
Re: Review Request 62901: Used protobuf arenas for creating messages
in ProtobufProcess.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62901/#review187790
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 62898.
Failed command: `python.exe .\support\apply-reviews.py -n -r 62898`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62901
Relevant logs:
- [apply-review-62898-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62901/logs/apply-review-62898-stdout.log):
```
error: patch failed: 3rdparty/libprocess/include/process/protobuf.hpp:128
error: 3rdparty/libprocess/include/process/protobuf.hpp: patch does not apply
```
- Mesos Reviewbot Windows
On Oct. 12, 2017, 1:10 p.m., Dmitry Zhuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62901/
> -----------------------------------------------------------
>
> (Updated Oct. 12, 2017, 1:10 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-6971
> https://issues.apache.org/jira/browse/MESOS-6971
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This changes `ProtobufProcess` to use arenas for creating messages.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/protobuf.hpp e46a076b19de711900f9023ae5c53db45951b547
>
>
> Diff: https://reviews.apache.org/r/62901/diff/2/
>
>
> Testing
> -------
>
> Ran benchmark
> ```
> ./benchmarks --gtest_filter=ProcessTest.Process_BENCHMARK_MessagePassing
> ```
> before and after changes with the following results:
> ```
> Without arenas
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 14.800662ms
> Size: 10, elapsed: 23.607056ms
> Size: 34, elapsed: 53.234174ms
> Size: 64, elapsed: 86.6773ms
> Size: 154, elapsed: 191.359858ms
> Size: 304, elapsed: 330.10146ms
> Size: 605, elapsed: 612.198306ms
> Size: 1507, elapsed: 1.515456025secs
> Size: 3011, elapsed: 3.006522946secs
> Size: 4515, elapsed: 4.532226307secs
> Size: 6019, elapsed: 6.045134458secs
> Size: 9027, elapsed: 9.077230263secs
> Size: 12034, elapsed: 12.156748523secs
> Size: 15042, elapsed: 15.199843166secs
> Size: 30081, elapsed: 30.394480817secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (83252 ms)
>
>
> With arenas
> [ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
> Size: 4, elapsed: 18.910807ms
> Size: 10, elapsed: 28.725021ms
> Size: 34, elapsed: 53.235732ms
> Size: 64, elapsed: 86.577933ms
> Size: 154, elapsed: 157.530026ms
> Size: 304, elapsed: 284.589934ms
> Size: 605, elapsed: 541.077226ms
> Size: 1507, elapsed: 1.311257359secs
> Size: 3011, elapsed: 2.591459371secs
> Size: 4515, elapsed: 3.879464983secs
> Size: 6019, elapsed: 5.168277919secs
> Size: 9027, elapsed: 8.265426958secs
> Size: 12034, elapsed: 11.087320011secs
> Size: 15042, elapsed: 13.854052262secs
> Size: 30081, elapsed: 28.361437857secs
> [ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (75702 ms)
> ```
>
>
> Thanks,
>
> Dmitry Zhuk
>
>
Re: Review Request 62901: Used protobuf arenas for creating messages
in ProtobufProcess.
Posted by Dmitry Zhuk <dz...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62901/
-----------------------------------------------------------
(Updated Oct. 12, 2017, 1:10 p.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Updated benchmark results.
Bugs: MESOS-6971
https://issues.apache.org/jira/browse/MESOS-6971
Repository: mesos
Description
-------
This changes `ProtobufProcess` to use arenas for creating messages.
Diffs (updated)
-----
3rdparty/libprocess/include/process/protobuf.hpp e46a076b19de711900f9023ae5c53db45951b547
Diff: https://reviews.apache.org/r/62901/diff/2/
Changes: https://reviews.apache.org/r/62901/diff/1-2/
Testing (updated)
-------
Ran benchmark
```
./benchmarks --gtest_filter=ProcessTest.Process_BENCHMARK_MessagePassing
```
before and after changes with the following results:
```
Without arenas
[ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
Size: 4, elapsed: 14.800662ms
Size: 10, elapsed: 23.607056ms
Size: 34, elapsed: 53.234174ms
Size: 64, elapsed: 86.6773ms
Size: 154, elapsed: 191.359858ms
Size: 304, elapsed: 330.10146ms
Size: 605, elapsed: 612.198306ms
Size: 1507, elapsed: 1.515456025secs
Size: 3011, elapsed: 3.006522946secs
Size: 4515, elapsed: 4.532226307secs
Size: 6019, elapsed: 6.045134458secs
Size: 9027, elapsed: 9.077230263secs
Size: 12034, elapsed: 12.156748523secs
Size: 15042, elapsed: 15.199843166secs
Size: 30081, elapsed: 30.394480817secs
[ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (83252 ms)
With arenas
[ RUN ] ProcessTest.Process_BENCHMARK_MessagePassing
Size: 4, elapsed: 18.910807ms
Size: 10, elapsed: 28.725021ms
Size: 34, elapsed: 53.235732ms
Size: 64, elapsed: 86.577933ms
Size: 154, elapsed: 157.530026ms
Size: 304, elapsed: 284.589934ms
Size: 605, elapsed: 541.077226ms
Size: 1507, elapsed: 1.311257359secs
Size: 3011, elapsed: 2.591459371secs
Size: 4515, elapsed: 3.879464983secs
Size: 6019, elapsed: 5.168277919secs
Size: 9027, elapsed: 8.265426958secs
Size: 12034, elapsed: 11.087320011secs
Size: 15042, elapsed: 13.854052262secs
Size: 30081, elapsed: 28.361437857secs
[ OK ] ProcessTest.Process_BENCHMARK_MessagePassing (75702 ms)
```
Thanks,
Dmitry Zhuk