RFC: WIP: Framework for recording stats for linker phases

Message ID 87bju7c10l.fsf@redhat.com
State New
Headers
Series RFC: WIP: Framework for recording stats for linker phases |

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

Nick Clifton March 11, 2025, 10:30 a.m. UTC
  Hi Guys,

  The attached patch is a work in progress that adds the ability to
  record resource usage statistics for various phases of the linker's
  operation.

  The feature is enabled when the --stats command line option is used
  but the data is only emitted if the --Map option is also used.  I
  choose this method as it keeps things simple, instead of adding even
  more command line options.

  I am not sure what phases should be recorded at the moment.  I would
  like to be able to drill down to things like reloc processing and
  string merging, but I am also concerned about how to handle recording
  data when multiple phases are active at the same time.

  Anyway I am posting the code here in case anyone is interested and
  wants to have a look.  If you have any comments or suggestions please
  do let me know.

Cheers
  Nick
  

Patch

diff --git a/ld/config.in b/ld/config.in
index 2d7b6406d2b..e10c9e73cc6 100644
--- a/ld/config.in
+++ b/ld/config.in
@@ -122,6 +122,9 @@ 
 /* Define to 1 if you have the `getpagesize' function. */
 #undef HAVE_GETPAGESIZE
 
+/* Define to 1 if you have the `getrusage' function. */
+#undef HAVE_GETRUSAGE
+
 /* Define if the GNU gettext() function is already present or preinstalled. */
 #undef HAVE_GETTEXT
 
diff --git a/ld/configure b/ld/configure
index b7af25d1e5f..3f745ac883e 100755
--- a/ld/configure
+++ b/ld/configure
@@ -18753,7 +18753,7 @@  fi
 
 done
 
-for ac_func in close glob lseek mkstemp open realpath waitpid
+for ac_func in close getrusage glob lseek mkstemp open realpath waitpid
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/ld/configure.ac b/ld/configure.ac
index 228f2ee4089..1ee0c0c77f9 100644
--- a/ld/configure.ac
+++ b/ld/configure.ac
@@ -414,7 +414,7 @@  AC_SUBST(NATIVE_LIB_DIRS)
 AC_CHECK_HEADERS(fcntl.h elf-hints.h limits.h inttypes.h stdint.h \
 		 sys/file.h sys/mman.h sys/param.h sys/stat.h sys/time.h \
 		 sys/types.h unistd.h)
-AC_CHECK_FUNCS(close glob lseek mkstemp open realpath waitpid)
+AC_CHECK_FUNCS(close getrusage glob lseek mkstemp open realpath waitpid)
 
 BFD_BINARY_FOPEN
 
diff --git a/ld/ld.h b/ld/ld.h
index 254f0a097bb..0cb4d4982c3 100644
--- a/ld/ld.h
+++ b/ld/ld.h
@@ -330,6 +330,21 @@  typedef struct
   enum compressed_debug_section_type compress_debug;
 } ld_config_type;
 
+typedef enum
+{
+  /* The phase names are in alphabetical order.  */
+  PHASE_PARSE = 0,
+  PHASE_PLUGINS,
+  PHASE_PROCESS,
+  PHASE_WRITE,
+
+  NUM_PHASES
+}
+ld_phase;
+
+extern void ld_start_phase (ld_phase);
+extern void ld_stop_phase (ld_phase);
+
 extern ld_config_type config;
 
 extern FILE * saved_script_handle;
diff --git a/ld/ld.texi b/ld/ld.texi
index 6d51ccbe572..0bcb1af4553 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -2708,7 +2708,10 @@  many relocations.  @var{count} defaults to a value of 32768.
 @kindex --stats
 @item --stats
 Compute and display statistics about the operation of the linker, such
-as execution time and memory usage.
+as execution time and memory usage.  If the @option{--Map} option has
+been enabled then this information is also recorded in the map file,
+along with extra data about the resources used by different phases of
+the link process.
 
 @kindex --sysroot=@var{directory}
 @item --sysroot=@var{directory}
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 54a834e42a6..cdb67c302aa 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -51,6 +51,10 @@ 
 
 #include <string.h>
 
