[prog.c++] Очередные шаблоны против копипасты: упоролся тут на почве обеспечения гарантии strong exception safety

Делаю новую фичу в SObjectizer и в одном месте потребовалось реализовать вставку элемента в словарь из словарей словарей. Сначала сделал просто. Потом начал делать нормально ;)

И дошел до такой жести, которая пока что шокирует даже меня. Уж не знаю, останется ли это как оно есть сейчас или же будет еще как-то переделано, но захотелось рассказать о том, что получилось. Не то, чтобы "чистапаржать", но где-то в этом духе :)

Итак, есть вот такая вот структура данных (ключевой тип -- это bindings_map_t):

using one_sink_bindings_t = std::map< std::type_index, single_sink_binding_t >; using one_mbox_bindings_t = std::map<       msink_t,       one_sink_bindings_t,       so_5::impl::msink_less_comparator_t >; using bindings_map_t = std::map< mbox_id_t, one_mbox_bindings_t >;

Здесь смысл вот какой: может быть много mbox-ов, а у каждого из mbox-ов может быть несколько подписчиков. Отсюда и bindings_map_t, в котором ключом идентификатор mbox-а, а значениями -- описания подписчиков (в качестве описания one_mbox_bindings_t).

Итак, у mbox-а несколько подписчиков. А подписчик может иметь несколько разных подписок. Отсюда и one_mbox_bindings_t, где ключом является умный указатель на подписчика (msink), а значением -- описания подписок (в качестве описания one_sink_bindings_t).

Т.к. каждый подписчик может иметь несколько подписок, то появляется еще один словарь (one_sink_bindings_t), в котором ключом является идентификатор типа сообщения, а значением объект, который хранит подписку.

Соответственно, когда для mbox-а нужно создать новую подписку, то сперва ищется mbox в bindings_map_t. Если таковой не найден, то в bindings_map_t вносится новый элемент, если найдет, то используется уже имеющийся.

Точно так же затем в one_mbox_bindings_t проверяется есть ли информация по msink-у. Если нет, то вносится новая, если есть, то используется существующая.

Потом аналогично проверяется и one_sink_bindings_t. Только там наличие уже существующей информации рассматривается как ошибка: такая подписка есть, негоже создавать новую точно такую же.

Соответственно, если решать эту задачу "в лоб", то получается просто и достаточно компактно:

