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 Bannier <be...@mesosphere.io> on 2017/04/10 18:22:24 UTC

Re: Review Request 58288: Added stringify overload specialized for std::string.

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

(Updated April 10, 2017, 8:22 p.m.)


Review request for mesos and Neil Conway.


Changes
-------

Addressed comment from Neil.


Summary (updated)
-----------------

Added stringify overload specialized for std::string.


Repository: mesos


Description
-------

In our code base 'stringify' serves as an explicitly named conversion
function to 'string'. The default implementation invokes a type's
'operator<<' to serialize the object to 'string' via a 'stringstream'
which is not a cheap operation.

This patch adds a trivial overload of 'stringify' for 'string' which
directly returns its argument. This helps us avoid the overhead of the
default overload. Calls of 'stringify' with 'string' argument might
appear in generic code, e.g., currently when invoking 'stringify' on a
container of 'string's.


Diffs (updated)
-----

  3rdparty/stout/include/stout/stringify.hpp 698431583d2288d3c635211e651914316bfd3ee9 


Diff: https://reviews.apache.org/r/58288/diff/2/

Changes: https://reviews.apache.org/r/58288/diff/1-2/


Testing
-------

`make check` (Fedora 25).

At `-O2` with clang-5 this saves about 90% when stringifying a `string`.


Thanks,

Benjamin Bannier


Re: Review Request 58288: Added stringify overload specialized for std::string.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58288/#review171470
-----------------------------------------------------------


Ship it!




Ship It!

- Neil Conway


On April 10, 2017, 6:22 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58288/
> -----------------------------------------------------------
> 
> (Updated April 10, 2017, 6:22 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In our code base 'stringify' serves as an explicitly named conversion
> function to 'string'. The default implementation invokes a type's
> 'operator<<' to serialize the object to 'string' via a 'stringstream'
> which is not a cheap operation.
> 
> This patch adds a trivial overload of 'stringify' for 'string' which
> directly returns its argument. This helps us avoid the overhead of the
> default overload. Calls of 'stringify' with 'string' argument might
> appear in generic code, e.g., currently when invoking 'stringify' on a
> container of 'string's.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/stringify.hpp 698431583d2288d3c635211e651914316bfd3ee9 
> 
> 
> Diff: https://reviews.apache.org/r/58288/diff/2/
> 
> 
> Testing
> -------
> 
> `make check` (Fedora 25).
> 
> At `-O2` with clang-5 this saves about 90% when stringifying a `string`.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>