[committed] libstdc++: Remove try/catch overhead in std::variant::emplace

Jonathan Wakely jwakely@redhat.com
Fri Oct 15 17:29:41 GMT 2021


The __variant_construct_by_index helper function sets the new index
before constructing the new object. This means that if the construction
throws then the exception needs to be caught, so the index can be reset
to variant_npos, and then the exception rethrown. This means callers are
responsible for restoring the variant's invariants and they need the
overhead of a catch handler and a rethrow.

If we don't set the index until after construction completes then the
invariant is never broken, and callers can ignore the exception and let
it propagate. The callers all call _M_reset() first, which sets index to
variant_npos as required while the variant is valueless.

We need to be slightly careful here, because changing the order of
operations in __variant_construct_by_index and removing the try-block
from variant::emplace<I> changes an implicit ABI contract between those
two functions. If the linker were to create an executable containing an
instantiation of the old __variant_construct_by_index and an
instantiation of the new variant::emplace<I> code then we would have a
combination that breaks the invariant and doesn't have the exception
handling to restore it. To avoid this problem, we can rename the
__variant_construct_by_index function so that the new emplace<I> code
calls a new symbol, and is unaffected by the behaviour of the old
symbol.

libstdc++-v3/ChangeLog:

	* include/std/variant (__detail::__variant::__get_storage):
	Remove unused function.
	(__variant_construct_by_index): Set index after construction is
	complete. Rename to ...
	(__detail::__variant::__construct_by_index): ... this.
	(variant): Use new name for __variant_construct_by_index friend
	declaration. Remove __get_storage friend declaration.
	(variant::emplace): Use new name and remove try-blocks.

Tested powerpc64le-linux. Committed to trunk.

-------------- next part --------------
commit e27771e5dcd8cf2cb757db6177a3485acd28b88f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Oct 15 10:58:56 2021

    libstdc++: Remove try/catch overhead in std::variant::emplace
    
    The __variant_construct_by_index helper function sets the new index
    before constructing the new object. This means that if the construction
    throws then the exception needs to be caught, so the index can be reset
    to variant_npos, and then the exception rethrown. This means callers are
    responsible for restoring the variant's invariants and they need the
    overhead of a catch handler and a rethrow.
    
    If we don't set the index until after construction completes then the
    invariant is never broken, and callers can ignore the exception and let
    it propagate. The callers all call _M_reset() first, which sets index to
    variant_npos as required while the variant is valueless.
    
    We need to be slightly careful here, because changing the order of
    operations in __variant_construct_by_index and removing the try-block
    from variant::emplace<I> changes an implicit ABI contract between those
    two functions. If the linker were to create an executable containing an
    instantiation of the old __variant_construct_by_index and an
    instantiation of the new variant::emplace<I> code then we would have a
    combination that breaks the invariant and doesn't have the exception
    handling to restore it. To avoid this problem, we can rename the
    __variant_construct_by_index function so that the new emplace<I> code
    calls a new symbol, and is unaffected by the behaviour of the old
    symbol.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/variant (__detail::__variant::__get_storage):
            Remove unused function.
            (__variant_construct_by_index): Set index after construction is
            complete. Rename to ...
            (__detail::__variant::__construct_by_index): ... this.
            (variant): Use new name for __variant_construct_by_index friend
            declaration. Remove __get_storage friend declaration.
            (variant::emplace): Use new name and remove try-blocks.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 4a6826b7ba6..f49094130ee 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1094,19 +1094,21 @@ namespace __variant
 	>;
     }
 
-} // namespace __variant
-} // namespace __detail
-
   template<size_t _Np, typename _Variant, typename... _Args>
-    void __variant_construct_by_index(_Variant& __v, _Args&&... __args)
+    inline void
+    __construct_by_index(_Variant& __v, _Args&&... __args)
     {
-      __v._M_index = _Np;
       auto&& __storage = __detail::__variant::__get<_Np>(__v);
       ::new ((void*)std::addressof(__storage))
         remove_reference_t<decltype(__storage)>
 	  (std::forward<_Args>(__args)...);
+      // Construction didn't throw, so can set the new index now:
+      __v._M_index = _Np;
     }
 
+} // namespace __variant
+} // namespace __detail
+
   template<typename _Tp, typename... _Types>
     constexpr bool
     holds_alternative(const variant<_Types...>& __v) noexcept
