diff mbox series

bfd: release the PE COMDAT hash once all sections are made

Message ID 20260820192345.935-2-oleg.tolmatcev@gmail.com
State New
Headers
Series bfd: release the PE COMDAT hash once all sections are made |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Oleg Tolmatcev Aug. 20, 2026, 7:23 p.m. UTC 
  styp_to_sec_flags builds pe_data (abfd)->comdat_hash, one entry plus a
strdup'd symbol name per input section.  Nothing frees it during a link,
so it survives to exit.

Free it once the section table has been built.  This is safe because
styp_to_sec_flags recreates the hash whenever it finds it NULL.

This significantly reduces peak memory usage.

bfd/
	* coffgen.c (coff_real_object_p): Free the COMDAT hash after
	the section table has been built.

Signed-off-by: Oleg Tolmatcev <oleg.tolmatcev@gmail.com>
---
 bfd/coffgen.c | 7 +++++++
 1 file changed, 7 insertions(+)

In one of my tests this reduces peak memory usage by 12%: 773 MB out of 6532 MB.

Comments

Jan Beulich Aug. 21, 2026, 11:51 a.m. UTC | #1 
On 20.08.2026 21:23, Oleg Tolmatcev wrote:
> styp_to_sec_flags builds pe_data (abfd)->comdat_hash, one entry plus a
> strdup'd symbol name per input section.  Nothing frees it during a link,
> so it survives to exit.
> 
> Free it once the section table has been built.  This is safe because
> styp_to_sec_flags recreates the hash whenever it finds it NULL.
> 
> This significantly reduces peak memory usage.

This is the positive side. How much extra overhead does recreating the
hash impose? How often might it need recreating in the worst case? Looks
like the answer is zero / never (make_a_section_from_file() is called
solely from coff_real_object_p(), and is the sole caller of
bfd_coff_styp_to_sec_flags_hook()), but your description suggests
otherwise. May I suggest to replace the paragraph with

"Free it once the section table has been built.  This is safe because
 styp_to_sec_flags() would recreate the hash whenever it finds it NULL.
 That's not presently necessary, though."

? If that's okay with you, I can replace the piece of text while
committing.

Jan
Oleg Tolmatcev Aug. 21, 2026, 12:40 p.m. UTC | #2 
пт, 21 авг. 2026 г. в 13:51, Jan Beulich <jbeulich@suse.com>:
>
> On 20.08.2026 21:23, Oleg Tolmatcev wrote:
> > styp_to_sec_flags builds pe_data (abfd)->comdat_hash, one entry plus a
> > strdup'd symbol name per input section.  Nothing frees it during a link,
> > so it survives to exit.
> >
> > Free it once the section table has been built.  This is safe because
> > styp_to_sec_flags recreates the hash whenever it finds it NULL.
> >
> > This significantly reduces peak memory usage.
>
> This is the positive side. How much extra overhead does recreating the
> hash impose? How often might it need recreating in the worst case? Looks
> like the answer is zero / never (make_a_section_from_file() is called
> solely from coff_real_object_p(), and is the sole caller of
> bfd_coff_styp_to_sec_flags_hook()), but your description suggests
> otherwise. May I suggest to replace the paragraph with
>
> "Free it once the section table has been built.  This is safe because
>  styp_to_sec_flags() would recreate the hash whenever it finds it NULL.
>  That's not presently necessary, though."
>
> ? If that's okay with you, I can replace the piece of text while
> committing.
>
> Jan

I am of course okay with that too. Thank you.

Oleg
diff mbox series

Patch

diff --git a/bfd/coffgen.c b/bfd/coffgen.c
index f3060642a5d..92b867bd187 100644
--- a/bfd/coffgen.c
+++ b/bfd/coffgen.c
@@ -368,6 +368,13 @@  coff_real_object_p (bfd *abfd,
     }
 
   _bfd_coff_free_symbols (abfd);
+
+  if (obj_pe (abfd) && pe_data (abfd)->comdat_hash != NULL)
+    {
+      htab_delete (pe_data (abfd)->comdat_hash);
+      pe_data (abfd)->comdat_hash = NULL;
+    }
+
   return coff_object_cleanup;
 
  fail: