mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 44260: Moved metrics of the hierarchical allocator to its own file.
Date Thu, 03 Mar 2016 16:25:37 GMT


> On March 2, 2016, 2:58 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/metrics.hpp, line 37
> > <https://reviews.apache.org/r/44260/diff/1/?file=1276467#file1276467line37>
> >
> >     You don't really need to call `.self()` here, there exists an `defer` override
taking process instance.
> 
> Benjamin Bannier wrote:
>     At least my clang does not trigger that overload.
> 
> Alexander Rukletsov wrote:
>     https://github.com/apache/mesos/blob/9bbba94021dde42c9d9d1fa0662462c364797018/3rdparty/libprocess/include/process/defer.hpp#L177
> 
> Benjamin Bannier wrote:
>     The issue I see is below when directly using the `Process` here. Note that this works
for code in `HierarchicalAllocatorProcess`.
>     
>     || In file included from ../../src/master/allocator/mesos/metrics.cpp:17:
>     || In file included from ../../src/master/allocator/mesos/metrics.hpp:26:
>     || In file included from ../../3rdparty/libprocess/include/process/metrics/gauge.hpp:19:
>     || In file included from ../../3rdparty/libprocess/include/process/defer.hpp:19:
>     || ../../3rdparty/libprocess/include/process/deferred.hpp:110:14: error: no matching
conversion for functional-style cast from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::function<Future<double> ()>'
>     ||       return std::function<R()>(f);
>     ||              ^~~~~~~~~~~~~~~~~~~~
>     || ../../src/master/allocator/mesos/metrics.cpp:38:9: note: in instantiation of function
template specialization 'process::_Deferred<double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*)()>::operator
Deferred<process::Future<double> >' requested here
>     ||         process::defer(
>     ||         ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1593:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'nullptr_t' for 1st argument
>     ||     function(nullptr_t) _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1594:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'const std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(const function&);
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1595:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(function&&) _NOEXCEPT;
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1599:41: note: candidate template
ignored: disabled by 'enable_if' [with _Fp = double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*)()]
>     ||                                         __callable<_Fp>::value &&
>     ||                                         ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1605:7: note: candidate constructor
template not viable: requires 2 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&) _NOEXCEPT : __f_(0) {}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1608:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, nullptr_t) _NOEXCEPT : __f_(0)
{}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1610:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, const function&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1612:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, function&&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1614:7: note: candidate constructor
template not viable: requires at least 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc& __a, _Fp __f,
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1591:5: note: candidate constructor
not viable: requires 0 arguments, but 1 was provided
>     ||     function() _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || In file included from ../../src/master/allocator/mesos/metrics.cpp:17:
>     || In file included from ../../src/master/allocator/mesos/metrics.hpp:26:
>     || In file included from ../../3rdparty/libprocess/include/process/metrics/gauge.hpp:19:
>     || In file included from ../../3rdparty/libprocess/include/process/defer.hpp:19:
>     || ../../3rdparty/libprocess/include/process/deferred.hpp:118:39: error: no matching
conversion for functional-style cast from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::function<Future<double> ()>'
>     ||           return dispatch(pid_.get(), std::function<R()>(f_));
>     ||                                       ^~~~~~~~~~~~~~~~~~~~~
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1593:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'nullptr_t' for 1st argument
>     ||     function(nullptr_t) _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1594:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'const std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(const function&);
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1595:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(function&&) _NOEXCEPT;
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1599:41: note: candidate template
ignored: disabled by 'enable_if' [with _Fp = double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*)()]
>     ||                                         __callable<_Fp>::value &&
>     ||                                         ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1605:7: note: candidate constructor
template not viable: requires 2 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&) _NOEXCEPT : __f_(0) {}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1608:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, nullptr_t) _NOEXCEPT : __f_(0)
{}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1610:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, const function&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1612:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, function&&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1614:7: note: candidate constructor
template not viable: requires at least 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc& __a, _Fp __f,
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1591:5: note: candidate constructor
not viable: requires 0 arguments, but 1 was provided
>     ||     function() _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || 1 warning and 2 errors generated.
>     || Error while processing /XYZ/src/mesos/src/master/allocator/mesos/metrics.cpp.
>     || warning: 0.28.0": 'linker' input unused
>     || In file included from <built-in>:356:
>     || <command line>:4:24: warning: missing terminating '"' character [-Winvalid-pp-token]
>     || #define PACKAGE_STRING "mesos
>     ||                        ^
>     || In file included from ../../src/master/allocator/mesos/metrics.cpp:17:
>     || In file included from ../../src/master/allocator/mesos/metrics.hpp:26:
>     || In file included from ../../3rdparty/libprocess/include/process/metrics/gauge.hpp:19:
>     || In file included from ../../3rdparty/libprocess/include/process/defer.hpp:19:
>     || ../../3rdparty/libprocess/include/process/deferred.hpp:110:14: error: no matching
conversion for functional-style cast from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::function<Future<double> ()>'
>     ||       return std::function<R()>(f);
>     ||              ^~~~~~~~~~~~~~~~~~~~
>     || ../../src/master/allocator/mesos/metrics.cpp:38:9: note: in instantiation of function
template specialization 'process::_Deferred<double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*)()>::operator
Deferred<process::Future<double> >' requested here
>     ||         process::defer(
>     ||         ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1593:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'nullptr_t' for 1st argument
>     ||     function(nullptr_t) _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1594:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'const std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(const function&);
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1595:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(function&&) _NOEXCEPT;
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1599:41: note: candidate template
ignored: disabled by 'enable_if' [with _Fp = double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*)()]
>     ||                                         __callable<_Fp>::value &&
>     ||                                         ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1605:7: note: candidate constructor
template not viable: requires 2 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&) _NOEXCEPT : __f_(0) {}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1608:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, nullptr_t) _NOEXCEPT : __f_(0)
{}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1610:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, const function&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1612:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, function&&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1614:7: note: candidate constructor
template not viable: requires at least 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc& __a, _Fp __f,
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1591:5: note: candidate constructor
not viable: requires 0 arguments, but 1 was provided
>     ||     function() _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || In file included from ../../src/master/allocator/mesos/metrics.cpp:17:
>     || In file included from ../../src/master/allocator/mesos/metrics.hpp:26:
>     || In file included from ../../3rdparty/libprocess/include/process/metrics/gauge.hpp:19:
>     || In file included from ../../3rdparty/libprocess/include/process/defer.hpp:19:
>     || ../../3rdparty/libprocess/include/process/deferred.hpp:118:39: error: no matching
conversion for functional-style cast from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::function<Future<double> ()>'
>     ||           return dispatch(pid_.get(), std::function<R()>(f_));
>     ||                                       ^~~~~~~~~~~~~~~~~~~~~
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1593:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'nullptr_t' for 1st argument
>     ||     function(nullptr_t) _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1594:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'const std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(const function&);
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1595:5: note: candidate constructor
not viable: no known conversion from 'double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*const)()'
to 'std::__1::function<process::Future<double> ()>' for 1st argument
>     ||     function(function&&) _NOEXCEPT;
>     ||     ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1599:41: note: candidate template
ignored: disabled by 'enable_if' [with _Fp = double (mesos::internal::master::allocator::internal::HierarchicalAllocatorProcess::*)()]
>     ||                                         __callable<_Fp>::value &&
>     ||                                         ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1605:7: note: candidate constructor
template not viable: requires 2 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&) _NOEXCEPT : __f_(0) {}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1608:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, nullptr_t) _NOEXCEPT : __f_(0)
{}
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1610:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, const function&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1612:7: note: candidate constructor
template not viable: requires 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc&, function&&);
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1614:7: note: candidate constructor
template not viable: requires at least 3 arguments, but 1 was provided
>     ||       function(allocator_arg_t, const _Alloc& __a, _Fp __f,
>     ||       ^
>     || /XYZ/src/llvm/P/bin/../include/c++/v1/functional:1591:5: note: candidate constructor
not viable: requires 0 arguments, but 1 was provided
>     ||     function() _NOEXCEPT : __f_(0) {}
>     ||     ^
>     || 1 warning and 2 errors generated.
>     || Error while processing /XYZ/src/mesos/src/master/allocator/mesos/metrics.cpp.

Somehow this doesn't seem to work with `HierarchicalAllocatorProcess` the way it does with
e.g., `Master`. Leaving in the self for now, but definitely worth investigating.

Here's a reproducer:

class P : public mesos::internal::master::allocator::MesosAllocatorProcess
{
public:
  double hi() const { return 0; }
  double ho() { return 0; }
};

class M : public ProtobufProcess<M>
{
public:
  double hi() { return 0; }
};

void f(const P& p)
{
  process::metrics::Gauge("defer via UPID and free function",
                          process::defer(p, lambda::bind(&P::hi, &p)));

  // THIS DOES NOT WORK
  // process::metrics::Gauge("defer via Process and member fct ptr",
  //                         process::defer(p, P::hi)));
}

void g(const M& m) {
  // WORKS AS EXPECTED
  process::metrics::Gauge("defer via Process and member fct ptr",
                          process::defer(m, &M::hi));
}


- Benjamin


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


On March 3, 2016, 5:17 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44260/
> -----------------------------------------------------------
> 
> (Updated March 3, 2016, 5:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4718
>     https://issues.apache.org/jira/browse/MESOS-4718
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved metrics of the hierarchical allocator to its own file.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 0eabfad66f43adff12d08dbf34ca3db1b801c49e 
>   src/Makefile.am b30cc25f61856d6417437547baaa0bb338a30d63 
>   src/master/allocator/mesos/hierarchical.hpp 3043888630b066505410d3b32c5b3f813cc458c1

>   src/master/allocator/mesos/metrics.hpp PRE-CREATION 
>   src/master/allocator/mesos/metrics.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44260/diff/
> 
> 
> Testing
> -------
> 
> `make distcheck` on OS X.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message