class actual_binding_handler_t    {       bindings_map_t m_bindings;       template< typename Single_Sink_Modificator >       void       do_actual_bind(          const std::type_index & msg_type,          const mbox_t & from,          const msink_t & dest,          Single_Sink_Modificator && single_sink_modificator )          {             auto it_mbox = m_bindings.find( from->id() );             if( it_mbox == m_bindings.end() )                it_mbox = m_bindings.emplace( from->id(), one_mbox_bindings_t{} ).first;             auto & msinks = it_mbox->second;             auto it_msink = msinks.find( dest );             if( it_msink == msinks.end() )                it_msink = msinks.emplace( dest, one_sink_bindings_t{} ).first;             auto & msgs = it_msink->second;             auto it_msg = msgs.find( msg_type );             if( it_msg == msgs.end() )                it_msg = msgs.emplace( msg_type, single_sink_binding_t{} ).first;             else                {                   SO_5_THROW_EXCEPTION(                         rc_evt_handler_already_provided,                         std::string{ "msink already subscribed to a message" } +                         "(mbox:'" + from->query_name() +                         "', msg_type:'" + msg_type.name() + "'" );                }             single_sink_modificator( msg_type, it_msg->second );          }

Одна проблема: этот код обеспечивает только гарантию basic exception safety. Т.е. если где-то по ходу работы выскочит исключение, то не произойдет никаких утечек. Но ряд словарей может остаться модифицированными. Так, если исключение выскочит на самом последнем шаге, когда вставка информации во все словари уже произошла, то эта самая информация (пусть даже в минимальном виде) в словарях останется. Хотя она там и не нужна.

Поэтому я сделал первую попытку написать do_actual_bind так, чтобы обеспечивалась гарантия strong exception safety. Т.е. чтобы ранее сделанные изменения автоматически откатывались при вылете исключения.

Получился вот такой вот макаронный монстр:

class actual_binding_handler_t    {       bindings_map_t m_bindings;       template< typename Single_Sink_Modificator >       void       do_actual_bind(          const std::type_index & msg_type,          const mbox_t & from,          const msink_t & dest,          Single_Sink_Modificator && single_sink_modificator )          {             auto it_mbox = m_bindings.find( from->id() );             bool bindings_modified = false;             if( it_mbox == m_bindings.end() )                {                   it_mbox = m_bindings.emplace(                         from->id(),                         one_mbox_bindings_t{} ).first;                   bindings_modified = true;                }             so_5::details::do_with_rollback_on_exception(                   [&]() {                      auto & msinks = it_mbox->second;                      auto it_msink = msinks.find( dest );                      bool msinks_modified = false;                      if( it_msink == msinks.end() )                         {                            it_msink = msinks.emplace(                                  dest,                                  one_sink_bindings_t{} ).first;                            msinks_modified = true;                         }                      so_5::details::do_with_rollback_on_exception(                            [&]() {                               auto & msgs = it_msink->second;                               auto it_msg = msgs.find( msg_type );                               bool msgs_modified = false;                               if( it_msg == msgs.end() )                                  {                                     it_msg = msgs.emplace(                                           msg_type,                                           single_sink_binding_t{} ).first;                                     msgs_modified = true;                                  }                               else                                  {                                     SO_5_THROW_EXCEPTION(                                           rc_evt_handler_already_provided,                                           std::string{ "msink already subscribed to a message" } +                                           "(mbox:'" + from->query_name() +                                           "', msg_type:'" + msg_type.name() + "'" );                                  }                               so_5::details::do_with_rollback_on_exception(                                     [&]() {                                        single_sink_modificator( msg_type, it_msg->second );                                     },                                     [&msgs, &it_msg, msgs_modified]() {                                        if( msgs_modified )                                           msgs.erase( it_msg );                                     } );                            },                            [&msinks, it_msink, msinks_modified]() {                               if( msinks_modified )                                  msinks.erase( it_msink );                            } );                   },                   [this, &it_mbox, bindings_modified]() {                      if( bindings_modified )                         m_bindings.erase( it_mbox );                   } );          }

Здесь вспомогательная do_with_rollback_on_exception -- это что-то вроде блока try..catch с повторным пробросом исключения в catch (но, надеюсь, гораздо более эффективное, т.к. работает без try..catch, а на базе деструкторов, которые вызываются при раскрутке стека).

Понятное дело, что подобная простыня оскорбляет мое чувство прекрасного, поэтому начались поиски более компактного варианта. Который, после нескольких не очень длительных итераций, был найден вот в таком виде:

class actual_binding_handler_t    {       bindings_map_t m_bindings;       template< typename Single_Sink_Modificator >       void       do_actual_bind(          const std::type_index & msg_type,          const mbox_t & from,          const msink_t & dest,          Single_Sink_Modificator && single_sink_modificator )          {             insertion_it_with_auto_erase_if_not_committed_t it_mbox{ m_bindings, from->id() };             insertion_it_with_auto_erase_if_not_committed_t it_msink{ it_mbox->second, dest };             insertion_it_with_auto_erase_if_not_committed_t it_msg{ it_msink->second, msg_type };             // If new item wasn't inserted then it's an error.             if( !it_msg.modified() )                {                   SO_5_THROW_EXCEPTION(                         rc_evt_handler_already_provided,                         std::string{ "msink already subscribed to a message" } +                         "(mbox:'" + from->query_name() +                         "', msg_type:'" + msg_type.name() + "'" );                }             single_sink_modificator( msg_type, it_msg->second );             it_msg.commit();             it_msink.commit();             it_mbox.commit();          }

Как можно догадаться, вся эта лаконичная красота достигается благодаря магическому классу insertion_it_with_auto_erase_if_not_committed_t. А вот его реализация уже не такая компактная, как мне хотелось бы:

template< class Container > class insertion_it_with_auto_erase_if_not_committed_t final    {       using iterator_type = typename Container::iterator;       Container & m_container;       iterator_type m_it;       bool m_modified;       bool m_commited{ false };    public:       insertion_it_with_auto_erase_if_not_committed_t(          Container & container,          typename Container::key_type const & k )          :  m_container{ container }          ,  m_it{ container.find( k ) }          {             if( container.end() == m_it )                {                   m_it = m_container.emplace( k, typename Container::mapped_type{} ).first;                   m_modified = true;                }             else                m_modified = false;          }       insertion_it_with_auto_erase_if_not_committed_t(          const insertion_it_with_auto_erase_if_not_committed_t & ) = delete;       insertion_it_with_auto_erase_if_not_committed_t(          insertion_it_with_auto_erase_if_not_committed_t && ) = delete;       ~insertion_it_with_auto_erase_if_not_committed_t() noexcept          {             if( m_modified && !m_commited )                m_container.erase( m_it );          }       void       commit() noexcept          {             m_commited = true;          }       [[nodiscard]]       iterator_type       operator->() const          {             return m_it;          }       [[nodiscard]]       bool       modified() const noexcept          {             return m_modified;          }    }; // Deduction guide for insertion_it_with_auto_erase_if_not_committed_t. template<typename C, typename K> insertion_it_with_auto_erase_if_not_committed_t(C &, const K&) ->       insertion_it_with_auto_erase_if_not_committed_t<C>;

Принцип работы прост: в конструкторе пытаемся найти элемент в указанном контейнере по указанному ключу. Если найден, то OK, сохраняем итератор и признак того, что контейнер не модифицировался. Если не найден, то добавляем новый элемент с таким ключом, но пустым значением и фиксируем, что контейнер модифицировался.

В деструкторе смотрим была ли модификация. Если была и операция не завершилась нормально (т.е. флаг commit не выставлен), то откатываем изменения.

Ну и вишенкой на торте, поскольку у нас C++17, правила вывода актуального типа для insertion_it_with_auto_erase_if_not_committed_t. Без этого простой и красивой реализации do_actual_binding не получалось.

---

Чесно говоря, я сам не очень понимаю, как к полученному результату относиться. Меня он, в принципе, устраивает. Если бы удалось сделать реализацию insertion_it_with_auto_erase_if_not_committed_t чуть менее объемной, то было бы вообще замечательно. Но и так сойдет.

Хотя, если бы я работал в команде под чьим-то началом и подобный мой код не прошел бы ревью со стороны моего руководителя, то я бы отнесся к требованию переписать попроще с пониманием. Однако, наверное, какие-то доводы в пользу своего решения привел бы и постарался бы вытянуть объективные контр-доводы. А так же какие-то подсказки о том, как же сделать проще. Ибо пока что не очень представляю как именно.

---

PS. Более простой структурой данных (например, одним словарем с составным ключом) не обойтись, т.к. нужно иметь возможность удалить все подписки для конкретной пары mbox+msink. Если делать ключ из mbox+msink+msg_type, то просто так не получится получить весь диапазон значений для mbox+msink+* (любой msg_type).

Upd. Похоже, на счет единственного словаря с составным ключом я погорячился. Можно сделать что-то вроде:

struct subscription_key_t    {       enum class category          {             lower_bound,             normal,             upper_bound          };       mbox_id_t m_mbox_id;       msink_t m_sink;       category m_category;       std::type_index m_msg_type;       // Constructor for a normal key.       subscription_key_t(          mbox_id_t mbox_id,          msink_t sink,          const std::type_index & msg_type )          :  m_mbox_id{ mbox_id }          ,  m_sink{ std::move(sink) }          ,  m_category{ category::normal }          ,  m_msg_type{ msg_type }          {}       // Constructor for case of lower/upper bounds.       subscription_key_t(          category_t category,          mbox_id_t mbox_id,          msink_t sink )          :  m_mbox_id{ mbox_id }          ,  m_sink{ std::move(sink) }          ,  m_category{ category }          ,  m_msg_type{ typeid(void) }          {}    }; [[nodiscard]] bool operator<( const subscription_key_t & a, const subscription_key_t & b )    {       const auto tie = []( const subscription_key_t & k ) {             return std::tie( k.m_mbox_id, k.m_sink, k.m_category, k.m_msg_type );          };       return tie( a ) < tie( b );    }

Тогда для поиска всех подписок по паре (mbox+msink) можно было бы сделать так:

auto lb = m_bindings.lower_bound(       subscription_key_t{ subscription_key_t::category::lower_bound, mbox_id, msink } ); auto ub = m_bindings.upper_bound(       subscription_key_t{ subscription_key_t::category::upper_bound, mbox_id, msink } );

Кстати говоря, было бы полезно найти время и попробовать провести замеры производительности для варианта с вложенными словарями и варианта с единственным словарем, но со сложным составным ключом.

Upd2. Сделал простой замер производительности двух вариантов решения: с несколькими вложенными std::map-ами и простыми ключами, с единственным std::map-ом и сложным составным ключом. Исходник теста здесь. Результаты получились вот такими:

=== mboxes: 2000, msinks: 100 *** several nested maps *** bind: 3449ms unbind_all: 2815ms bind: 3132ms unbind: 2540ms *** just one map *** bind: 5035ms unbind_all: 2420ms bind: 5052ms unbind: 3626ms

=== mboxes: 200, msinks: 1000 *** several nested maps *** bind: 3249ms unbind_all: 2337ms bind: 3047ms unbind: 1829ms *** just one map *** bind: 6022ms unbind_all: 2761ms bind: 6133ms unbind: 4044ms

=== mboxes: 20000, msinks: 10 *** several nested maps *** bind: 1810ms unbind_all: 1398ms bind: 1656ms unbind: 1528ms *** just one map *** bind: 3153ms unbind_all: 1334ms bind: 3074ms unbind: 2271ms

Т.е., к моему удивлению, несколько вложенных std::map-ов оказываются эффективнее. Удивлен потому, что вставка значений в несколько map-ов -- это несколько динамических аллокаций, а динамические алокации -- это дорого. Но, видимо, операции на большом std::map с "тяжелым" составным ключом оказываются еще дороже.