@@ -1342,8 +1344,9 @@ namespace __variant
       template <typename... _UTypes, typename _Tp>
 	friend decltype(auto) __variant_cast(_Tp&&);
       template<size_t _Np, typename _Variant, typename... _Args>
-	friend void __variant_construct_by_index(_Variant& __v,
-						 _Args&&... __args);
+	friend void
+	__detail::__variant::__construct_by_index(_Variant& __v,
+						  _Args&&... __args);
 
       static_assert(sizeof...(_Types) > 0,
 		    "variant must have at least one alternative");
@@ -1507,12 +1510,13 @@ namespace __variant
 	  static_assert(_Np < sizeof...(_Types),
 			"The index must be in [0, number of alternatives)");
 	  using type = variant_alternative_t<_Np, variant>;
+	  namespace __variant = std::__detail::__variant;
 	  // Provide the strong exception-safety guarantee when possible,
 	  // to avoid becoming valueless.
 	  if constexpr (is_nothrow_constructible_v<type, _Args...>)
 	    {
 	      this->_M_reset();
-	      __variant_construct_by_index<_Np>(*this,
+	      __variant::__construct_by_index<_Np>(*this,
 		  std::forward<_Args>(__args)...);
 	    }
 	  else if constexpr (is_scalar_v<type>)
@@ -1521,9 +1525,9 @@ namespace __variant
 	      const type __tmp(std::forward<_Args>(__args)...);
 	      // But these steps won't throw:
 	      this->_M_reset();
-	      __variant_construct_by_index<_Np>(*this, __tmp);
+	      __variant::__construct_by_index<_Np>(*this, __tmp);
 	    }
-	  else if constexpr (__detail::__variant::_Never_valueless_alt<type>()
+	  else if constexpr (__variant::_Never_valueless_alt<type>()
 	      && _Traits::_S_move_assign)
 	    {
 	      // This construction might throw:
@@ -1537,17 +1541,8 @@ namespace __variant
 	      // This case only provides the basic exception-safety guarantee,
 	      // i.e. the variant can become valueless.
 	      this->_M_reset();
-	      __try
-		{
-		  __variant_construct_by_index<_Np>(*this,
-		    std::forward<_Args>(__args)...);
-		}
-	      __catch (...)
-		{
-		  using __index_type = decltype(this->_M_index);
-		  this->_M_index = static_cast<__index_type>(variant_npos);
-		  __throw_exception_again;
-		}
+	      __variant::__construct_by_index<_Np>(*this,
+		std::forward<_Args>(__args)...);
 	    }
 	  return std::get<_Np>(*this);
 	}
@@ -1561,6 +1556,7 @@ namespace __variant
 	  static_assert(_Np < sizeof...(_Types),
 			"The index must be in [0, number of alternatives)");
 	  using type = variant_alternative_t<_Np, variant>;
+	  namespace __variant = std::__detail::__variant;
 	  // Provide the strong exception-safety guarantee when possible,
 	  // to avoid becoming valueless.
 	  if constexpr (is_nothrow_constructible_v<type,
@@ -1568,10 +1564,10 @@ namespace __variant
 						   _Args...>)
 	    {
 	      this->_M_reset();
-	      __variant_construct_by_index<_Np>(*this, __il,
+	      __variant::__construct_by_index<_Np>(*this, __il,
 		  std::forward<_Args>(__args)...);
 	    }
-	  else if constexpr (__detail::__variant::_Never_valueless_alt<type>()
+	  else if constexpr (__variant::_Never_valueless_alt<type>()
 	      && _Traits::_S_move_assign)
 	    {
 	      // This construction might throw:
@@ -1585,17 +1581,8 @@ namespace __variant
 	      // This case only provides the basic exception-safety guarantee,
 	      // i.e. the variant can become valueless.
 	      this->_M_reset();
-	      __try
-		{
-		  __variant_construct_by_index<_Np>(*this, __il,
-		    std::forward<_Args>(__args)...);
-		}
-	      __catch (...)
-		{
-		  using __index_type = decltype(this->_M_index);
-		  this->_M_index = static_cast<__index_type>(variant_npos);
-		  __throw_exception_again;
-		}
+	      __variant::__construct_by_index<_Np>(*this, __il,
+		std::forward<_Args>(__args)...);
 	    }
 	  return std::get<_Np>(*this);
 	}


More information about the Libstdc++ mailing list