From f533dd40ce98c987585757b2572f622ffb332da3 Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Fri, 28 May 2021 14:11:50 +0200
Subject: [PATCH] Make the in place std::variant<> extension smarter

With the suggestions to use `std::get_if<>` and to only do this for
nontrivial types from
https://github.com/fraillt/bitsery/issues/76#issuecomment-850371533.
---
 src/common/bitsery/ext/in-place-variant.h | 28 +++++++++++++----------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/common/bitsery/ext/in-place-variant.h b/src/common/bitsery/ext/in-place-variant.h
index 5157aa26..f903873c 100644
--- a/src/common/bitsery/ext/in-place-variant.h
+++ b/src/common/bitsery/ext/in-place-variant.h
@@ -40,24 +40,28 @@ class InPlaceVariant : public StdVariant<Overloads...> {
             des.adapter(), index, sizeof...(Ts),
             std::integral_constant<bool, Des::TConfig::CheckDataErrors>{});
 
+        // Most of this is copied directly from the original implementation.
+        // We just added the check here to reuse the existing object if
+        // possible.
         this->execIndex(index, obj, [this, &des](auto& data, auto index) {
             constexpr size_t Index = decltype(index)::value;
             using TElem =
                 typename std::variant_alternative<Index,
                                                   std::variant<Ts...>>::type;
-
-            // Most of this is copied directly from the original implementation.
-            // We just added the check here to reuse the existing object if
-            // possible.
-            if (std::holds_alternative<TElem>(data)) {
-                TElem& item = std::get<TElem>(data);
-                this->serializeType(des, item);
-            } else {
-                TElem item = ::bitsery::Access::create<TElem>();
-                this->serializeType(des, item);
-                data = std::variant<Ts...>(std::in_place_index_t<Index>{},
-                                           std::move(item));
+            // Reinitializing nontrivial types may be expensive especially when
+            // they reference heap data, so if `data` is already holding the
+            // requested variant then we'll deserialize into the existing object
+            if constexpr (std::is_trivial_v<TElem>) {
+                if (auto item = std::get_if<TElem>(&data)) {
+                    this->serializeType(des, *item);
+                    return;
+                }
             }
+
+            TElem item = ::bitsery::Access::create<TElem>();
+            this->serializeType(des, item);
+            data = std::variant<Ts...>(std::in_place_index_t<Index>{},
+                                       std::move(item));
         });
     }
 };