You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2015/03/24 01:27:10 UTC

Review Request 32419: Used move assignment in the master's json endpoints.

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

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


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32419/#review77513
-----------------------------------------------------------


Patch looks great!

Reviews applied: [32419]

All tests passed.

- Mesos ReviewBot


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32419/#review77499
-----------------------------------------------------------

Ship it!


LGTM!

- Jie Yu


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Cody Maloney <co...@mesosphere.io>.

> On March 24, 2015, 4:10 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 470
> > <https://reviews.apache.org/r/32419/diff/1/?file=903657#file903657line470>
> >
> >     Are you sure the outer `object` is not shadowed? Even if it's not, let us maybe leave the original naming to avoid name collision and therefore confusion.
> 
> Ben Mahler wrote:
>     Yikes, good eye alex, this looks like a bug. I'll leave it as it was, or I'll update the names to be "flags", "slaves", etc instead of "object", "array", etc.
> 
> Ben Mahler wrote:
>     I'll also add a test for the miscellaneous items always present in state.json, if one doesn't exist already.
> 
> Alexander Rukletsov wrote:
>     That would be great! In the meanwhile I'll try to do what I have promised: test this change (including gcc4.4) and get some numbers.

Just as a side note, would be nice if we could enable `-Wshadow` to catch this on GCC > 4.8 & clang

See: https://gcc.gnu.org/gcc-4.8/changes.html for why 4.8+


- Cody


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


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Ben Mahler <be...@gmail.com>.

> On March 24, 2015, 4:10 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 470
> > <https://reviews.apache.org/r/32419/diff/1/?file=903657#file903657line470>
> >
> >     Are you sure the outer `object` is not shadowed? Even if it's not, let us maybe leave the original naming to avoid name collision and therefore confusion.
> 
> Ben Mahler wrote:
>     Yikes, good eye alex, this looks like a bug. I'll leave it as it was, or I'll update the names to be "flags", "slaves", etc instead of "object", "array", etc.

I'll also add a test for the miscellaneous items always present in state.json, if one doesn't exist already.


- Ben


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


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On March 24, 2015, 4:10 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 470
> > <https://reviews.apache.org/r/32419/diff/1/?file=903657#file903657line470>
> >
> >     Are you sure the outer `object` is not shadowed? Even if it's not, let us maybe leave the original naming to avoid name collision and therefore confusion.
> 
> Ben Mahler wrote:
>     Yikes, good eye alex, this looks like a bug. I'll leave it as it was, or I'll update the names to be "flags", "slaves", etc instead of "object", "array", etc.
> 
> Ben Mahler wrote:
>     I'll also add a test for the miscellaneous items always present in state.json, if one doesn't exist already.

That would be great! In the meanwhile I'll try to do what I have promised: test this change (including gcc4.4) and get some numbers.


- Alexander


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


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Ben Mahler <be...@gmail.com>.

> On March 24, 2015, 4:10 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, line 470
> > <https://reviews.apache.org/r/32419/diff/1/?file=903657#file903657line470>
> >
> >     Are you sure the outer `object` is not shadowed? Even if it's not, let us maybe leave the original naming to avoid name collision and therefore confusion.

Yikes, good eye alex, this looks like a bug. I'll leave it as it was, or I'll update the names to be "flags", "slaves", etc instead of "object", "array", etc.


- Ben


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


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

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


Looks good, but I would like to get some numbers especially on the ancient but still officially supported gcc 4.4.7 before we commit this.


src/master/http.cpp
<https://reviews.apache.org/r/32419/#comment125704>

    Are you sure the outer `object` is not shadowed? Even if it's not, let us maybe leave the original naming to avoid name collision and therefore confusion.


- Alexander Rukletsov


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's state.json endpoint.

Posted by Michael Park <mc...@gmail.com>.

> 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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Ben Mahler <be...@gmail.com>.

> 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.

`object.values["statuses"]` is a variant JSON::Value (defaults to JSON::Null IIUC), not a JSON::Array, does that compile..?


- Ben


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


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's json endpoints.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32419/#review77657
-----------------------------------------------------------



src/common/http.cpp
<https://reviews.apache.org/r/32419/#comment125809>

    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.


- Michael Park


On March 24, 2015, 12:27 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32419/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 12:27 a.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
> 
>


Re: Review Request 32419: Used move assignment in the master's state.json endpoint.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
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.


Changes
-------

* Fixed an accidental shadowing issue.


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

Used move assignment in the master's state.json endpoint.


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 (updated)
-----

  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