Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38869]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十月 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated 十月 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Michael Park]

> On Oct. 13, 2015, 5:20 p.m., Michael Park wrote:
> > All nits.
> > 
> > ```
> > make check -j 4 GTEST_FILTER=""  1984.45s user 77.00s system 367% cpu 
> > 9:20.95 total
> > make check -j 4 GTEST_FILTER=""  30.08s user 8.57s system 90% cpu 42.577 
> > total
> > ```

Oops. The `make check` numbers represent the compilation times for when a 
change is made in the implementation.

(1) Before: Added a comment in the function body in `hierarchical.hpp`.
(2) After: Added a comment in the function body in `hierarchical.cpp`.


- Michael


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


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Michael Park]

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

Ship it!


All nits.

```
make check -j 4 GTEST_FILTER=""  1984.45s user 77.00s system 367% cpu 9:20.95 
total
make check -j 4 GTEST_FILTER=""  30.08s user 8.57s system 90% cpu 42.577 total
```


src/master/allocator/mesos/hierarchical.hpp (line 80)


`s/internal:://`



src/master/allocator/mesos/hierarchical.hpp (line 99)


`s/internal:://`



src/master/allocator/mesos/hierarchical.hpp (line 389)


`s/internal:://`



src/master/allocator/mesos/hierarchical.cpp (line 53)


Not yours, but `s/Filter:/Filter :/`



src/master/allocator/mesos/hierarchical.cpp (line 96)


Not yours, but `s/Filter:/Filter :/`



src/master/allocator/mesos/hierarchical.cpp (lines 112 - 113)


This now fits in one line. Here and below.


- Michael Park


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Ben Mahler]

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


Thanks for doing this! One minor issue about the use of the factory stuff below.


src/master/allocator/mesos/hierarchical.hpp (lines 65 - 91)


Do we need all of this factory stuff here? Any reason we can't just pass in 
std::functions as the factories instead of having the polymorphism indirection 
here?


- Ben Mahler


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Guangya Liu]

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


Does it make sense to make the default allocator as a module?

- Guangya Liu


On 九月 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated 九月 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Joris Van Remoortere]

> On Sept. 30, 2015, 9:55 a.m., Alexander Rukletsov wrote:
> > I like when the code is pushed to `.cpp` files : ). Just to confirm that 
> > this is our intention: with the prvious design, `RoleSorter` and 
> > `FrameworkSorter` could be anything (well, anything that can be used as a 
> > sorter), now we restrict sorters to subclass `Sorter`. I think this is 
> > fine, but would like to confirm with you.
> > 
> > A high level question I have: why do you like to preserve template 
> > approach? I think we can require allocator writers to provide a factory 
> > with their own allocator and get rid of templates completely. What do you 
> > think?

Indeed, the old implementation was just enforcing the same interface as 
`Sorter` but through templates.
I kept the template approach because it minimizes the size and impact of this 
patch. I think we can make a separate discussion around removing the template 
all together.
All the tests, and possibly code that people have written outside of our repo 
will continue to work using the current approach as the external interface does 
not change.
I realize that people shouldn't be relying on these internal files, but I think 
we can isolate this change to just a compile time optimization, leaving out the 
API change.
What are your thoughts?


> On Sept. 30, 2015, 9:55 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.hpp, lines 110-114
> > 
> >
> > Why have you decided to leave the c-tor here and not in the cpp file?

The diff generated by this is rather large, and I wanted to make it very 
obvious which parts of the code are changing (The top of the diff), and make it 
as easy as possible for someone to verify that nothing else changed (i.e. the 
rest of the file was just copy pasted, with template prototypes removed).


> On Sept. 30, 2015, 9:55 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 131-136
> > 
> >
> > How about pulling this code to the c-tor? this will allow us not to 
> > store a copy of `sorterFactory` in allocator.

We can. I didn't want to break any assumptions between constructor vs 
initializer ordering of these events.
Would it make sense to make this a separate patch as it changes the behavior?


- Joris


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


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Joris Van Remoortere]

> On Sept. 30, 2015, 5:10 p.m., Joseph Wu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 19
> > 
> >
> > Shouldn't this go at the bottom of the includes?