+#if defined (HAVE_GETRUSAGE)
+#include <sys/resource.h>
+#endif
+
 #ifndef TARGET_SYSTEM_ROOT
 #define TARGET_SYSTEM_ROOT ""
 #endif
@@ -224,7 +228,9 @@  ld_cleanup (void)
       bfd_close_all_done (ibfd);
     }
 #if BFD_SUPPORTS_PLUGINS
+  ld_start_phase (PHASE_PLUGINS);
   plugin_call_cleanup ();
+  ld_stop_phase (PHASE_PLUGINS);
 #endif
   if (output_filename && delete_output_file_on_failure)
     unlink_if_ordinary (output_filename);
@@ -270,6 +276,99 @@  display_external_script (void)
   free (buf);
 }
 
+struct ld_phase_data
+{
+  const char *    name;
+  bool            started;
+
+  unsigned long   start;
+  unsigned long   duration;
+
+#if defined (HAVE_GETRUSAGE)
+  struct rusage   begin;
+  struct rusage   use;
+#endif
+};
+
+static struct ld_phase_data phase_data [NUM_PHASES] =
+{
+  [PHASE_PARSE]   = { .name = "parse" },
+  [PHASE_PLUGINS] = { .name = "plugins" },
+  [PHASE_PROCESS] = { .name = "process" },
+  [PHASE_WRITE]   = { .name = "write" },
+};
+
+void
+ld_start_phase (ld_phase phase)
+{
+  struct ld_phase_data * pd = phase_data + phase;
+
+  if (!config.stats)
+    return;
+
+  /* Do not overwrite the fields if we have already started recording.  */
+  if (pd->started)
+    return;
+
+  /* FIXME: It is OK if another phase is also active at this point.
+     It just means that this phase is a sub-phase of that phase.
+     However if this phase does not stop before the containing phase
+     then we have phase overlap, which is bad for the consistency of
+     the recorded data.  So ideally we should either not allow sub-phases
+     at all, or else record all phases that are active here and make
+     sure that they stop in reverse order.  */
+
+  pd->started = true;
+  pd->start = get_run_time ();
+
+#if defined (HAVE_GETRUSAGE)
+  struct rusage usage;
+
+  if (getrusage (RUSAGE_SELF, & usage) != 0)
+    /* FIXME: Complain ?  */
+    return;
+  
+  memcpy (& pd->begin, & usage, sizeof usage);
+#endif
+}
+
+void
+ld_stop_phase (ld_phase phase)
+{
+  struct ld_phase_data * pd = phase_data + phase;
+
+  if (!config.stats)
+    return;
+
+  if (!pd->started)
+    /* FIXME: What should we do ?  */
+    return;
+
+  pd->duration += get_run_time () - pd->start;
+  pd->started = false;
+
+  /* FIXME: Should we check to see if any other phase has STARTED set to TRUE ?
+     If so, then we have a phase overlap, which is bad for recording incremental data.  */
+
+#if defined (HAVE_GETRUSAGE)
+  struct rusage usage;
+
+  if (getrusage (RUSAGE_SELF, & usage) != 0)
+    /* FIXME: Complain ?  */
+    return;
+
+  /* We record any field that has increased.  */
+#define RECORD_INCREASE(field)						\
+  if (pd->begin.ru_##field < usage.ru_##field)				\
+    pd->use.ru_##field += usage.ru_##field - pd->begin.ru_##field ;
+
+  /* FIXME: Which fields should we record ?  */
+  RECORD_INCREASE (maxrss);
+  RECORD_INCREASE (ixrss);
+  RECORD_INCREASE (isrss);
+#endif
+}
+
 int
 main (int argc, char **argv)
 {
@@ -286,6 +385,8 @@  main (int argc, char **argv)
   program_name = argv[0];
   xmalloc_set_program_name (program_name);
 
+  ld_start_phase (PHASE_PARSE);
+  
   expandargv (&argc, &argv);
 
   if (bfd_init () != BFD_INIT_MAGIC)
@@ -404,11 +505,17 @@  main (int argc, char **argv)
   if (config.hash_table_size != 0)
     bfd_hash_set_default_size (config.hash_table_size);
 
+  ld_stop_phase (PHASE_PARSE);
+  
 #if BFD_SUPPORTS_PLUGINS
+  ld_start_phase (PHASE_PLUGINS);
   /* Now all the plugin arguments have been gathered, we can load them.  */
   plugin_load_plugins ();
+  ld_stop_phase (PHASE_PLUGINS);
 #endif /* BFD_SUPPORTS_PLUGINS */
 
+  ld_start_phase (PHASE_PARSE);
+
   ldemul_set_symbols ();
 
   /* If we have not already opened and parsed a linker script,
@@ -531,8 +638,13 @@  main (int argc, char **argv)
       link_info.has_map_file = true;
     }
 
+  ld_stop_phase (PHASE_PARSE);
+  ld_start_phase (PHASE_PROCESS);
+
   lang_process ();
 
+  ld_stop_phase (PHASE_PROCESS);
+
   /* Print error messages for any missing symbols, for any warning
      symbols, and possibly multiple definitions.  */
   if (bfd_link_relocatable (&link_info))
@@ -558,7 +670,11 @@  main (int argc, char **argv)
   link_info.output_bfd->flags
     |= flags & bfd_applicable_file_flags (link_info.output_bfd);
 
+
+  ld_start_phase (PHASE_WRITE);
   ldwrite ();
+  ld_stop_phase (PHASE_WRITE);
+
 
   if (config.map_file != NULL)
     lang_map ();
@@ -661,6 +777,32 @@  main (int argc, char **argv)
       fprintf (stderr, _("%s: total time in link: %ld.%06ld\n"),
 	       program_name, run_time / 1000000, run_time % 1000000);
       fflush (stderr);
+
+      if (config.map_file != NULL)
+	{
+	  minfo (_("\nStats: total time in link: %ld (microseconds)\n"), run_time);
+
+#if defined (HAVE_GETRUSAGE)
+	  struct rusage usage;
+
+	  if (getrusage (RUSAGE_SELF, & usage) == 0)
+	    {
+	      minfo ("Stats: system cpu time: %ld (seconds)\n", usage.ru_stime.tv_sec);
+	      minfo ("Stats: max resident set size: %ld (kilobytes)\n", usage.ru_maxrss);
+	    }
+#endif
+	  int i;
+
+	  for (i = 0; i < NUM_PHASES; i++)
+	    {
+	      minfo ("Stats: phase: %s: cpu time:   %ld (microseconds)\n", phase_data[i].name, phase_data[i].duration);
+#if defined (HAVE_GETRUSAGE)
+	      minfo ("Stats: phase: %s: memory:     %ld (kilobytes)\n", phase_data[i].name, phase_data[i].use.ru_maxrss);
+	      minfo ("Stats: phase: %s: shared mem: %ld (kilobytes)\n", phase_data[i].name, phase_data[i].use.ru_ixrss);
+	      minfo ("Stats: phase: %s: stack:      %ld (kilobytes)\n", phase_data[i].name, phase_data[i].use.ru_isrss);
+#endif
+	    }
+	}
     }
 
   /* Prevent ld_cleanup from deleting the output file.  */
@@ -942,8 +1084,11 @@  add_archive_element (struct bfd_link_info *info,
       && (!no_more_claiming
 	  || bfd_get_lto_type (abfd) != lto_fat_ir_object))
     {
+      ld_start_phase (PHASE_PLUGINS);
       /* We must offer this archive member to the plugins to claim.  */
       plugin_maybe_claim (input);
+      ld_stop_phase (PHASE_PLUGINS);
+
       if (input->flags.claimed)
 	{
 	  if (no_more_claiming)