| Message ID | 6aa18081-cde2-48f8-b426-5127e32b1bec@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [_Hashtable] Optimize destructor | expand |
On Mon, 10 Jun 2024 at 05:38, François Dumont <frs.dumont@gmail.com> wrote: > > Hi > > libstdc++: [_Hashtable] Optimize destructor > > Hashtable destructor do not need to call clear() method that in addition to > destroying all nodes also reset all buckets to nullptr. > > libstdc++-v3/ChangeLog: > > * include/bits/hashtable.h (~_Hashtable()): Replace clear call with > a _M_deallocate_nodes call. > > Tested under Linux x64, ok to commit ? This will only matter at -O0 because the compiler will optimize out the memset and zeroing in the destructor, because those are dead stores. The call to memset in the destructor is incorrect anyway, that would be undefined behaviour for fancy pointers of non-trivial type. OK for trunk.
On Mon, 10 Jun 2024 at 09:52, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Mon, 10 Jun 2024 at 05:38, François Dumont <frs.dumont@gmail.com> wrote: > > > > Hi > > > > libstdc++: [_Hashtable] Optimize destructor > > > > Hashtable destructor do not need to call clear() method that in addition to > > destroying all nodes also reset all buckets to nullptr. > > > > libstdc++-v3/ChangeLog: > > > > * include/bits/hashtable.h (~_Hashtable()): Replace clear call with > > a _M_deallocate_nodes call. > > > > Tested under Linux x64, ok to commit ? > > This will only matter at -O0 because the compiler will optimize out > the memset and zeroing in the destructor, because those are dead > stores. > > The call to memset in the destructor is incorrect anyway, that would > be undefined behaviour for fancy pointers of non-trivial type. It should be something like this, which the compiler can still optimize for raw pointers: --- a/libstdc++-v3/include/bits/hashtable.h +++ b/libstdc++-v3/include/bits/hashtable.h @@ -2582,8 +2582,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION clear() noexcept { this->_M_deallocate_nodes(_M_begin()); - __builtin_memset(_M_buckets, 0, - _M_bucket_count * sizeof(__node_base_ptr)); + for (size_t __i = 0; __i < _M_bucket_count; ++__i) + _M_buckets[__i] = nullptr; _M_element_count = 0; _M_before_begin._M_nxt = nullptr; } > > OK for trunk.
diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h index e8e51714d72..45b232111da 100644 --- a/libstdc++-v3/include/bits/hashtable.h +++ b/libstdc++-v3/include/bits/hashtable.h @@ -1666,7 +1666,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION "Cache the hash code or qualify your functors involved" " in hash code and bucket index computation with noexcept"); - clear(); + this->_M_deallocate_nodes(_M_begin()); _M_deallocate_buckets(); }