mutation_partition_view: pass cell by value to visitor

mutation_partition_view needs to create an atomic_cell from
IDL-serialised data. Then that cell is passed to the visitor. However,
because generic mutation_partition_visitor interface was used, the cell
was passed by constant reference which forced the visitor to needlessly
copy it.

This patch takes advantage of the fact that mutation_partition_view is
devirtualised now and adjust the interfaces of its visitors so that the
cell can be passed without copying.
This commit is contained in:
Paweł Dziepak
2018-03-16 13:18:13 +00:00
parent 569176aad1
commit bf330a99f0
4 changed files with 33 additions and 26 deletions

View File

@@ -92,6 +92,10 @@ public:
_p.apply(t);
}
void accept_static_cell(column_id id, atomic_cell cell) {
return accept_static_cell(id, atomic_cell_view(cell));
}
virtual void accept_static_cell(column_id id, atomic_cell_view cell) override {
const column_mapping_entry& col = _visited_column_mapping.static_column_at(id);
const column_definition* def = _p_schema.get_column_definition(col.name());
@@ -119,6 +123,10 @@ public:
_current_row = &r;
}
void accept_row_cell(column_id id, atomic_cell cell) {
return accept_row_cell(id, atomic_cell_view(cell));
}
virtual void accept_row_cell(column_id id, atomic_cell_view cell) override {
const column_mapping_entry& col = _visited_column_mapping.regular_column_at(id);
const column_definition* def = _p_schema.get_column_definition(col.name());

View File

@@ -134,14 +134,7 @@ void read_and_visit_row(ser::row_view rv, const column_mapping& cm, column_kind
if (!_col.type()->is_atomic()) {
throw std::runtime_error("A collection expected, got an atomic cell");
}
// FIXME: Pass view to cell to avoid copy
auto&& outer = current_allocator();
with_allocator(standard_allocator(), [&] {
auto cell = read_atomic_cell(*_col.type(), acv);
with_allocator(outer, [&] {
_visitor.accept_atomic_cell(_id, cell);
});
});
_visitor.accept_atomic_cell(_id, read_atomic_cell(*_col.type(), acv));
}
void operator()(ser::collection_cell_view& ccv) const {
if (_col.type()->is_atomic()) {
@@ -200,8 +193,8 @@ void mutation_partition_view::do_accept(const column_mapping& cm, Visitor& visit
struct static_row_cell_visitor {
Visitor& _visitor;
void accept_atomic_cell(column_id id, const atomic_cell& ac) const {
_visitor.accept_static_cell(id, ac);
void accept_atomic_cell(column_id id, atomic_cell ac) const {
_visitor.accept_static_cell(id, std::move(ac));
}
void accept_collection(column_id id, const collection_mutation& cm) const {
_visitor.accept_static_cell(id, cm);
@@ -220,8 +213,8 @@ void mutation_partition_view::do_accept(const column_mapping& cm, Visitor& visit
struct cell_visitor {
Visitor& _visitor;
void accept_atomic_cell(column_id id, const atomic_cell& ac) const {
_visitor.accept_row_cell(id, ac);
void accept_atomic_cell(column_id id, atomic_cell ac) const {
_visitor.accept_row_cell(id, std::move(ac));
}
void accept_collection(column_id id, const collection_mutation& cm) const {
_visitor.accept_row_cell(id, cm);
@@ -280,9 +273,8 @@ mutation_fragment frozen_mutation_fragment::unfreeze(const schema& s)
public:
clustering_row_builder(const schema& s, clustering_key key, row_tombstone t, row_marker m)
: _s(s), _mf(mutation_fragment::clustering_row_tag_t(), std::move(key), std::move(t), std::move(m), row()) { }
void accept_atomic_cell(column_id id, const atomic_cell& ac) {
auto& type = *_s.regular_column_at(id).type;
_mf.as_mutable_clustering_row().cells().append_cell(id, atomic_cell_or_collection(atomic_cell(type, ac)));
void accept_atomic_cell(column_id id, atomic_cell ac) {
_mf.as_mutable_clustering_row().cells().append_cell(id, atomic_cell_or_collection(std::move(ac)));
}
void accept_collection(column_id id, const collection_mutation& cm) {
auto& ctype = *static_pointer_cast<const collection_type_impl>(_s.regular_column_at(id).type);
@@ -303,9 +295,8 @@ mutation_fragment frozen_mutation_fragment::unfreeze(const schema& s)
mutation_fragment _mf;
public:
explicit static_row_builder(const schema& s) : _s(s), _mf(static_row()) { }
void accept_atomic_cell(column_id id, const atomic_cell& ac) {
auto& type = *_s.static_column_at(id).type;
_mf.as_mutable_static_row().cells().append_cell(id, atomic_cell_or_collection(atomic_cell(type, ac)));
void accept_atomic_cell(column_id id, atomic_cell ac) {
_mf.as_mutable_static_row().cells().append_cell(id, atomic_cell_or_collection(std::move(ac)));
}
void accept_collection(column_id id, const collection_mutation& cm) {
auto& ctype = *static_pointer_cast<const collection_type_impl>(_s.static_column_at(id).type);

View File

@@ -34,17 +34,17 @@ class converting_mutation_partition_applier;
GCC6_CONCEPT(
template<typename T>
concept bool MutationViewVisitor = requires (T visitor, tombstone t, atomic_cell_view acv,
concept bool MutationViewVisitor = requires (T visitor, tombstone t, atomic_cell ac,
collection_mutation_view cmv, range_tombstone rt,
position_in_partition_view pipv, row_tombstone row_tomb,
row_marker rm) {
visitor.accept_partition_tombstone(t);
visitor.accept_static_cell(column_id(), acv);
visitor.accept_static_cell(column_id(), std::move(ac));
visitor.accept_static_cell(column_id(), cmv);
visitor.accept_row_tombstone(rt);
visitor.accept_row(pipv, row_tomb, rm,
is_dummy::no, is_continuous::yes);
visitor.accept_row_cell(column_id(), acv);
visitor.accept_row_cell(column_id(), std::move(ac));
visitor.accept_row_cell(column_id(), cmv);
};
)

View File

@@ -25,7 +25,7 @@
#include "mutation_partition_view.hh"
// Partition visitor which builds mutation_partition corresponding to the data its fed with.
class partition_builder : public mutation_partition_visitor {
class partition_builder final : public mutation_partition_visitor {
private:
const schema& _schema;
mutation_partition& _partition;
@@ -43,9 +43,13 @@ public:
}
virtual void accept_static_cell(column_id id, atomic_cell_view cell) override {
row& r = _partition.static_row();
auto& cdef = _schema.static_column_at(id);
r.append_cell(id, atomic_cell_or_collection(*cdef.type, cell));
accept_static_cell(id, atomic_cell(*cdef.type, cell));
}
void accept_static_cell(column_id id, atomic_cell&& cell) {
row& r = _partition.static_row();
r.append_cell(id, atomic_cell_or_collection(std::move(cell)));
}
virtual void accept_static_cell(column_id id, collection_mutation_view collection) override {
@@ -66,9 +70,13 @@ public:
}
virtual void accept_row_cell(column_id id, atomic_cell_view cell) override {
row& r = _current_row->cells();
auto& cdef = _schema.regular_column_at(id);
r.append_cell(id, atomic_cell_or_collection(*cdef.type, cell));
accept_row_cell(id, atomic_cell(*cdef.type, cell));
}
void accept_row_cell(column_id id, atomic_cell&& cell) {
row& r = _current_row->cells();
r.append_cell(id, atomic_cell_or_collection(std::move(cell)));
}
virtual void accept_row_cell(column_id id, collection_mutation_view collection) override {