public inbox for glibc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "nicolas at freedelity dot be" <sourceware-bugzilla@sourceware.org>
To: glibc-bugs@sourceware.org
Subject: [Bug malloc/30579] trim_threshold in realloc lead to high memory usage
Date: Wed, 28 Jun 2023 07:56:15 +0000	[thread overview]
Message-ID: <bug-30579-131-rkjvSCi2Dh@http.sourceware.org/bugzilla/> (raw)
In-Reply-To: <bug-30579-131@http.sourceware.org/bugzilla/>

https://sourceware.org/bugzilla/show_bug.cgi?id=30579

--- Comment #3 from Nicolas Dusart <nicolas at freedelity dot be> ---
I'm concerned that this issue may have been considered resolved for you as it
may seem that it originated from a misunderstanding of my part.
But I have to stress out that this change may not be appropriate. It has
significantly impacted some of our processes which were not usable anymore and
a lot of debugging were necessary to spot the root cause.
I suspect it could similarly affect many other project using glibc. It may take
some time before production environment use new version of glibc, so I'd expect
these concern to pop out in a few month or years.
I believe we could both benefit from further discussion on this matter, and I
would greatly appreciate gaining insight into the reasoning behind this change.

With that in mind, I'd like to respectfully re-express my concerns about the
changes observed in the recent behavior of realloc:

 - First, as I've mentioned before, it appears that the new behavior will tend
to accumulate small blocks of less than `M_TRIM_THRESHOLD` bytes inside the
heap. Total size of all these blocks may largely exceed `M_TRIM_THRESHOLD`
bytes.

 - Secondly, this new behavior of realloc seems to be misaligned with the
documentation of M_TRIM_THRESHOLD. According to the documentation, the
"M_TRIM_THRESHOLD parameter specifies the minimum size (in bytes) that this
block of memory [at the top of the heap] must reach before sbrk is used to trim
the heap."
The current behavior, however, doesn't seem to be adhering to this definition.

 - I acknowledge that mallopt might be a solution to adapt the behavior
throughout the code based on specific needs. However, it seems to be a feasible
solution only in a single-threaded process. In a multi-threaded process, some
threads might benefit from a large M_TRIM_THRESHOLD while others might need a
smaller one. Unfortunately, mallopt doesn't seem to provide a solution in this
context.

These points bring me to the argument that this shift in behavior can be
considered a breaking change as it might have a significant impact on existing
processes, particularly those that rely on the current documentation of your
library.
Previously, we could consider that at maximum of `M_TRIM_THRESHOLD` unused
bytes could be reserved indefinitely to our process even if we tried to
deallocate this memory. That was a limit that can be controlled.
With the new behavior, the amount of unused memory reserved for our process is
unbounded. Furthermore, the unused slots cannot be reused by our process. This
implies that this space is lost, not just to the rest of the system, but also
to the process itself until the next free() call is made. In some processes,
this newly realloced space is meant to be kept indefinitely.

Could you provide some insight into the reasons why these arguments might not
be considered strong enough to avoid such a breaking change?

Even if the decision is to stick with the new behavior, it would seem necessary
to update the documentation to reflect these changes accurately. This would
ensure that developers are made aware of this new behavior and can take
necessary steps to mitigate any impact on their processes.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

  parent reply	other threads:[~2023-06-28  7:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-22 13:54 [Bug malloc/30579] New: " nicolas at freedelity dot be
2023-06-22 14:20 ` [Bug malloc/30579] " siddhesh at sourceware dot org
2023-06-22 15:10 ` nicolas at freedelity dot be
2023-06-28  7:56 ` nicolas at freedelity dot be [this message]
2023-06-28 16:31 ` siddhesh at sourceware dot org
2023-07-06 15:38 ` cvs-commit at gcc dot gnu.org
2023-07-06 15:40 ` cvs-commit at gcc dot gnu.org
2023-07-06 15:42 ` siddhesh at sourceware dot org
2023-07-06 16:09 ` nicolas at freedelity dot be

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-30579-131-rkjvSCi2Dh@http.sourceware.org/bugzilla/ \
    --to=sourceware-bugzilla@sourceware.org \
    --cc=glibc-bugs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).