Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-19 Thread Niklas Nielsen 

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

Ship it!


LGTM (awaiting dependent patch to land) - Ben, any concerns/issues?

- Niklas Nielsen


On Sept. 19, 2014, 12:01 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 19, 2014, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-19 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [25789, 25798]

All tests passed.

- Mesos ReviewBot


On Sept. 19, 2014, 7:01 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 19, 2014, 7:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-19 Thread Joris Van Remoortere 

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

(Updated Sept. 19, 2014, 7:01 p.m.)


Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Changes
---

Simplify strings::join() invocation.


Repository: mesos-git


Description
---

Follow up from 25079.
Reduces compile time overall by ~1/3rd. 
Move implementation of libprocess help into cpp file.
Use strings::join to refactor macro expansion that was injecting lots of 
overhead even when the expanded functions were not used.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am edbe54b 
  3rdparty/libprocess/include/process/help.hpp 4333b5b 
  3rdparty/libprocess/src/help.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/25798/diff/


Testing
---

make check on 3rdparty
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-19 Thread Benjamin Hindman 

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



3rdparty/libprocess/include/process/help.hpp


Do we need the stringstream at all? Could we simplify these with:

return strings::join("\n", std::forward(args)..., "\n");


- Benjamin Hindman


On Sept. 19, 2014, 12:26 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 19, 2014, 12:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-18 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [25789, 25798]

All tests passed.

- Mesos ReviewBot


On Sept. 19, 2014, 12:26 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 19, 2014, 12:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-18 Thread Joris Van Remoortere 

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

(Updated Sept. 19, 2014, 12:26 a.m.)


Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Changes
---

fixing style issues.


Repository: mesos-git


Description
---

Follow up from 25079.
Reduces compile time overall by ~1/3rd. 
Move implementation of libprocess help into cpp file.
Use strings::join to refactor macro expansion that was injecting lots of 
overhead even when the expanded functions were not used.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am edbe54b 
  3rdparty/libprocess/include/process/help.hpp 4333b5b 
  3rdparty/libprocess/src/help.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/25798/diff/


Testing
---

make check on 3rdparty
support/mesos-style.py


Thanks,

Joris Van Remoortere

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-18 Thread Joris Van Remoortere 


> On Sept. 18, 2014, 11:14 p.m., Niklas Nielsen wrote:
> > 3rdparty/libprocess/include/process/help.hpp, line 120
> > 
> >
> > std::endl?
> 
> Cody Maloney wrote:
> std::endl is a flush + '\n'. Note the whole codebase only uses '\n' 
> newlines, so this works well for now. If we wanted to support changing line 
> endings based on platform, there would be a lot more infrastructure needed.

std::endl is actually a manipulator that also implied flushing. It can not be 
passed as the delimiter to join, so the line would look inconsistent:
strings::join(ss, "\n", std::forward(args)...) << std::endl;


- Joris


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


On Sept. 18, 2014, 10:53 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 18, 2014, 10:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-18 Thread Cody Maloney 


> On Sept. 18, 2014, 11:14 p.m., Niklas Nielsen wrote:
> > 3rdparty/libprocess/include/process/help.hpp, line 120
> > 
> >
> > std::endl?

std::endl is a flush + '\n'. Note the whole codebase only uses '\n' newlines, 
so this works well for now. If we wanted to support changing line endings based 
on platform, there would be a lot more infrastructure needed.


- Cody


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


On Sept. 18, 2014, 10:53 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 18, 2014, 10:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-18 Thread Niklas Nielsen 

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



3rdparty/libprocess/include/process/help.hpp


&& sticks to T, so 'T&&' (consistent with current style - know that we have 
been discussing how it actually binds).

Here and below.



3rdparty/libprocess/include/process/help.hpp


Can we rename 'ss' to something more descriptive? Here and below.



3rdparty/libprocess/include/process/help.hpp


std::endl?



3rdparty/libprocess/src/help.cpp


Unused?



3rdparty/libprocess/src/help.cpp


'using std::string' and trim rest of file? :-)



3rdparty/libprocess/src/help.cpp


Is this a copy of the help text in the header? If so, let's delete it here.



3rdparty/libprocess/src/help.cpp


Two new lines between implementing functions here and rest of file.



3rdparty/libprocess/src/help.cpp


Mind indenting per 
http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/



3rdparty/libprocess/src/help.cpp


The indentation in the return here (and below) seems to be off.



3rdparty/libprocess/src/help.cpp


The indentation seem a bit off here.


- Niklas Nielsen


On Sept. 18, 2014, 3:53 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 18, 2014, 3:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-18 Thread Dominic Hamon 

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


+1

- Dominic Hamon


On Sept. 18, 2014, 3:53 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25798/
> ---
> 
> (Updated Sept. 18, 2014, 3:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Follow up from 25079.
> Reduces compile time overall by ~1/3rd. 
> Move implementation of libprocess help into cpp file.
> Use strings::join to refactor macro expansion that was injecting lots of 
> overhead even when the expanded functions were not used.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am edbe54b 
>   3rdparty/libprocess/include/process/help.hpp 4333b5b 
>   3rdparty/libprocess/src/help.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25798/diff/
> 
> 
> Testing
> ---
> 
> make check on 3rdparty
> support/mesos-style.py
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 25798: Refactor Libprocess Help to reduce compile time.

2014-09-18 Thread Joris Van Remoortere 

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

Review request for mesos, Benjamin Hindman and Niklas Nielsen.


Repository: mesos-git


Description
---

Follow up from 25079.
Reduces compile time overall by ~1/3rd. 
Move implementation of libprocess help into cpp file.
Use strings::join to refactor macro expansion that was injecting lots of 
overhead even when the expanded functions were not used.


Diffs
-

  3rdparty/libprocess/Makefile.am edbe54b 
  3rdparty/libprocess/include/process/help.hpp 4333b5b 
  3rdparty/libprocess/src/help.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/25798/diff/


Testing
---

make check on 3rdparty
support/mesos-style.py


Thanks,

Joris Van Remoortere