[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16279279#comment-16279279
]
Michael Park commented on MESOS-6972:
-
{noformat}
commit bbd8381ebce3522841e80ae53f56b3049342f15b
Author: Dmitry Zhuk
Date: Tue Dec 5 13:47:53 2017 -0800
Replaced `std::shared_ptr` with `std::unique_ptr` in `Future`.
Review: https://reviews.apache.org/r/63913/
{noformat}
{noformat}
commit bca8c6a05d03a2162c04703a9c1ac8172fdfae8a
Author: Dmitry Zhuk
Date: Tue Dec 5 13:45:56 2017 -0800
Replaced `std::shared_ptr` with `std::unique_ptr` in `dispatch`.
Since `dispatch` can now handle move-only parameters, `Promise` and
function object can be wrapped into `std::unique_ptr` for efficiency.
{noformat}
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: performance, tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16279122#comment-16279122
]
Michael Park commented on MESOS-6972:
-
{noformat}
commit 6839897c5464fce6b8cbd253d959a7e2efd72987 (HEAD -> master,
upstream/master)
Author: Dmitry Zhuk
Date: Tue Dec 5 11:21:27 2017 -0800
Made `Event` move-only in libprocess.
Review: https://reviews.apache.org/r/64347/
{noformat}
{noformat}
commit c9e6a03c02e9f8dc040b937ccd5ae89e5530fd7e
Author: Dmitry Zhuk
Date: Tue Dec 5 11:21:11 2017 -0800
Used `std::move` for `Event`s consumption in the master.
Review: https://reviews.apache.org/r/63641/
{noformat}
{noformat}
commit 8014e3f9e1838745a6f3af7c1e2a557bd74349b0
Author: Dmitry Zhuk
Date: Tue Dec 5 11:20:55 2017 -0800
Added `CallableOnce` support in `Future`.
`Future` guarantees that callbacks are called at most once, so it can
use `lambda::CallableOnce` to expicitly declare this, and allow
corresponding optimizations with moves.
Review: https://reviews.apache.org/r/63638/
{noformat}
{noformat}
commit 09b72e9bbf87793ce84df5d5f9d5f292c60fa5ee
Author: Dmitry Zhuk
Date: Tue Dec 5 11:20:41 2017 -0800
Added `CallableOnce` support in `defer`.
This allows `defer` result to be converted to `CallableOnce`, ensuring
that bound arguments are moved, when call is made, and avoiding making
copies of bound arguments.
Review: https://reviews.apache.org/r/63637/
{noformat}
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: performance, tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16279069#comment-16279069
]
Michael Park commented on MESOS-6972:
-
{noformat}
commit 62b4727310873e80c82516971150082924d9075a
Author: Dmitry Zhuk
Date: Tue Dec 5 11:04:00 2017 -0800
Reduced # of supported arguments in `_Deferred` conversion operators.
Conversion of `_Deferred` to `std::function` and `Deferred` currently
supports up to 12 parameters in function signature. However, this is
unnecessary and is a huge overhead. Most usages require just one
parameter (e.g. when `defer` is used with `Future`). And there are few
usages with two parameters (in `master.cpp` to initialize allocator, and
in `slave.cpp` to install signal handler). This number of parameters is
different from the number of parameters passed to `defer`, but it's
related and can be defined as maximum number of placeholders that can be
passed to `defer`.
Given that `deferred.hpp` is indirectly included in most source files,
it is beneficial to keep this number low. This patch changes maximum
number of parameters to 2.
Review: https://reviews.apache.org/r/64338/
{noformat}
{noformat}
commit 13eda27802cfe05800d4dcbed22c46ca7b46bafb
Author: Dmitry Zhuk
Date: Tue Dec 5 10:40:37 2017 -0800
Prepared `defer` for use in callable-once contexts.
This changes `defer` to use `lambda::partial` instead of `std::bind`,
which allows it be used in callable-once contexts.
Review: https://reviews.apache.org/r/63635/
{noformat}
{noformat}
commit 0d9ce98e9df97be06144d2e29cf23a9c090a06b3
Author: Dmitry Zhuk
Date: Tue Dec 5 10:39:47 2017 -0800
Changed dispatch to use callable once functors.
`dispatch` guarantees that functor will be called at most once, and
therefore it allows optimizations, such as moves of deferred objects.
Review: https://reviews.apache.org/r/63634/
{noformat}
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: performance, tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16279049#comment-16279049
]
Michael Park commented on MESOS-6972:
-
{noformat}
commit ececa115966067551e00098f973f0104a0782730
Author: Michael Park
Date: Tue Dec 5 09:57:51 2017 -0800
Removed `constexpr`-ness from `cpp17::invoke`.
`std::invoke` is not marked `constexpr`.
Review: https://reviews.apache.org/r/64332/
{noformat}
{noformat}
commit 59329cbc605d578572e90db62070b2bb99fdbb9c
Author: Michael Park
Date: Tue Dec 5 09:57:40 2017 -0800
Added more `cpp17::invoke` test cases.
Review: https://reviews.apache.org/r/64331/
{noformat}
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: performance, tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16279050#comment-16279050
]
Michael Park commented on MESOS-6972:
-
{noformat}
commit c9462f4927cfffb1f3a90827467ded730c0f40b9
Author: Dmitry Zhuk dz...@twopensource.com
Date: Tue Dec 5 10:11:28 2017 -0800
Migrated to use the `EventConsumer` interface.
Review: https://reviews.apache.org/r/63632/
{noformat}
{noformat}
commit 6b91f62769a1f8c525162fc716b1d5c231c77811
Author: Dmitry Zhuk dz...@twopensource.com
Date: Tue Dec 5 10:11:23 2017 -0800
Separated `Event` visitation and consumption.
This introduces the `EventConsumer` interface, which add support of
`Event`s with move-only data. This allows consumers to move data out of
`Event`, rather than copying it. This is required to implement move-only
objects support in `defer` to guarantee that deferred function object is
invoked only once, allowing deferred parameters to be moved into call.
Review: https://reviews.apache.org/r/63631/
{noformat}
{noformat}
commit 1074a9c3fd7aa9d2e4b484f86dbe657271abecc0
Author: Dmitry Zhuk dz...@twopensource.com
Date: Tue Dec 5 07:23:28 2017 -0800
Fixed the signature of `CallableOnce::operator()`.
This changes `operator()` signature to match the one defined in
`CallableOnce` template parameter. Previously used form incorrectly
specifies that `operator()` can be invoked with arbitrary number and
types of parameters, which can break other templates using SFINAE to
check if function object can be invoked with specific parameters.
Review: https://reviews.apache.org/r/64337/
{noformat}
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: performance, tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16279048#comment-16279048
]
Michael Park commented on MESOS-6972:
-
{noformat}
commit 8670d2e9224485b94a40654d4a2102d8340fe4ac (private/ci/mpark/utility,
ci/mpark/utility)
Author: Michael Park
Date: Mon Dec 4 17:25:36 2017 -0800
Added `lambda::partial` to .
Review: https://reviews.apache.org/r/64274/
{noformat}
{noformat}
commit 3b9db404cb0b50edef1958423b26364e63ccaa27
Author: Michael Park
Date: Mon Dec 4 17:25:33 2017 -0800
Added `cpp17::invoke` in .
Review: https://reviews.apache.org/r/64248/
{noformat}
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: performance, tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16254343#comment-16254343
]
Benjamin Mahler commented on MESOS-6972:
Move support for install handlers is committed:
{noformat}
commit 8cd1bd2fcb8a4d2330a781a86b62e55ada6d4984 (HEAD -> master, apache/master,
apache/HEAD)
Author: Dmitry Zhuk
Date: Wed Nov 15 14:19:39 2017 -0800
Enabled rvalue reference parameters in protobuf handlers.
Using rvalue reference parameter in protobuf handler opts-out of arena
usage, and allows the handler to move the message.
Review: https://reviews.apache.org/r/63639/
{noformat}
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: performance, tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16046889#comment-16046889
]
Benjamin Mahler commented on MESOS-6972:
[~dzhuk] I see, sounds like we have two options that depend on the install
handler implementation. One case is const-access style, in which arenas and
const access are best, and the other is wanting to move out the data from the
protobuf, in which case non-const access (for moveability) with no arenas is
best.
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16043468#comment-16043468
]
Dmitry Zhuk commented on MESOS-6972:
Yeah, this will not work with arenas, but I'm not sure if arenas are a better
alternative.
I presume you're referring to using arenas for parsing incoming messages?
I just think about {{reregisterSlave}}, which basically takes all incoming data
and puts it inside master's internal data structures. This seems like a typical
flow for handling message: doing some validation, etc. and then passing the
incoming data to the same or another process with {{defer}}. So technically we
get faster message parsing with arena, but then we need to deep copy this data
for passing it further anyway. Without arenas, parsing is slower, but then we
can use {{std::move}} and {{Swap}} (I have some ideas about making something
like {{protobuf::move}} invoking {{Swap}} behind the scenes) to avoid any extra
overhead on copying. Am I missing something here?
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16043178#comment-16043178
]
Benjamin Mahler commented on MESOS-6972:
[~dzhuk] great, yeah I was thinking of this option as well but wasn't sure if
this works when we use arena allocation, since Swap performs deep copies if one
side is from an arena and the other is not:
https://developers.google.com/protocol-buffers/docs/reference/arenas#message-class-methods
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (MESOS-6972) Improve performance of protobuf message passing by removing RepeatedPtrField to vector conversion.
[
https://issues.apache.org/jira/browse/MESOS-6972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16042695#comment-16042695
]
Dmitry Zhuk commented on MESOS-6972:
[~bmahler], what do you think about taking a different approach: since in many
cases {{RepeatedPtrField}} needs to be converted to {{vector}} anyway, e.g. to
pass to internal data structures, we can instead {{Swap}} values from
{{RepeatedPtrField}} to {{vector}}. This does not add too much overhead
compared to {{RepeatedPtrField}}.
Currently a handler can be installed by calling something like this:
{code}
template
void install(
void (T::*method)(const process::UPID&, P1C),
P1 (M::*param1)() const);
{code}
{{P1 (M::*param1)() const}} is an issue here, as it prevents from swapping from
returned const {{RepeatedPtrField}}.
We can either remove {{const}} from {{param1}} and update code to use
{{mutable_*}} versions to access message properties, or keep it as is, and do
{{const_cast}} as a part of conversion. The latter however, somewhat depends on
protobuf internals and could break for non-primitive defaults.
I've already tested this change ({{const_cast}} variant), and results look
promising for hotspots like {{reregisterSlave}}.
> Improve performance of protobuf message passing by removing RepeatedPtrField
> to vector conversion.
> --
>
> Key: MESOS-6972
> URL: https://issues.apache.org/jira/browse/MESOS-6972
> Project: Mesos
> Issue Type: Improvement
>Reporter: Benjamin Mahler
> Labels: tech-debt
>
> Currently, all protobuf message handlers must take a {{vector}} for repeated
> fields, rather than a {{RepeatedPtrField}}.
> This requires that a copy be performed of the repeated field's entries (see
> [here|https://github.com/apache/mesos/blob/9228ebc239dac42825390bebc72053dbf3ae7b09/3rdparty/libprocess/include/process/protobuf.hpp#L78-L87]),
> which can be very expensive in some cases. We should avoid requiring this
> expense on the callers.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
