From fbcd5e2f1e44a27b2282e026b1cf58fb17efb56e Mon Sep 17 00:00:00 2001 From: John Preston Date: Thu, 14 Sep 2017 22:27:41 +0300 Subject: [PATCH] Try to use const-ref better in rpl. --- Telegram/SourceFiles/base/observer.h | 3 +- .../history/history_shared_media.cpp | 7 +- .../history/history_user_photos.cpp | 5 +- Telegram/SourceFiles/rpl/consumer.h | 83 +++++++++++++++++-- .../SourceFiles/rpl/distinct_until_changed.h | 8 +- Telegram/SourceFiles/rpl/event_stream.h | 23 ++--- Telegram/SourceFiles/rpl/filter.h | 8 +- Telegram/SourceFiles/rpl/flatten_latest.h | 12 +-- Telegram/SourceFiles/rpl/map.h | 46 ++++++++-- Telegram/SourceFiles/rpl/then.h | 16 ++-- .../storage/storage_shared_media.cpp | 2 +- .../storage/storage_user_photos.cpp | 2 +- Telegram/SourceFiles/ui/widgets/labels.cpp | 8 +- Telegram/SourceFiles/ui/wrap/wrap.h | 2 +- .../SourceFiles/window/player_wrap_widget.cpp | 4 +- .../SourceFiles/window/player_wrap_widget.h | 2 +- Telegram/gyp/tests/tests.gyp | 1 + 17 files changed, 171 insertions(+), 61 deletions(-) diff --git a/Telegram/SourceFiles/base/observer.h b/Telegram/SourceFiles/base/observer.h index 3b505e80c..e557aeeae 100644 --- a/Telegram/SourceFiles/base/observer.h +++ b/Telegram/SourceFiles/base/observer.h @@ -470,7 +470,8 @@ inline rpl::producer ObservableViewer( auto lifetime = rpl::lifetime(); lifetime.make_state( observable.add_subscription([consumer](auto &&update) { - consumer.put_next_copy(update); + consumer.put_next_forward( + std::forward(update)); })); return lifetime; }; diff --git a/Telegram/SourceFiles/history/history_shared_media.cpp b/Telegram/SourceFiles/history/history_shared_media.cpp index a4cb3bfd0..9717943f6 100644 --- a/Telegram/SourceFiles/history/history_shared_media.cpp +++ b/Telegram/SourceFiles/history/history_shared_media.cpp @@ -434,14 +434,14 @@ rpl::producer SharedMediaViewer( limitBefore, limitAfter); auto applyUpdate = [=](auto &&update) { - if (builder->applyUpdate(std::move(update))) { + if (builder->applyUpdate(std::forward(update))) { consumer.put_next(builder->snapshot()); } }; auto requestMediaAround = [ peer = App::peer(key.peerId), type = key.type - ](SharedMediaSliceBuilder::AroundData data) { + ](const SharedMediaSliceBuilder::AroundData &data) { Auth().api().requestSharedMedia( peer, type, @@ -465,8 +465,7 @@ rpl::producer SharedMediaViewer( Auth().storage().query(Storage::SharedMediaQuery( key, limitBefore, - limitAfter - )) + limitAfter)) | rpl::on_next(applyUpdate) | rpl::on_done([=] { builder->checkInsufficientMedia(); }) | rpl::start(lifetime); diff --git a/Telegram/SourceFiles/history/history_user_photos.cpp b/Telegram/SourceFiles/history/history_user_photos.cpp index 823421f70..1e4b9fe60 100644 --- a/Telegram/SourceFiles/history/history_user_photos.cpp +++ b/Telegram/SourceFiles/history/history_user_photos.cpp @@ -222,11 +222,12 @@ rpl::producer UserPhotosViewer( limitBefore, limitAfter); auto applyUpdate = [=](auto &&update) { - if (builder->applyUpdate(std::move(update))) { + if (builder->applyUpdate(std::forward(update))) { consumer.put_next(builder->snapshot()); } }; - auto requestPhotosAround = [user = App::user(key.userId)](PhotoId photoId) { + auto requestPhotosAround = [user = App::user(key.userId)]( + PhotoId photoId) { Auth().api().requestUserPhotos(user, photoId); }; builder->insufficientPhotosAround() diff --git a/Telegram/SourceFiles/rpl/consumer.h b/Telegram/SourceFiles/rpl/consumer.h index 38366a5da..bbd69f985 100644 --- a/Telegram/SourceFiles/rpl/consumer.h +++ b/Telegram/SourceFiles/rpl/consumer.h @@ -25,6 +25,28 @@ Copyright (c) 2014-2017 John Preston, https://desktop.telegram.org #include namespace rpl { +namespace details { + +template +const Arg &const_ref_val(); + +template < + typename Func, + typename Arg, + typename = decltype(std::declval()(const_ref_val()))> +void const_ref_call_helper(Func &handler, const Arg &value, int) { + handler(value); +} + +template < + typename Func, + typename Arg> +void const_ref_call_helper(Func &handler, const Arg &value, double) { + auto copy = value; + handler(std::move(copy)); +} + +} // namespace details struct no_value { no_value() = delete; @@ -57,15 +79,27 @@ public: bool put_next(Value &&value) const; bool put_next_copy(const Value &value) const; + bool put_next_forward(Value &&value) const { + return put_next(std::move(value)); + } + bool put_next_forward(const Value &value) const { + return put_next_copy(value); + } void put_error(Error &&error) const; void put_error_copy(const Error &error) const; + void put_error_forward(Error &&error) const { + return put_error(std::move(error)); + } + void put_error_forward(const Error &error) const { + return put_error_copy(error); + } void put_done() const; void add_lifetime(lifetime &&lifetime) const; template Type *make_state(Args&& ...args) const; - + void terminate() const; bool operator==(const consumer &other) const { @@ -108,7 +142,9 @@ template class consumer::abstract_consumer_instance { public: virtual bool put_next(Value &&value) = 0; + virtual bool put_next_copy(const Value &value) = 0; virtual void put_error(Error &&error) = 0; + virtual void put_error_copy(const Error &error) = 0; virtual void put_done() = 0; void add_lifetime(lifetime &&lifetime); @@ -141,7 +177,9 @@ public: } bool put_next(Value &&value) override; + bool put_next_copy(const Value &value) override; void put_error(Error &&error) override; + void put_error_copy(const Error &error) override; void put_done() override; private: @@ -197,8 +235,13 @@ bool consumer::put_next(Value &&value) const { template bool consumer::put_next_copy(const Value &value) const { - auto copy = value; - return put_next(std::move(copy)); + if (_instance) { + if (_instance->put_next_copy(value)) { + return true; + } + _instance = nullptr; + } + return false; } template @@ -210,8 +253,9 @@ void consumer::put_error(Error &&error) const { template void consumer::put_error_copy(const Error &error) const { - auto copy = error; - return put_error(std::move(error)); + if (_instance) { + std::exchange(_instance, nullptr)->put_error_copy(error); + } } template @@ -293,6 +337,21 @@ bool consumer::consumer_instance::put_nex return true; } +template +template +bool consumer::consumer_instance::put_next_copy( + const Value &value) { + std::unique_lock lock(this->_mutex); + if (this->_terminated) { + return false; + } + auto handler = this->_next; + lock.unlock(); + + details::const_ref_call_helper(handler, value, 0); + return true; +} + template template void consumer::consumer_instance::put_error( @@ -307,6 +366,20 @@ void consumer::consumer_instance::put_err } } +template +template +void consumer::consumer_instance::put_error_copy( + const Error &error) { + std::unique_lock lock(this->_mutex); + if (!this->_terminated) { + auto handler = std::move(this->_error); + lock.unlock(); + + details::const_ref_call_helper(handler, error, 0); + this->terminate(); + } +} + template template void consumer::consumer_instance::put_done() { diff --git a/Telegram/SourceFiles/rpl/distinct_until_changed.h b/Telegram/SourceFiles/rpl/distinct_until_changed.h index 10b774eae..ac03c52fd 100644 --- a/Telegram/SourceFiles/rpl/distinct_until_changed.h +++ b/Telegram/SourceFiles/rpl/distinct_until_changed.h @@ -37,13 +37,13 @@ public: base::optional >(); return std::move(initial).start( - [consumer, previous](Value &&value) { + [consumer, previous](auto &&value) { if (!(*previous) || (**previous) != value) { *previous = value; - consumer.put_next(std::move(value)); + consumer.put_next_forward(std::forward(value)); } - }, [consumer](Error &&error) { - consumer.put_error(std::move(error)); + }, [consumer](auto &&error) { + consumer.put_error_forward(std::forward(error)); }, [consumer] { consumer.put_done(); }); diff --git a/Telegram/SourceFiles/rpl/event_stream.h b/Telegram/SourceFiles/rpl/event_stream.h index 947eb6198..e56f0bba7 100644 --- a/Telegram/SourceFiles/rpl/event_stream.h +++ b/Telegram/SourceFiles/rpl/event_stream.h @@ -37,10 +37,13 @@ public: event_stream(); event_stream(event_stream &&other); - void fire(Value &&value); + template + void fire_forward(OtherValue &&value); + void fire(Value &&value) { + return fire_forward(std::move(value)); + } void fire_copy(const Value &value) { - auto copy = value; - fire(std::move(copy)); + return fire_forward(value); } producer events() const; producer events_starting_with( @@ -49,8 +52,7 @@ public: } producer events_starting_with_copy( const Value &value) const { - auto copy = value; - return events_starting_with(std::move(copy)); + return single(value) | then(events()); } ~event_stream(); @@ -72,7 +74,8 @@ event_stream::event_stream(event_stream &&other) } template -void event_stream::fire(Value &&value) { +template +void event_stream::fire_forward(OtherValue &&value) { if (!_consumers) { return; } @@ -87,8 +90,8 @@ void event_stream::fire(Value &&value) { return !consumer.put_next_copy(value); }); - // Move value for the last consumer. - if (prev->put_next(std::move(value))) { + // Perhaps move value for the last consumer. + if (prev->put_next_forward(std::forward(value))) { if (removeFrom != prev) { *removeFrom++ = std::move(*prev); } else { @@ -152,8 +155,8 @@ event_stream::~event_stream() { template inline auto to_stream(event_stream &stream) { - return on_next([&stream](Value &&value) { - stream.fire(std::move(value)); + return on_next([&stream](auto &&value) { + stream.fire_forward(std::forward(value)); }); } diff --git a/Telegram/SourceFiles/rpl/filter.h b/Telegram/SourceFiles/rpl/filter.h index ff6cc49b4..8826ff2c9 100644 --- a/Telegram/SourceFiles/rpl/filter.h +++ b/Telegram/SourceFiles/rpl/filter.h @@ -45,13 +45,13 @@ public: [ consumer, predicate = std::move(predicate) - ](Value &&value) { + ](auto &&value) { const auto &immutable = value; if (predicate(immutable)) { - consumer.put_next(std::move(value)); + consumer.put_next_forward(std::forward(value)); } - }, [consumer](Error &&error) { - consumer.put_error(std::move(error)); + }, [consumer](auto &&error) { + consumer.put_error_forward(std::forward(error)); }, [consumer] { consumer.put_done(); }); diff --git a/Telegram/SourceFiles/rpl/flatten_latest.h b/Telegram/SourceFiles/rpl/flatten_latest.h index b237c8d7d..2d8e519b4 100644 --- a/Telegram/SourceFiles/rpl/flatten_latest.h +++ b/Telegram/SourceFiles/rpl/flatten_latest.h @@ -40,10 +40,10 @@ public: [consumer, state](rpl::producer &&inner) { state->finished = false; state->alive = std::move(inner).start( - [consumer](Value &&value) { - consumer.put_next(std::move(value)); - }, [consumer](Error &&error) { - consumer.put_error(std::move(error)); + [consumer](auto &&value) { + consumer.put_next_forward(std::forward(value)); + }, [consumer](auto &&error) { + consumer.put_error_forward(std::forward(error)); }, [consumer, state] { if (state->finished) { consumer.put_done(); @@ -51,8 +51,8 @@ public: state->finished = true; } }); - }, [consumer](Error &&error) { - consumer.put_error(std::move(error)); + }, [consumer](auto &&error) { + consumer.put_error_forward(std::forward(error)); }, [consumer, state] { if (state->finished) { consumer.put_done(); diff --git a/Telegram/SourceFiles/rpl/map.h b/Telegram/SourceFiles/rpl/map.h index 1e7f28dd9..80e3dab2d 100644 --- a/Telegram/SourceFiles/rpl/map.h +++ b/Telegram/SourceFiles/rpl/map.h @@ -25,6 +25,40 @@ Copyright (c) 2014-2017 John Preston, https://desktop.telegram.org namespace rpl { namespace details { +template < + typename Transform, + typename NewValue, + typename Error, + typename Value> +class map_transform_helper { +public: + map_transform_helper( + const consumer &consumer, + Transform &&transform) + : _transform(std::move(transform)) + , _consumer(consumer) { + } + template < + typename OtherValue, + typename = std::enable_if_t< + std::is_rvalue_reference_v>> + void operator()(OtherValue &&value) const { + _consumer.put_next_forward(_transform(std::move(value))); + } + template < + typename OtherValue, + typename = decltype( + std::declval()(const_ref_val()))> + void operator()(const OtherValue &value) const { + _consumer.put_next_forward(_transform(value)); + } + +private: + consumer _consumer; + Transform _transform; + +}; + template class map_helper { public: @@ -46,13 +80,11 @@ public: transform = std::move(_transform) ](const consumer &consumer) mutable { return std::move(initial).start( - [ - consumer, - transform = std::move(transform) - ](Value &&value) { - consumer.put_next(transform(std::move(value))); - }, [consumer](Error &&error) { - consumer.put_error(std::move(error)); + map_transform_helper( + consumer, + std::move(transform) + ), [consumer](auto &&error) { + consumer.put_error_forward(std::forward(error)); }, [consumer] { consumer.put_done(); }); diff --git a/Telegram/SourceFiles/rpl/then.h b/Telegram/SourceFiles/rpl/then.h index a2be3627c..84aa20cb7 100644 --- a/Telegram/SourceFiles/rpl/then.h +++ b/Telegram/SourceFiles/rpl/then.h @@ -34,19 +34,19 @@ auto then(producer &&following) { following = std::move(following) ](const consumer &consumer) mutable { return std::move(initial).start( - [consumer](Value &&value) { - consumer.put_next(std::move(value)); - }, [consumer](Error &&error) { - consumer.put_error(std::move(error)); + [consumer](auto &&value) { + consumer.put_next_forward(std::forward(value)); + }, [consumer](auto &&error) { + consumer.put_error_forward(std::forward(error)); }, [ consumer, following = std::move(following) ]() mutable { consumer.add_lifetime(std::move(following).start( - [consumer](Value &&value) { - consumer.put_next(std::move(value)); - }, [consumer](Error &&error) { - consumer.put_error(std::move(error)); + [consumer](auto &&value) { + consumer.put_next_forward(std::forward(value)); + }, [consumer](auto &&error) { + consumer.put_error_forward(std::forward(error)); }, [consumer] { consumer.put_done(); })); diff --git a/Telegram/SourceFiles/storage/storage_shared_media.cpp b/Telegram/SourceFiles/storage/storage_shared_media.cpp index f812b4b42..1d0f1e5e9 100644 --- a/Telegram/SourceFiles/storage/storage_shared_media.cpp +++ b/Telegram/SourceFiles/storage/storage_shared_media.cpp @@ -234,7 +234,7 @@ std::map::iterator auto type = static_cast(index); list.sliceUpdated() - | rpl::on_next([this, peer, type](SliceUpdate &&update) { + | rpl::on_next([this, peer, type](const SliceUpdate &update) { _sliceUpdated.fire(SharedMediaSliceUpdate( peer, type, diff --git a/Telegram/SourceFiles/storage/storage_user_photos.cpp b/Telegram/SourceFiles/storage/storage_user_photos.cpp index 59e589497..817f043e9 100644 --- a/Telegram/SourceFiles/storage/storage_user_photos.cpp +++ b/Telegram/SourceFiles/storage/storage_user_photos.cpp @@ -122,7 +122,7 @@ UserPhotos::enforceLists(UserId user) { } result = _lists.emplace(user, List {}).first; result->second.sliceUpdated( - ) | rpl::on_next([this, user](SliceUpdate &&update) { + ) | rpl::on_next([this, user](const SliceUpdate &update) { _sliceUpdated.fire(UserPhotosSliceUpdate( user, update.photoIds, diff --git a/Telegram/SourceFiles/ui/widgets/labels.cpp b/Telegram/SourceFiles/ui/widgets/labels.cpp index 728e38c66..d3d932530 100644 --- a/Telegram/SourceFiles/ui/widgets/labels.cpp +++ b/Telegram/SourceFiles/ui/widgets/labels.cpp @@ -171,8 +171,8 @@ FlatLabel::FlatLabel( , _st(st) , _contextCopyText(lang(lng_context_copy_text)) { std::move(text) - | rpl::on_next([this](QString &&value) { - setText(std::move(value)); + | rpl::on_next([this](const QString &value) { + setText(value); }) | rpl::start(lifetime()); } @@ -186,8 +186,8 @@ FlatLabel::FlatLabel( , _st(st) , _contextCopyText(lang(lng_context_copy_text)) { std::move(text) - | rpl::on_next([this](TextWithEntities &&value) { - setMarkedText(std::move(value)); + | rpl::on_next([this](const TextWithEntities &value) { + setMarkedText(value); }) | rpl::start(lifetime()); } diff --git a/Telegram/SourceFiles/ui/wrap/wrap.h b/Telegram/SourceFiles/ui/wrap/wrap.h index 0f568cae7..27285f3c5 100644 --- a/Telegram/SourceFiles/ui/wrap/wrap.h +++ b/Telegram/SourceFiles/ui/wrap/wrap.h @@ -126,7 +126,7 @@ Wrap::Wrap(QWidget *parent, object_ptr child) , _wrapped(std::move(child)) { if (_wrapped) { _wrapped->sizeValue() - | rpl::on_next([this](QSize &&value) { + | rpl::on_next([this](const QSize &value) { wrappedSizeUpdated(value); }) | rpl::start(lifetime()); diff --git a/Telegram/SourceFiles/window/player_wrap_widget.cpp b/Telegram/SourceFiles/window/player_wrap_widget.cpp index 5c0419e06..235967106 100644 --- a/Telegram/SourceFiles/window/player_wrap_widget.cpp +++ b/Telegram/SourceFiles/window/player_wrap_widget.cpp @@ -27,13 +27,13 @@ namespace Window { PlayerWrapWidget::PlayerWrapWidget(QWidget *parent) : Parent(parent, object_ptr(parent)) { sizeValue() - | rpl::on_next([this](QSize &&size) { + | rpl::on_next([this](const QSize &size) { updateShadowGeometry(size); }) | rpl::start(lifetime()); } -void PlayerWrapWidget::updateShadowGeometry(QSize size) { +void PlayerWrapWidget::updateShadowGeometry(const QSize &size) { auto skip = Adaptive::OneColumn() ? 0 : st::lineWidth; entity()->setShadowGeometryToLeft( skip, diff --git a/Telegram/SourceFiles/window/player_wrap_widget.h b/Telegram/SourceFiles/window/player_wrap_widget.h index 2d8825ea6..85fba6e90 100644 --- a/Telegram/SourceFiles/window/player_wrap_widget.h +++ b/Telegram/SourceFiles/window/player_wrap_widget.h @@ -29,7 +29,7 @@ public: } private: - void updateShadowGeometry(QSize size); + void updateShadowGeometry(const QSize &size); }; diff --git a/Telegram/gyp/tests/tests.gyp b/Telegram/gyp/tests/tests.gyp index c3ddd5d5c..421bceb20 100644 --- a/Telegram/gyp/tests/tests.gyp +++ b/Telegram/gyp/tests/tests.gyp @@ -98,6 +98,7 @@ ], 'sources': [ '<(src_loc)/rpl/before_next.h', + '<(src_loc)/rpl/combine_latest.h', '<(src_loc)/rpl/complete.h', '<(src_loc)/rpl/consumer.h', '<(src_loc)/rpl/deferred.h',