mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Park <mp...@apache.org>
Subject Re: Review Request 43023: Avoid construction of temporary strings in `NumberWriter` for doubles.
Date Sun, 31 Jan 2016 04:51:43 GMT


> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 162
> > <https://reviews.apache.org/r/43023/diff/1/?file=1227235#file1227235line162>
> >
> >     I assume we have some evidence that this optimization is warranted?
> 
> Michael Park wrote:
>     With this patch + https://reviews.apache.org/r/43024/, the # of calls to `operator
new` and `operator delete` reduces by roughly 1/3.
> 
> Neil Conway wrote:
>     Can we add this to the commit message?

Yep, added to the description for now.


> On Jan. 31, 2016, 1:43 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp, line 173
> > <https://reviews.apache.org/r/43023/diff/1/?file=1227235#file1227235line173>
> >
> >     This seems a little more subtle than is warranted.
> 
> Michael Park wrote:
>     The `i` part perhaps? Is it better if we were to call it `back`?
>     ```
>     *stream_ << buffer << (buffer[back] == '.' ? "0" : "");
>     ```
> 
> Neil Conway wrote:
>     Yeah -- not clear that "i" never points to the NUL character for non-empty strings,
etc. Probably would be clearer without the trinary expression.

The ternary expression was there from before though, that's not something to be "warranted"
right?
Are you ok with this if I were to `s/i/back/`? or is it not sufficient?


- Michael


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


On Jan. 31, 2016, 12:06 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43023/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4566
>     https://issues.apache.org/jira/browse/MESOS-4566
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp addec8ec6504e2a8f5b838fce3ebd4db224ab022

> 
> Diff: https://reviews.apache.org/r/43023/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message