> On March 24, 2015, 9:32 p.m., Michael Park wrote:
> > src/common/http.cpp, lines 179-187
> > <https://reviews.apache.org/r/32419/diff/1/?file=903656#file903656line179>
> >
> > Just a suggestion for an alternative pattern here.
> >
> > Rather than
> >
> > ```
> > {
> > JSON::Array array;
> > array.values.reserve(size);
> > /* Fill in array... */
> > object.values["statuses"] = std::move(array);
> > }
> > ```
> >
> > How about:
> >
> > ```
> > {
> > JSON::Array &array = object.values["statuses"];
> > array.values.reserve(size);
> > /* Fill in array... */
> > }
> > ```
> >
> > Seems to me like this code is just as readable and doing no work is
> > better than doing some work.
>
> Ben Mahler wrote:
> `object.values["statuses"]` is a variant JSON::Value (defaults to
> JSON::Null IIUC), not a JSON::Array, does that compile..?
I see. I didn't try compiling it. To make it work I guess it would have to look
something like:
```
{
object.values["statuses"] = JSON::Array();
JSON::Array &array = object.values["statuses"].as<JSON::Array>();
array.values.reserve(size);
/* Fill in array... */
}
```
which I don't like either. Never mind.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32419/#review77657
-----------------------------------------------------------
On March 24, 2015, 11:42 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
>
> (Updated March 24, 2015, 11:42 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-2353
> https://issues.apache.org/jira/browse/MESOS-2353
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is a simple attempt at improvement for MESOS-2353 in the interim of
> writing a more comprehensive JSON benchmark.
>
> Per the discussion on [r/31700](https://reviews.apache.org/r/31700/), moves
> here avoid the very expensive copies of the large JSON objects so long as
> variant has move assignment defined (or generated) and our objects have move
> generated.
>
> It appears that variant has move assignment and our json classes can have
> move assignment generated, so this should help.
>
>
> Diffs
> -----
>
> src/common/http.cpp d3b7ca3727567ff252f978720f518ad0912e533b
> src/master/http.cpp e1a87d646e9690e39a9e84ae383622018ce80401
>
> Diff: https://reviews.apache.org/r/32419/diff/
>
>
> Testing
> -------
>
> make check
>
> No benchmarking performed given the analysis from Michael in
> [r/31700](https://reviews.apache.org/r/31700/). I will also be trying this
> out on our large production clusters.
>
>
> Thanks,
>
> Ben Mahler
>
>
