You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2018/07/20 03:39:01 UTC

Review Request 67992: Avoid performance cost of ostringstream in http::OK json constructors.

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

Review request for mesos, Alexander Rukletsov and Benno Evers.


Repository: mesos


Description
-------

In the common case where no jsonp is passed this avoids an extra copy
out from the ostringstream. In the jsonp case, the performance should
be near identical or better as we now pre-reserve the space needed
for the body.


Diffs
-----

  3rdparty/libprocess/src/http.cpp cbb910282a4aff8a914dbd13ff20fc70d929c269 


Diff: https://reviews.apache.org/r/67992/diff/1/


Testing
-------

make check


Thanks,

Benjamin Mahler


Re: Review Request 67992: Avoid performance cost of ostringstream in http::OK json constructors.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67992/#review206274
-----------------------------------------------------------


Fix it, then Ship it!





3rdparty/libprocess/src/http.cpp
Line 724 (original), 720 (patched)
<https://reviews.apache.org/r/67992/#comment289177>

    Let's not use C-style casts.



3rdparty/libprocess/src/http.cpp
Lines 729 (patched)
<https://reviews.apache.org/r/67992/#comment289178>

    Ditto.


- Benno Evers


On July 20, 2018, 3:39 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67992/
> -----------------------------------------------------------
> 
> (Updated July 20, 2018, 3:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the common case where no jsonp is passed this avoids an extra copy
> out from the ostringstream. In the jsonp case, the performance should
> be near identical or better as we now pre-reserve the space needed
> for the body.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/http.cpp cbb910282a4aff8a914dbd13ff20fc70d929c269 
> 
> 
> Diff: https://reviews.apache.org/r/67992/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 67992: Avoid performance cost of ostringstream in http::OK json constructors.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67992/#review206257
-----------------------------------------------------------



FAIL: Failed to apply the dependent review: 67987.

Failed command: `python.exe .\support\python3\apply-reviews.py -n -r 67987`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1959/mesos-review-67992

Relevant logs:

- [apply-review-67987-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1959/mesos-review-67992/logs/apply-review-67987-stdout.log):

```
error: missing binary patch data for '3rdparty/rapidjson-1.1.0.tar.gz'
error: binary patch does not apply to '3rdparty/rapidjson-1.1.0.tar.gz'
error: 3rdparty/rapidjson-1.1.0.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On July 20, 2018, 3:39 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67992/
> -----------------------------------------------------------
> 
> (Updated July 20, 2018, 3:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the common case where no jsonp is passed this avoids an extra copy
> out from the ostringstream. In the jsonp case, the performance should
> be near identical or better as we now pre-reserve the space needed
> for the body.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/http.cpp cbb910282a4aff8a914dbd13ff20fc70d929c269 
> 
> 
> Diff: https://reviews.apache.org/r/67992/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 67992: Avoid performance cost of ostringstream in http::OK json constructors.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67992/#review206415
-----------------------------------------------------------


Ship it!




Ship It!

- Alexander Rukletsov


On July 20, 2018, 3:39 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67992/
> -----------------------------------------------------------
> 
> (Updated July 20, 2018, 3:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In the common case where no jsonp is passed this avoids an extra copy
> out from the ostringstream. In the jsonp case, the performance should
> be near identical or better as we now pre-reserve the space needed
> for the body.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/http.cpp cbb910282a4aff8a914dbd13ff20fc70d929c269 
> 
> 
> Diff: https://reviews.apache.org/r/67992/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>