Use of C++ dynamic global object initialization with thread guards

[Kim Barrett]

> On Dec 4, 2023, at 2:28 AM, Florian Weimer <fweimer at redhat.com> wrote:
> 
> As far as I understand it, the Hotspot C++ style guide advises not use
> C++ run-time library features.

Largely true.

>  However, it seems that the use of
> dynamic initialization guards (that involve calls to __cxa_guard_acquire
> and __cxa_guard_release) has increased quite a bit over the years.

Also true.  We weren’t supposed to be using that at all before adopting C++11/14,
which defined function scoped static initialization to be thread safe.  But now that
we have that feature, we’re allowed to use it, and it’s sometimes/often better than
hand-rolled alternatives, at least as far as source code simplicity and readability.

> The implementation of __cxa_guard_acquire is not entirely trivial
> because it detects recursive initialization and throws
> __gnu_cxx::recursive_init_error, which means that it pulls in the C++
> unwinder (at least with a traditional GNU/Linux build of libstdc++.a).

Does it?  Seems like it shouldn’t.  We build with -fno-exceptions, and the definition
of throw_recursive_init_exception is conditionalized on __cpp_exceptions, only
throwing when that macro is defined.  It calls __builtin_trap() if that macro isn’t
defined.

And that’s the sort of thing I expected to find.  For something from the runtime library
to throw when under the influence of -fno-exceptions seems like a bug.

> Furthermore, most uses of C++ dynamic initialization involve a
> computation that is idempotent and have unused bit patterns in the
> initialized value.  This means that a separate guard variable is not
> needed, and a simple atomic store/atomic load could be used.

That’s the kind of complexity we’d really rather avoid if possible.

> In other cases, the use of global objects seems unnecessary.  For
> example, src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp
> has a dynamically initialized static variable max_elem_size:
> 
> [… code snipped …]
> The min_element_size() member function is inline and just returns
> _min_element_size, which is declared const, so it cannot change over
> time.  This means that caching that value is pointless, and the static
> should probably removed.

I agree this seems like an inappropriate use of static.  It only works here because
the manager object being referenced is a singleton.

> Would it make sense to minimize the use of __cxa_guard_acquire and
> __cxa_guard_release?  There are currently 400 such calls, but many of
> them appear in templated code, so I could get it down to ~80 calls with
> about a days of work.

I don’t think so, since I currently don’t think the described problem (pulling in
exception unwinding) actually exists.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Message signed with OpenPGP
URL: <https://mail.openjdk.org/pipermail/hotspot-dev/attachments/20231206/7fbdd8e5/signature-0001.asc>