You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2014/11/14 21:13:07 UTC
Review Request 28052: Match future dispatch messages with type info.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/include/Makefile.am 542ae1c39283087da6fef69be29c811ac35f00f4
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/include/process/type.hpp PRE-CREATION
3rdparty/libprocess/src/process.cpp 5842705ae78ebe36c1c01bfcd468e9ebe55de3be
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Cody Maloney <co...@mesosphere.io>.
> On Nov. 15, 2014, 4:15 a.m., Michael Park wrote:
> > Suggestion: We can simply return `const std::type_info*` rather than `std::string` for `canonicalize`. The implementation would stay pretty much the same, and we wouldn't have to construct these `MatchesTypeFunc` instances.
> >
> > It looks to me the whole `MatchesTypeFunc` approach was used to try to defer the evaluation of `typeid`. But `typeid` is guaranteed to be resolved at compile-time unless it's invoked on a dynamic type. In this case we're only calling it on member function pointers, which means it'll be resolved at compile-time and therefore won't ever incur any RTTI costs.
>
> Timothy Chen wrote:
> I see, but the type_info that is returned from typeid(method) is a const value reference, which type_info has no copy or default constructor. Is it always gurantee to be a static object, and I can take the address of what typeid returns to me even when it goes out of scope ?
On Linux typeid() should always returns the same object for the same parameters (Even when crossing .so boundaries). When it doesn't there are other more general issues.
The C++ specification mandates that type_info objects returned by typeid() have 'static' duration / are always alive/valid anywhere int he program, so pointer and back is just fine.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review61624
-----------------------------------------------------------
On Nov. 15, 2014, 1:53 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2014, 1:53 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/Makefile.am 542ae1c39283087da6fef69be29c811ac35f00f4
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/include/process/type.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 5842705ae78ebe36c1c01bfcd468e9ebe55de3be
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Michael Park <mc...@gmail.com>.
> On Nov. 15, 2014, 4:15 a.m., Michael Park wrote:
> > Suggestion: We can simply return `const std::type_info*` rather than `std::string` for `canonicalize`. The implementation would stay pretty much the same, and we wouldn't have to construct these `MatchesTypeFunc` instances.
> >
> > It looks to me the whole `MatchesTypeFunc` approach was used to try to defer the evaluation of `typeid`. But `typeid` is guaranteed to be resolved at compile-time unless it's invoked on a dynamic type. In this case we're only calling it on member function pointers, which means it'll be resolved at compile-time and therefore won't ever incur any RTTI costs.
>
> Timothy Chen wrote:
> I see, but the type_info that is returned from typeid(method) is a const value reference, which type_info has no copy or default constructor. Is it always gurantee to be a static object, and I can take the address of what typeid returns to me even when it goes out of scope ?
>
> Cody Maloney wrote:
> On Linux typeid() should always returns the same object for the same parameters (Even when crossing .so boundaries). When it doesn't there are other more general issues.
>
> The C++ specification mandates that type_info objects returned by typeid() have 'static' duration / are always alive/valid anywhere int he program, so pointer and back is just fine.
Yeah, what Cody said. The const-ref to `std::type_info` that is returned by `typeid` is required to have `static` duration. So you can simply return `&typeid(method)` from `canonicalize`.
For comparison, I would suggest using `std::type_info::operator==` rather than comparing the pointers since it's not required that `typeid` return the reference to the same object for the same type.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review61624
-----------------------------------------------------------
On Nov. 15, 2014, 1:53 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2014, 1:53 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/Makefile.am 542ae1c39283087da6fef69be29c811ac35f00f4
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/include/process/type.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 5842705ae78ebe36c1c01bfcd468e9ebe55de3be
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
> On Nov. 15, 2014, 4:15 a.m., Michael Park wrote:
> > Suggestion: We can simply return `const std::type_info*` rather than `std::string` for `canonicalize`. The implementation would stay pretty much the same, and we wouldn't have to construct these `MatchesTypeFunc` instances.
> >
> > It looks to me the whole `MatchesTypeFunc` approach was used to try to defer the evaluation of `typeid`. But `typeid` is guaranteed to be resolved at compile-time unless it's invoked on a dynamic type. In this case we're only calling it on member function pointers, which means it'll be resolved at compile-time and therefore won't ever incur any RTTI costs.
I see, but the type_info that is returned from typeid(method) is a const value reference, which type_info has no copy or default constructor. Is it always gurantee to be a static object, and I can take the address of what typeid returns to me even when it goes out of scope ?
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review61624
-----------------------------------------------------------
On Nov. 15, 2014, 1:53 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2014, 1:53 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/Makefile.am 542ae1c39283087da6fef69be29c811ac35f00f4
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/include/process/type.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 5842705ae78ebe36c1c01bfcd468e9ebe55de3be
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review61624
-----------------------------------------------------------
Suggestion: We can simply return `const std::type_info*` rather than `std::string` for `canonicalize`. The implementation would stay pretty much the same, and we wouldn't have to construct these `MatchesTypeFunc` instances.
It looks to me the whole `MatchesTypeFunc` approach was used to try to defer the evaluation of `typeid`. But `typeid` is guaranteed to be resolved at compile-time unless it's invoked on a dynamic type. In this case we're only calling it on member function pointers, which means it'll be resolved at compile-time and therefore won't ever incur any RTTI costs.
- Michael Park
On Nov. 15, 2014, 1:53 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2014, 1:53 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/Makefile.am 542ae1c39283087da6fef69be29c811ac35f00f4
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/include/process/type.hpp PRE-CREATION
> 3rdparty/libprocess/src/process.cpp 5842705ae78ebe36c1c01bfcd468e9ebe55de3be
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review62392
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment104444>
s/dispatchs/dispatches/
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment104445>
s/type_info/std::type_info/
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment104446>
s/libprocess/Process/
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment104447>
Why did you add this newline?
- Benjamin Hindman
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 1, 2014, 10:41 p.m., Cody Maloney wrote:
> > 3rdparty/libprocess/include/process/gmock.hpp, lines 346-347
> > <https://reviews.apache.org/r/28052/diff/4/?file=771005#file771005line346>
> >
> > The type_info::name() is (on linux, x86_64 using the standard ABI) defined to be the symbol name.
> >
> > See: https://mentorembedded.github.io/cxx-abi/abi.html#typeid
> >
> > Other platforms don't have the same ABI spec though.
"The name() member function returns the address of an NTBS, unique to the type, containing the mangled name of the type. The mangled name of the NTBS is also defined by the ABI to allow consistent reference to it, and the Vague Linkage section specifies how to produce a unique copy."
"manged name of the type" to me is the name of the type, or am I missing something?
For example, ```int* p = NULL'```, typeid(p).name() should be ```"int *"```, not ```p``` I presume.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63451
-----------------------------------------------------------
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63451
-----------------------------------------------------------
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment105706>
The type_info::name() is (on linux, x86_64 using the standard ABI) defined to be the symbol name.
See: https://mentorembedded.github.io/cxx-abi/abi.html#typeid
Other platforms don't have the same ABI spec though.
- Cody Maloney
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 1, 2014, 8:13 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/gmock.hpp, lines 346-347
> > <https://reviews.apache.org/r/28052/diff/4/?file=771005#file771005line346>
> >
> > We only checks `typeid` of a method right now. What if there are two methods having the same type but different name? That will cause `FUTURE_DISPATCH` or `DROP_DISPATCH` to capture/drop a method that is not expected.
Do you have an example that could happen? From what I see in the type_info information the actual symbol name is used for matching, which contains the method name too.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63427
-----------------------------------------------------------
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 1, 2014, 8:13 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/gmock.hpp, lines 346-347
> > <https://reviews.apache.org/r/28052/diff/4/?file=771005#file771005line346>
> >
> > We only checks `typeid` of a method right now. What if there are two methods having the same type but different name? That will cause `FUTURE_DISPATCH` or `DROP_DISPATCH` to capture/drop a method that is not expected.
>
> Timothy Chen wrote:
> Do you have an example that could happen? From what I see in the type_info information the actual symbol name is used for matching, which contains the method name too.
>
> Jie Yu wrote:
> Yeah, the review bot tests failed due to this issue https://reviews.apache.org/r/28570/
>
> Are you refering to type_info::name()? As far as I know, this is the type name not the symbol name:
> http://www.cplusplus.com/reference/typeinfo/type_info/name/
Hi Jie, it looks like it's failing on a EXPECT_CALL macro which is implemented directly by gmock.
My recent changes only touches FUTURE_DISPATCH and DROP_DISPATCH calls only, as we do the matching ourselves.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63427
-----------------------------------------------------------
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 1, 2014, 8:13 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/gmock.hpp, lines 346-347
> > <https://reviews.apache.org/r/28052/diff/4/?file=771005#file771005line346>
> >
> > We only checks `typeid` of a method right now. What if there are two methods having the same type but different name? That will cause `FUTURE_DISPATCH` or `DROP_DISPATCH` to capture/drop a method that is not expected.
>
> Timothy Chen wrote:
> Do you have an example that could happen? From what I see in the type_info information the actual symbol name is used for matching, which contains the method name too.
>
> Jie Yu wrote:
> Yeah, the review bot tests failed due to this issue https://reviews.apache.org/r/28570/
>
> Are you refering to type_info::name()? As far as I know, this is the type name not the symbol name:
> http://www.cplusplus.com/reference/typeinfo/type_info/name/
>
> Timothy Chen wrote:
> Hi Jie, it looks like it's failing on a EXPECT_CALL macro which is implemented directly by gmock.
> My recent changes only touches FUTURE_DISPATCH and DROP_DISPATCH calls only, as we do the matching ourselves.
Tim, that's because the DROP_DISPATCH() dropped a message that is not supposed to be dropped (they have the same signature but different name).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63427
-----------------------------------------------------------
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 1, 2014, 8:13 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/include/process/gmock.hpp, lines 346-347
> > <https://reviews.apache.org/r/28052/diff/4/?file=771005#file771005line346>
> >
> > We only checks `typeid` of a method right now. What if there are two methods having the same type but different name? That will cause `FUTURE_DISPATCH` or `DROP_DISPATCH` to capture/drop a method that is not expected.
>
> Timothy Chen wrote:
> Do you have an example that could happen? From what I see in the type_info information the actual symbol name is used for matching, which contains the method name too.
Yeah, the review bot tests failed due to this issue https://reviews.apache.org/r/28570/
Are you refering to type_info::name()? As far as I know, this is the type name not the symbol name:
http://www.cplusplus.com/reference/typeinfo/type_info/name/
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63427
-----------------------------------------------------------
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63427
-----------------------------------------------------------
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment105679>
We only checks `typeid` of a method right now. What if there are two methods having the same type but different name? That will cause `FUTURE_DISPATCH` or `DROP_DISPATCH` to capture/drop a method that is not expected.
- Jie Yu
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review63739
-----------------------------------------------------------
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment106033>
Jie, you're totally right. `typeid` on its own would only check the types of the functions. I think we should check both the function pointer (as before) as well as the `typeid`.
For non-virtual functions, the function pointer is sufficient to uniquely identify them.
For virtual functions, the member function pointer value is the offset in the vtable (and an adjustment for multiple inhertance but we'll punt on this for now). To uniquely identify virtual functions we need to compare the class that the function is a part of and the offset within the vtable. The `typeid` check gets us the former and the previous function pointer check gets us the latter.
- Michael Park
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/#review62439
-----------------------------------------------------------
3rdparty/libprocess/include/process/gmock.hpp
<https://reviews.apache.org/r/28052/#comment104482>
I was thinking since the comment was for the below two methods, I create one more line so it's more clearer they belong in the same section. I can remove this.
- Timothy Chen
On Nov. 20, 2014, 9 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28052/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2014, 9 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-2112
> https://issues.apache.org/jira/browse/MESOS-2112
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
>
>
> The downside of this approach is that, we no longer can do the following in tests:
>
> class A : ProcessBase<A> {
> public:
> virtual void foo() {};
> };
>
> class B : public A {
> public:
> virtual void foo() {};
> };
>
> Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
>
> process.dispatch(B.self(), &A::foo);
>
> AWAIT_READY(future);
>
> We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
>
> Review: https://reviews.apache.org/r/28052
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
> 3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
> 3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
> 3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
> 3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
>
> Diff: https://reviews.apache.org/r/28052/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
(Updated Nov. 20, 2014, 9 a.m.)
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description (updated)
-------
The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
The downside of this approach is that, we no longer can do the following in tests:
class A : ProcessBase<A> {
public:
virtual void foo() {};
};
class B : public A {
public:
virtual void foo() {};
};
Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
process.dispatch(B.self(), &A::foo);
AWAIT_READY(future);
We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
Review: https://reviews.apache.org/r/28052
Diffs
-----
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
(Updated Nov. 20, 2014, 8:59 a.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Added comment around future dispatch.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description (updated)
-------
Review: https://reviews.apache.org/r/28052
Diffs (updated)
-----
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/src/process.cpp 7a986d7defb03043ec70a48e161ede50deef9b26
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
(Updated Nov. 18, 2014, 6:35 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description (updated)
-------
Review: https://reviews.apache.org/r/28052
The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
The downside of this approach is that, we no longer can do the following in tests:
class A : ProcessBase<A> {
public:
virtual void foo() {};
};
class B : public A {
public:
virtual void foo() {};
};
Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
process.dispatch(B.self(), &A::foo);
AWAIT_READY(future);
---------------------------------------------------
We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
Diffs
-----
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/src/process.cpp 404a32dae1b3d22cb55dcbd084623d6b0cb82c71
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
(Updated Nov. 18, 2014, 1:41 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description (updated)
-------
Review: https://reviews.apache.org/r/28052
The fix in this rb is to instead of matching pointer values, but to compare type_info from the member function pointers passed in. This ensures we're matching the same method.
The downside of this approach is that, we no longer can do the following in tests:
class A : ProcessBase<A> {
public:
virtual void foo() {};
};
class B : public A {
public:
virtual void foo() {};
};
Future<Nothing> future = FUTURE_DISPATCH(_, &B::foo);
dispatch(B.self(), &A::foo);
AWAIT_READY(future);
---------------------------------------------------
We no longer can match the future on a derived method, but on dispatch passing the base member function pointer. This only used to work since the virtual offsets of both is matching the same address, but arguable is not the right behavior since the reverse is a false positive but will also match. We've done this in a few places in tests around the AllocatorProcess.
Diffs
-----
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/src/process.cpp 404a32dae1b3d22cb55dcbd084623d6b0cb82c71
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
(Updated Nov. 18, 2014, 1:19 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description
-------
Review: https://reviews.apache.org/r/28052
Diffs
-----
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/src/process.cpp 404a32dae1b3d22cb55dcbd084623d6b0cb82c71
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
(Updated Nov. 17, 2014, 6:02 p.m.)
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description
-------
Review: https://reviews.apache.org/r/28052
Diffs (updated)
-----
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/src/process.cpp 404a32dae1b3d22cb55dcbd084623d6b0cb82c71
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 28052: Match future dispatch messages with type
info.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28052/
-----------------------------------------------------------
(Updated Nov. 15, 2014, 1:53 a.m.)
Review request for mesos and Benjamin Hindman.
Bugs: MESOS-2112
https://issues.apache.org/jira/browse/MESOS-2112
Repository: mesos-git
Description (updated)
-------
Review: https://reviews.apache.org/r/28052
Diffs (updated)
-----
3rdparty/libprocess/include/Makefile.am 542ae1c39283087da6fef69be29c811ac35f00f4
3rdparty/libprocess/include/process/c++11/dispatch.hpp 76da2828cf36b6143d627dac66f3e0cc4416bae4
3rdparty/libprocess/include/process/dispatch.hpp bceda2a2ae8963921e8e19660243a8644feab227
3rdparty/libprocess/include/process/event.hpp 294e2158876b25b06456a3619e547082d71e31ed
3rdparty/libprocess/include/process/gmock.hpp d6f2fc8fef8acd1498a36c86e09b28d986eefa15
3rdparty/libprocess/include/process/type.hpp PRE-CREATION
3rdparty/libprocess/src/process.cpp 5842705ae78ebe36c1c01bfcd468e9ebe55de3be
Diff: https://reviews.apache.org/r/28052/diff/
Testing
-------
make check
Thanks,
Timothy Chen