You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Cody Maloney <co...@mesosphere.io> on 2015/03/27 01:38:00 UTC
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/
-----------------------------------------------------------
(Updated March 27, 2015, 12:37 a.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Add missing include.
Bugs: MESOS-292
https://issues.apache.org/jira/browse/MESOS-292
Repository: mesos
Description
-------
Split the mesos master, slave flags into header + source file to
improve compile time significantly. Should be no functional changes.
Largely copy-paste, with a little reworking to remove unnecessary
headers from the .hpp, and order headers a little more reliably
(as well as remove duplicate includes) in the .cpp.
# Impact of these changes
`time make check -j8`
Before:
make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
After:
make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
Intel i7-4790K.
The numbers aren't incredibly stable (Other software running,
overclocking enabled, etc). They are a good general measure though of
speedup.
I do a `make check` rather than just `make` because that is what devs
do in a day-to-day flow, and if runtime is significantly impacted, it
will show up.
Test steps:
```
# Warm cache
../configure --disable-python --disable-java
make check
# Timed build
rm -rf *
../configure --disable-python --disable-java
time make check
```
Note: Similar, likely greater improvements in compile time would happen
if stout were made to not be header only. Specifically <stout/os.hpp>.
Diffs (updated)
-----
src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
src/logging/flags.cpp PRE-CREATION
src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
src/master/flags.cpp PRE-CREATION
src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
src/slave/flags.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/32558/diff/
Testing
-------
make check on ArchLinux with GCC 4.9.2
make distcheck CentOS 7
Thanks,
Cody Maloney
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78621
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy Chen
On March 31, 2015, 2:16 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 2:16 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > Looks good Cody, just curious about many of the includes in the .cpp flag files, had a hard time telling how these were relevant.
Mainly just things left over from copying from the old. Removed most. Precise comments below.
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > src/master/flags.hpp, lines 405-406
> > <https://reviews.apache.org/r/32558/diff/4/?file=910387#file910387line405>
> >
> > Where is the mesos.hpp include for these? Looks like you removed it?
It was included implicitly by <mesos/module/module.hpp>. Added it back.
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > src/master/flags.cpp, lines 19-21
> > <https://reviews.apache.org/r/32558/diff/4/?file=910388#file910388line19>
> >
> > I can't tell how these are relevant to this file, is there something I'm missing?
<stout/flags.hpp>: Implicitly included through 'logging/flags.hpp'. That include may go away in time. This is included explicitly since we use add() which does a lot of template intantiation which depends directly on <stout/flags.hpp>
Dropped json, protobuf. What they were once used for is covered by "common/parse.hpp".
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > src/master/flags.cpp, lines 23-24
> > <https://reviews.apache.org/r/32558/diff/4/?file=910388#file910388line23>
> >
> > How are these being used in this file?
Removed. Included via header now.
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > src/master/flags.cpp, line 30
> > <https://reviews.apache.org/r/32558/diff/4/?file=910388#file910388line30>
> >
> > Ditto here, where is this relevant?
Dropped. Not needed in this file, had copied the old includes over and reworked them to follow proper include order.
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > src/slave/flags.cpp, line 19
> > <https://reviews.apache.org/r/32558/diff/4/?file=910390#file910390line19>
> >
> > Why do you need this?
Same as why it is in "master/flags.hpp".
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > src/slave/flags.cpp, line 21
> > <https://reviews.apache.org/r/32558/diff/4/?file=910390#file910390line21>
> >
> > Why do you need this?
Fixed.
> On April 2, 2015, 7:45 p.m., Ben Mahler wrote:
> > src/slave/flags.cpp, line 25
> > <https://reviews.apache.org/r/32558/diff/4/?file=910390#file910390line25>
> >
> > What's this one needed for?
Fixed.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78703
-----------------------------------------------------------
On April 9, 2015, 6:27 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated April 9, 2015, 6:27 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78703
-----------------------------------------------------------
Looks good Cody, just curious about many of the includes in the .cpp flag files, had a hard time telling how these were relevant.
src/master/flags.hpp
<https://reviews.apache.org/r/32558/#comment127653>
Where is the mesos.hpp include for these? Looks like you removed it?
src/master/flags.cpp
<https://reviews.apache.org/r/32558/#comment127655>
I can't tell how these are relevant to this file, is there something I'm missing?
src/master/flags.cpp
<https://reviews.apache.org/r/32558/#comment127656>
How are these being used in this file?
src/master/flags.cpp
<https://reviews.apache.org/r/32558/#comment127657>
Ditto here, where is this relevant?
src/slave/flags.cpp
<https://reviews.apache.org/r/32558/#comment127659>
Why do you need this?
src/slave/flags.cpp
<https://reviews.apache.org/r/32558/#comment127660>
Why do you need this?
src/slave/flags.cpp
<https://reviews.apache.org/r/32558/#comment127661>
What's this one needed for?
- Ben Mahler
On March 31, 2015, 2:16 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 2:16 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78320
-----------------------------------------------------------
Patch looks great!
Reviews applied: [32558]
All tests passed.
- Mesos ReviewBot
On March 31, 2015, 2:16 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 2:16 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review79558
-----------------------------------------------------------
Patch looks great!
Reviews applied: [32558]
All tests passed.
- Mesos ReviewBot
On April 9, 2015, 6:27 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated April 9, 2015, 6:27 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review79619
-----------------------------------------------------------
Ship it!
Weird, there was quite a lot of funky argument wrapping introduced, not sure if you were using a tool that changed the wrapping? Anyway, I'll clean it up for you and get this committed shortly.
src/logging/flags.cpp
<https://reviews.apache.org/r/32558/#comment129124>
For example, this was changed.
src/logging/flags.cpp
<https://reviews.apache.org/r/32558/#comment129123>
Another example of weird wrapping here with the 0 argument at the end.
- Ben Mahler
On April 9, 2015, 6:27 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated April 9, 2015, 6:27 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/
-----------------------------------------------------------
(Updated April 9, 2015, 6:27 p.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Update includes per Ben Mahler's comments.
Bugs: MESOS-292
https://issues.apache.org/jira/browse/MESOS-292
Repository: mesos
Description
-------
Split the mesos master, slave flags into header + source file to
improve compile time significantly. Should be no functional changes.
Largely copy-paste, with a little reworking to remove unnecessary
headers from the .hpp, and order headers a little more reliably
(as well as remove duplicate includes) in the .cpp.
# Impact of these changes
`time make check -j8`
Before:
make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
After:
make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
Intel i7-4790K.
The numbers aren't incredibly stable (Other software running,
overclocking enabled, etc). They are a good general measure though of
speedup.
I do a `make check` rather than just `make` because that is what devs
do in a day-to-day flow, and if runtime is significantly impacted, it
will show up.
Test steps:
```
# Warm cache
../configure --disable-python --disable-java
make check
# Timed build
rm -rf *
../configure --disable-python --disable-java
time make check
```
Note: Similar, likely greater improvements in compile time would happen
if stout were made to not be header only. Specifically <stout/os.hpp>.
Diffs (updated)
-----
src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8
src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
src/logging/flags.cpp PRE-CREATION
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
src/master/flags.cpp PRE-CREATION
src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
src/slave/flags.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/32558/diff/
Testing
-------
make check on ArchLinux with GCC 4.9.2
make distcheck CentOS 7
Thanks,
Cody Maloney
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/
-----------------------------------------------------------
(Updated March 31, 2015, 2:16 a.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Update include order
Bugs: MESOS-292
https://issues.apache.org/jira/browse/MESOS-292
Repository: mesos
Description
-------
Split the mesos master, slave flags into header + source file to
improve compile time significantly. Should be no functional changes.
Largely copy-paste, with a little reworking to remove unnecessary
headers from the .hpp, and order headers a little more reliably
(as well as remove duplicate includes) in the .cpp.
# Impact of these changes
`time make check -j8`
Before:
make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
After:
make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
Intel i7-4790K.
The numbers aren't incredibly stable (Other software running,
overclocking enabled, etc). They are a good general measure though of
speedup.
I do a `make check` rather than just `make` because that is what devs
do in a day-to-day flow, and if runtime is significantly impacted, it
will show up.
Test steps:
```
# Warm cache
../configure --disable-python --disable-java
make check
# Timed build
rm -rf *
../configure --disable-python --disable-java
time make check
```
Note: Similar, likely greater improvements in compile time would happen
if stout were made to not be header only. Specifically <stout/os.hpp>.
Diffs (updated)
-----
src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
src/logging/flags.cpp PRE-CREATION
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
src/master/flags.cpp PRE-CREATION
src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
src/slave/flags.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/32558/diff/
Testing
-------
make check on ArchLinux with GCC 4.9.2
make distcheck CentOS 7
Thanks,
Cody Maloney
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Joerg Schad <jo...@mesosphere.io>.
> On March 27, 2015, 11:43 a.m., Joerg Schad wrote:
> > LGTM
> > Great effort!
> > On my machine (Apple LLVM version 6.0 (clang-600.0.56)) on the current master this reduced 'make -j8' from
> > real 5m8.217s, user 25m56.168s, sys 2m47.850s
> > to
> > real 4m28.660s, user 23m0.723s, sys 2m36.788s
Only point I would mention is that this points to a resolved Jira which is strange.
And do we have (or could we open) and Jira for the stout/os.hpp splitting? Would be definitely worth starting a discussion about it.
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78031
-----------------------------------------------------------
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Joerg Schad <jo...@mesosphere.io>.
> On March 27, 2015, 11:43 a.m., Joerg Schad wrote:
> > LGTM
> > Great effort!
> > On my machine (Apple LLVM version 6.0 (clang-600.0.56)) on the current master this reduced 'make -j8' from
> > real 5m8.217s, user 25m56.168s, sys 2m47.850s
> > to
> > real 4m28.660s, user 23m0.723s, sys 2m36.788s
>
> Joerg Schad wrote:
> Only point I would mention is that this points to a resolved Jira which is strange.
> And do we have (or could we open) and Jira for the stout/os.hpp splitting? Would be definitely worth starting a discussion about it.
I guess it is rather an issue with the Jira (seems weird to see "Remove unnecessary includes of headers" closed). Shouldn't we generate an Epic Jira for tracking all these efforts?
- Joerg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78031
-----------------------------------------------------------
On March 31, 2015, 2:16 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 2:16 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78031
-----------------------------------------------------------
LGTM
Great effort!
On my machine (Apple LLVM version 6.0 (clang-600.0.56)) on the current master this reduced 'make -j8' from
real 5m8.217s, user 25m56.168s, sys 2m47.850s
to
real 4m28.660s, user 23m0.723s, sys 2m36.788s
- Joerg Schad
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
> On March 31, 2015, 1:36 a.m., Ben Mahler wrote:
> > Great work Cody! For posterity, how did you pinpoint flags.hpp?
Manual experimentation a while ago with Joris. We were searching for general compile time improvements. A conversation recently reminded me of this and that we never pushed a patch for it. Checked again, and saw really we should try to land it.
General process was: Find a couple files that take a long time to compile (Where does the command printing of make sit for a long time), comment out the body (#if 0 #endif), see if it still takes a while. If so, binary search through the headers and figure out which ones are worst. Go into those headers, repeat until you find the root file code.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78303
-----------------------------------------------------------
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78303
-----------------------------------------------------------
Great work Cody! For posterity, how did you pinpoint flags.hpp?
- Ben Mahler
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78228
-----------------------------------------------------------
I know the code-base is currently inconsistent, but let's follow the include style rules for now so as not to make it more-so:
```c++
#include <mesos/module/module.hpp> // Includes from mesos public include directory.
#include "master/master.hpp" // Includes from mesos src directory.
```
If we want to change the overall include style rules, let's make that a seperate proposal / patch.
- Joris Van Remoortere
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Michael Park <mc...@gmail.com>.
> On March 27, 2015, 4:41 p.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 29
> > <https://reviews.apache.org/r/32558/diff/3/?file=907340#file907340line29>
> >
> > How about keeping the same style with namespaces? Just to be consistent with everywhere else in the code base.
>
> Cody Maloney wrote:
> What do you mean by this? Do a bunch of "using namespace declarations"? That will make the code longer, and the translation less direct than it currently is. Also if it is 'using namespace' then a bunch of the explicit namespace prefixes previously in the code should be removed, which makes this a less direct translation. Currently it is just copy/base from header to source file the body of the function, then rework the includes to not include duplicates. Anything more in this move and I think the odds of introducing a bug in what should be mechanical code motion increases significantly.
I think he means:
```cpp
namespace mesos {
namespace internal {
namespace slave {
Flags::Flags() { ... }
} // namespace slave {
} // namespace internal {
} // namespace mesos {
```
As opposed to:
```cpp
mesos::internal::slave::Flags::Flags() { ... }
```
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78071
-----------------------------------------------------------
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
> On March 27, 2015, 4:41 p.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 29
> > <https://reviews.apache.org/r/32558/diff/3/?file=907340#file907340line29>
> >
> > How about keeping the same style with namespaces? Just to be consistent with everywhere else in the code base.
>
> Cody Maloney wrote:
> What do you mean by this? Do a bunch of "using namespace declarations"? That will make the code longer, and the translation less direct than it currently is. Also if it is 'using namespace' then a bunch of the explicit namespace prefixes previously in the code should be removed, which makes this a less direct translation. Currently it is just copy/base from header to source file the body of the function, then rework the includes to not include duplicates. Anything more in this move and I think the odds of introducing a bug in what should be mechanical code motion increases significantly.
>
> Michael Park wrote:
> I think he means:
>
> ```cpp
> namespace mesos {
> namespace internal {
> namespace slave {
>
> Flags::Flags() { ... }
>
> } // namespace slave {
> } // namespace internal {
> } // namespace mesos {
> ```
>
> As opposed to:
>
> ```cpp
> mesos::internal::slave::Flags::Flags() { ... }
> ```
>
> Cody Maloney wrote:
> I can restructure it like that, I think it obscures what is happening and what could possibly be happening if you aren't seeing this diff (There is one very specific function being defined, it is the only function).
>
> Reading through the style guide (Mesos, Google) I don't see any rules around this.
One more thing to think abou: if I switch to stepping into the namespaces like that, all the code I'm copy/pasting which references stuff in the specific namespaces I'm now inside should have those namespaces removed, wihch considerably complicates the code change / move removes the mechanical aspect of it. If I don't change the namespaces with that I've introduced another inconsistency.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78071
-----------------------------------------------------------
On March 31, 2015, 2:16 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 2:16 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
> On March 27, 2015, 4:41 p.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 29
> > <https://reviews.apache.org/r/32558/diff/3/?file=907340#file907340line29>
> >
> > How about keeping the same style with namespaces? Just to be consistent with everywhere else in the code base.
What do you mean by this? Do a bunch of "using namespace declarations"? That will make the code longer, and the translation less direct than it currently is. Also if it is 'using namespace' then a bunch of the explicit namespace prefixes previously in the code should be removed, which makes this a less direct translation. Currently it is just copy/base from header to source file the body of the function, then rework the includes to not include duplicates. Anything more in this move and I think the odds of introducing a bug in what should be mechanical code motion increases significantly.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78071
-----------------------------------------------------------
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
> On March 27, 2015, 4:41 p.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 29
> > <https://reviews.apache.org/r/32558/diff/3/?file=907340#file907340line29>
> >
> > How about keeping the same style with namespaces? Just to be consistent with everywhere else in the code base.
>
> Cody Maloney wrote:
> What do you mean by this? Do a bunch of "using namespace declarations"? That will make the code longer, and the translation less direct than it currently is. Also if it is 'using namespace' then a bunch of the explicit namespace prefixes previously in the code should be removed, which makes this a less direct translation. Currently it is just copy/base from header to source file the body of the function, then rework the includes to not include duplicates. Anything more in this move and I think the odds of introducing a bug in what should be mechanical code motion increases significantly.
>
> Michael Park wrote:
> I think he means:
>
> ```cpp
> namespace mesos {
> namespace internal {
> namespace slave {
>
> Flags::Flags() { ... }
>
> } // namespace slave {
> } // namespace internal {
> } // namespace mesos {
> ```
>
> As opposed to:
>
> ```cpp
> mesos::internal::slave::Flags::Flags() { ... }
> ```
I can restructure it like that, I think it obscures what is happening and what could possibly be happening if you aren't seeing this diff (There is one very specific function being defined, it is the only function).
Reading through the style guide (Mesos, Google) I don't see any rules around this.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78071
-----------------------------------------------------------
On March 31, 2015, 2:16 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 2:16 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Timothy Chen <tn...@apache.org>.
> On March 27, 2015, 4:41 p.m., Timothy Chen wrote:
> > src/slave/flags.cpp, line 29
> > <https://reviews.apache.org/r/32558/diff/3/?file=907340#file907340line29>
> >
> > How about keeping the same style with namespaces? Just to be consistent with everywhere else in the code base.
>
> Cody Maloney wrote:
> What do you mean by this? Do a bunch of "using namespace declarations"? That will make the code longer, and the translation less direct than it currently is. Also if it is 'using namespace' then a bunch of the explicit namespace prefixes previously in the code should be removed, which makes this a less direct translation. Currently it is just copy/base from header to source file the body of the function, then rework the includes to not include duplicates. Anything more in this move and I think the odds of introducing a bug in what should be mechanical code motion increases significantly.
>
> Michael Park wrote:
> I think he means:
>
> ```cpp
> namespace mesos {
> namespace internal {
> namespace slave {
>
> Flags::Flags() { ... }
>
> } // namespace slave {
> } // namespace internal {
> } // namespace mesos {
> ```
>
> As opposed to:
>
> ```cpp
> mesos::internal::slave::Flags::Flags() { ... }
> ```
>
> Cody Maloney wrote:
> I can restructure it like that, I think it obscures what is happening and what could possibly be happening if you aren't seeing this diff (There is one very specific function being defined, it is the only function).
>
> Reading through the style guide (Mesos, Google) I don't see any rules around this.
>
> Cody Maloney wrote:
> One more thing to think abou: if I switch to stepping into the namespaces like that, all the code I'm copy/pasting which references stuff in the specific namespaces I'm now inside should have those namespaces removed, wihch considerably complicates the code change / move removes the mechanical aspect of it. If I don't change the namespaces with that I've introduced another inconsistency.
I think it's fine to leave as is, as I realize we don't usually do this in cpp files anyways. I'll drop this.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78071
-----------------------------------------------------------
On March 31, 2015, 2:16 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 31, 2015, 2:16 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 56ed9d9f047ebc3507c8ba235bba4e1ffcf76cd3
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78071
-----------------------------------------------------------
src/slave/flags.cpp
<https://reviews.apache.org/r/32558/#comment126476>
How about keeping the same style with namespaces? Just to be consistent with everywhere else in the code base.
- Timothy Chen
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review78010
-----------------------------------------------------------
Patch looks great!
Reviews applied: [32558]
All tests passed.
- Mesos ReviewBot
On March 27, 2015, 1:21 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 1:21 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/
-----------------------------------------------------------
(Updated March 27, 2015, 1:21 a.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Add missing include found by `make distcheck`.
Bugs: MESOS-292
https://issues.apache.org/jira/browse/MESOS-292
Repository: mesos
Description
-------
Split the mesos master, slave flags into header + source file to
improve compile time significantly. Should be no functional changes.
Largely copy-paste, with a little reworking to remove unnecessary
headers from the .hpp, and order headers a little more reliably
(as well as remove duplicate includes) in the .cpp.
# Impact of these changes
`time make check -j8`
Before:
make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
After:
make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
Intel i7-4790K.
The numbers aren't incredibly stable (Other software running,
overclocking enabled, etc). They are a good general measure though of
speedup.
I do a `make check` rather than just `make` because that is what devs
do in a day-to-day flow, and if runtime is significantly impacted, it
will show up.
Test steps:
```
# Warm cache
../configure --disable-python --disable-java
make check
# Timed build
rm -rf *
../configure --disable-python --disable-java
time make check
```
Note: Similar, likely greater improvements in compile time would happen
if stout were made to not be header only. Specifically <stout/os.hpp>.
Diffs (updated)
-----
src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
src/logging/flags.cpp PRE-CREATION
src/master/allocator/allocator.hpp b67b8fddbd7a3fffc6fe24d5e77cd1db8cb6f69b
src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
src/master/flags.cpp PRE-CREATION
src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
src/slave/flags.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/32558/diff/
Testing
-------
make check on ArchLinux with GCC 4.9.2
make distcheck CentOS 7
Thanks,
Cody Maloney
Re: Review Request 32558: Improve compile time of mesos by splitting
flags
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32558/#review77994
-----------------------------------------------------------
+1, but I haven't checked style in the resulting `cpp` files.
- Michael Park
On March 27, 2015, 12:37 a.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32558/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 12:37 a.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-292
> https://issues.apache.org/jira/browse/MESOS-292
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Split the mesos master, slave flags into header + source file to
> improve compile time significantly. Should be no functional changes.
>
> Largely copy-paste, with a little reworking to remove unnecessary
> headers from the .hpp, and order headers a little more reliably
> (as well as remove duplicate includes) in the .cpp.
>
> # Impact of these changes
> `time make check -j8`
> Before:
> make check 2732.93s user 103.89s system 514% cpu 9:11.63 total
>
> After:
> make check 2421.18s user 96.60s system 506% cpu 8:16.67 total
>
> 4 core machine, 8 hypter threads enabled. SSD, 16 GB RAM,
> Intel i7-4790K.
>
> The numbers aren't incredibly stable (Other software running,
> overclocking enabled, etc). They are a good general measure though of
> speedup.
>
> I do a `make check` rather than just `make` because that is what devs
> do in a day-to-day flow, and if runtime is significantly impacted, it
> will show up.
>
> Test steps:
> ```
> # Warm cache
> ../configure --disable-python --disable-java
> make check
> # Timed build
> rm -rf *
> ../configure --disable-python --disable-java
> time make check
> ```
>
> Note: Similar, likely greater improvements in compile time would happen
> if stout were made to not be header only. Specifically <stout/os.hpp>.
>
>
> Diffs
> -----
>
> src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb
> src/logging/flags.hpp 4facb33201cee5a82451a13ca05607c875574752
> src/logging/flags.cpp PRE-CREATION
> src/master/flags.hpp 85ce3285731c11b464aca6eaaa0a9165c73afebd
> src/master/flags.cpp PRE-CREATION
> src/slave/flags.hpp 3da71afad38ae41adefab979dbed2ae0b10a98ef
> src/slave/flags.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/32558/diff/
>
>
> Testing
> -------
>
> make check on ArchLinux with GCC 4.9.2
> make distcheck CentOS 7
>
>
> Thanks,
>
> Cody Maloney
>
>