Re: [PATCH] libstdc++: Allow emergency EH alloc pool size to be tuned [PR68606]

[Jonathan Wakely <jwakely@redhat.com>]

On Mon, 10 Oct 2022 at 07:18, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Fri, Oct 7, 2022 at 5:55 PM Jonathan Wakely via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > This needs a little more documentation (see the TODO in the manual),
> > rather than just the comments in the source. This isn't final, but I
> > think it's the direction I want to take.
> >
> > -- >8 --
> >
> > Implement a long-standing request to support tuning the size of the
> > emergency buffer for allocating exceptions after malloc fails, or to
> > disable that buffer entirely.
> >
> > It's now possible to disable the dynamic allocation of the buffer and
> > use a fixed-size static buffer, via --enable-libstdcxx-static-eh-pool.
> > This is a built-time choice that is baked into libstdc++ and so affects
> > all code linked against that build of libstdc++.
> >
> > The size of the pool can be set by --with-libstdcxx-eh-pool-obj-count=N
> > which is measured in units of sizeof(void*) not bytes. A given exception
> > type such as std::system_error depends on the target, so giving a size
> > in bytes wouldn't be portable across 16/32/64-bit targets.
> >
> > When libstdc++ is configured to use a dynamic buffer, the size of that
> > buffer can now be tuned at runtime by setting the GLIBCXX_TUNABLES
> > environment variable (c.f. PR libstdc++/88264). The number of exceptions
> > to reserve space for is controlled by the "glibcxx.eh_pool.obj_count"
> > and "glibcxx.eh_pool.obj_size" tunables. The pool will be sized to be
> > able to allocate obj_count exceptions of size obj_size*sizeof(void*) and
> > obj_count "dependent" exceptions rethrown by std::rethrow_exception.
> >
> > With the ability to tune the buffer size, we can reduce the default pool
> > size. Most users never need to throw 1kB exceptions in parallel from
> > hundreds of threads after malloc is OOM.
>
> But does it hurt?  Back in time when I reworked the allocator to be less
> wasteful the whole point was to allow more exceptions to be in-flight
> during OOM shutdown of a process with many threads.

It certainly hurts for small systems, but maybe we can keep the large
allocation for 64-bit targets (currently 73kB) and only reduce it for
32-bit (19kB) and 16-bit (3kB IIRC) targets.

And obviously if the new code to check an env var is backported, the
defaults shouldn't be changed in the release branch. Just enable the
new feature, but leave defaults the same.

N.B. the C++ ABI actually requires that the emergency pool should
block if too many threads try to access it at once. That would mean
the program slows down drastically, but doesn't empty the pool and
terminate if there are many threads all trying to throw on OOM. I'm
not convinced blocking is the right default, but making it an option
seems reasonable (I created PR107180 to track that).


> So if we reduce the default buffer size that should be documented
> in changes.html, maybe with a hint how to restore the old buffer size
> (configury flags required or runtime ENV setting)?

The git commit message gives the env setting to do that, and
acinclude.m4 gives the configure option to do that. I can certainly
add the info to changes.html where it's more likely to be noticed.


>
> Otherwise looks OK to me.

Great, thanks for taking a look.