diff mbox series

bfd: stop over-allocating aux entries for COFF section symbols

Message ID 20260820192914.1514-2-oleg.tolmatcev@gmail.com
State New
Headers
Series bfd: stop over-allocating aux entries for COFF section symbols |

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:29 p.m. UTC 
  coff_new_section_hook allocated ten combined_entry_type slots for every
section symbol, behind a comment conceding that the ten was a guess and
should not be a constant.  Nothing in BFD sets n_numaux above 1 on a
section symbol, and nothing indexes the array past native[1], so eight
of the ten were never touched on any target.

This runs once per input section, so it significantly reduces peak
memory usage.

bfd/
	* coffcode.h (coff_new_section_hook): Allocate one syment plus
	one aux entry rather than ten.

Signed-off-by: Oleg Tolmatcev <oleg.tolmatcev@gmail.com>
---
 bfd/coffcode.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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

Comments

Jan Beulich Aug. 21, 2026, noon UTC | #1 
On 20.08.2026 21:29, Oleg Tolmatcev wrote:
> coff_new_section_hook allocated ten combined_entry_type slots for every
> section symbol, behind a comment conceding that the ten was a guess and
> should not be a constant.  Nothing in BFD sets n_numaux above 1 on a
> section symbol, and nothing indexes the array past native[1], so eight
> of the ten were never touched on any target.
> 
> This runs once per input section, so it significantly reduces peak
> memory usage.
> 
> bfd/
> 	* coffcode.h (coff_new_section_hook): Allocate one syment plus
> 	one aux entry rather than ten.
> 
> Signed-off-by: Oleg Tolmatcev <oleg.tolmatcev@gmail.com>

Fundamentally okay, one comment though:

> --- a/bfd/coffcode.h
> +++ b/bfd/coffcode.h
> @@ -1831,9 +1831,9 @@ coff_new_section_hook (bfd * abfd, asection * section)
>    /* Allocate aux records for section symbols, to store size and
>       related info.
>  
> -     @@ The 10 is a guess at a plausible maximum number of aux entries
> -     (but shouldn't be a constant).  */
> -  amt = sizeof (combined_entry_type) * 10;
> +     One syment plus one aux: nothing sets n_numaux above 1 on a
> +     section symbol, or indexes this array past native[1].  */
> +  amt = sizeof (combined_entry_type) * 2;
>    native = (combined_entry_type *) bfd_zalloc (abfd, amt);
>    if (native == NULL)
>      return false;

sizeof(<type>) is generally at risk of going out of sync with the type
of the variable that is really meant to be used. Since you're already
touching that line, may I suggest to switch to 

  amt = sizeof (*native) * 2;

? Again - if that's okay with you, I can replace the piece of code while
committing.

Jan
Oleg Tolmatcev Aug. 21, 2026, 12:39 p.m. UTC | #2 
пт, 21 авг. 2026 г. в 14:00, Jan Beulich <jbeulich@suse.com>:
>
> On 20.08.2026 21:29, Oleg Tolmatcev wrote:
> > coff_new_section_hook allocated ten combined_entry_type slots for every
> > section symbol, behind a comment conceding that the ten was a guess and
> > should not be a constant.  Nothing in BFD sets n_numaux above 1 on a
> > section symbol, and nothing indexes the array past native[1], so eight
> > of the ten were never touched on any target.
> >
> > This runs once per input section, so it significantly reduces peak
> > memory usage.
> >
> > bfd/
> >       * coffcode.h (coff_new_section_hook): Allocate one syment plus
> >       one aux entry rather than ten.
> >
> > Signed-off-by: Oleg Tolmatcev <oleg.tolmatcev@gmail.com>
>
> Fundamentally okay, one comment though:
>
> > --- a/bfd/coffcode.h
> > +++ b/bfd/coffcode.h
> > @@ -1831,9 +1831,9 @@ coff_new_section_hook (bfd * abfd, asection * section)
> >    /* Allocate aux records for section symbols, to store size and
> >       related info.
> >
> > -     @@ The 10 is a guess at a plausible maximum number of aux entries
> > -     (but shouldn't be a constant).  */
> > -  amt = sizeof (combined_entry_type) * 10;
> > +     One syment plus one aux: nothing sets n_numaux above 1 on a
> > +     section symbol, or indexes this array past native[1].  */
> > +  amt = sizeof (combined_entry_type) * 2;
> >    native = (combined_entry_type *) bfd_zalloc (abfd, amt);
> >    if (native == NULL)
> >      return false;
>
> sizeof(<type>) is generally at risk of going out of sync with the type
> of the variable that is really meant to be used. Since you're already
> touching that line, may I suggest to switch to
>
>   amt = sizeof (*native) * 2;
>
> ? Again - if that's okay with you, I can replace the piece of code while
> committing.
>
> Jan

I am of course okay with it. Thank you.

Oleg
diff mbox series

Patch

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index 964d2c3e173..6875f856c08 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -1831,9 +1831,9 @@  coff_new_section_hook (bfd * abfd, asection * section)
   /* Allocate aux records for section symbols, to store size and
      related info.
 
-     @@ The 10 is a guess at a plausible maximum number of aux entries
-     (but shouldn't be a constant).  */
-  amt = sizeof (combined_entry_type) * 10;
+     One syment plus one aux: nothing sets n_numaux above 1 on a
+     section symbol, or indexes this array past native[1].  */
+  amt = sizeof (combined_entry_type) * 2;
   native = (combined_entry_type *) bfd_zalloc (abfd, amt);
   if (native == NULL)
     return false;