I believe we are still trying to follow the guideline of including the header 
that we're implementing as the first file in the include list.
I recognize this is not consistent in the code-base. Joerg is trying to fix 
that.


- Joris


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


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Ben Mahler]

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


Hm.. it is suprising that touching this header requires wide re-compilation. 
Could we audit why that is?

Looking through the includes, I can see tests/cluster.hpp pulls this in, but we 
should have a .cpp to avoid the dependency in the header. This should yield 
more dependency removals since we can cut other includes. The other culprit 
looks to be tests/mesos.hpp which exposes an allocator wrapper for mocking. 
We've been planning to split out the mocks into relevant mocks/*.hpp files, so 
this could be migrated to a mocks/allocator.hpp file that is only included when 
needed in a test. FWICT only 5 test files need this.

Doing the dependency cleanup should reduce the need for this refactor, and help 
speed up compilation more generally. I'm curious about the implications of this 
pattern for our other templated headers, so hoping to be able to avoid if 
possible :)

- Ben Mahler


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Alexander Rukletsov]

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


I like when the code is pushed to `.cpp` files : ). Just to confirm that this 
is our intention: with the prvious design, `RoleSorter` and `FrameworkSorter` 
could be anything (well, anything that can be used as a sorter), now we 
restrict sorters to subclass `Sorter`. I think this is fine, but would like to 
confirm with you.

A high level question I have: why do you like to preserve template approach? I 
think we can require allocator writers to provide a factory with their own 
allocator and get rid of templates completely. What do you think?


src/master/allocator/mesos/hierarchical.hpp 


We should leave at least
```
#include 
#include 
```
here.



src/master/allocator/mesos/hierarchical.hpp (line 22)


I think `` should be here. Alternatively, 
`HierarchicalAllocatorProcess`'s c-tor can be moved into `.cpp`.

Also, though `` pulls ``, I think it 
should be included explicitly.



src/master/allocator/mesos/hierarchical.hpp 


`` should stay here.



src/master/allocator/mesos/hierarchical.hpp (line 28)


I think `` should be included here.



src/master/allocator/mesos/hierarchical.hpp (line 62)


virtual d-tor?



src/master/allocator/mesos/hierarchical.hpp (lines 91 - 95)


Why have you decided to leave the c-tor here and not in the cpp file?



src/master/allocator/mesos/hierarchical.cpp (line 33)


remove this newline.



src/master/allocator/mesos/hierarchical.cpp (lines 131 - 136)


How about pulling this code to the c-tor? this will allow us not to store a 
copy of `sorterFactory` in allocator.


- Alexander Rukletsov


On Sept. 30, 2015, 1:08 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 38869: Added static->dynamic transformation to Allocator.

[Joris Van Remoortere]

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

Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
Joseph Wu.


Bugs: MESOS-3554
https://issues.apache.org/jira/browse/MESOS-3554


Repository: mesos


Description
---

This improves the compilation time of Mesos significantly, allowing
developers to iterate more quickly on allocator changes.


Diffs
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/master/allocator/mesos/hierarchical.hpp 
f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
  src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 

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


Testing
---

make check
touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
and relink.


Thanks,

Joris Van Remoortere

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Joris Van Remoortere]

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

(Updated Sept. 30, 2015, 1:08 a.m.)


Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
Joseph Wu.


Changes
---

moved includes around.


Bugs: MESOS-3554
https://issues.apache.org/jira/browse/MESOS-3554


Repository: mesos


Description
---

This improves the compilation time of Mesos significantly, allowing
developers to iterate more quickly on allocator changes.


Diffs (updated)
-

  src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
  src/master/allocator/mesos/hierarchical.hpp 
f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
  src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 

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


Testing
---

make check
touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
and relink.


Thanks,

Joris Van Remoortere

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Alex Clemmer]

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



src/Makefile.am (line 489)


Note that if review #38827 goes in before this, it would be great to add 
this line to the CMakeLists file in src/


- Alex Clemmer


On Sept. 30, 2015, 12:09 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 12:09 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38869]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 12:09 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Sept. 30, 2015, 12:09 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/master/allocator/mesos/hierarchical.hpp 
> f3a9b9d799695c11caad8ae64e1a53e08bb6e63d 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>