diff mbox series

RISC-V/objdump: Do not re-seacrh for mapping symbols when didn't find any.

Message ID 20240906115629.3587596-1-dmitry.bushev@syntacore.com
State New
Headers
Series RISC-V/objdump: Do not re-seacrh for mapping symbols when didn't find any. |

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

Dmitry Bushev Sept. 6, 2024, 11:56 a.m. UTC 
  From: Dmitry Bushev <dmitry.bushev@syntacore.com>

In object files without any mapping symbols it's better to not
search up to start of a section upon each instruction entry. Instead,
cache symbol number from which search was started and if no symbol found,
next time do not search past that symbol.

Signed-off-by: Dmitry Bushev <dmitry.bushev@syntacore.com>
---
 opcodes/riscv-dis.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Dmitry Bushev Sept. 13, 2024, 2:52 p.m. UTC | #1 
Hi,

I found a performance issue with mapping symbols in objdump. It is observable when disassembling large sections (10MB of code and 100k+ symbols) that does not contain any mapping symbols - it takes progressively more time to dump such sections than it takes alternative objdump implementations (like llvm-objdump). I suggest a simple fix for that.

Could you, please, review my patch?

With best regards,
Dmitry
Nelson Chu Sept. 24, 2024, 5:10 a.m. UTC | #2 
Hi,

On Fri, Sep 6, 2024 at 7:58 PM <dmitry.bushev@syntacore.com> wrote:

> From: Dmitry Bushev <dmitry.bushev@syntacore.com>
>
> In object files without any mapping symbols it's better to not
> search up to start of a section upon each instruction entry. Instead,
> cache symbol number from which search was started and if no symbol found,
> next time do not search past that symbol.
>
> Signed-off-by: Dmitry Bushev <dmitry.bushev@syntacore.com>
> ---
>  opcodes/riscv-dis.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 8ab138e45ce..c754e1d9460 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -69,6 +69,7 @@ struct riscv_private_data
>  /* Used for mapping symbols.  */
>  static int last_map_symbol = -1;
>  static bfd_vma last_stop_offset = 0;
> +static int no_mapping_symbols_before = 0;
>

Considering compatibility,
static int last_map_symbol = -1;
+static int no_map_symbol_before = 0;
static bfd_vma last_stop_offset = 0;
...


>    /* If the last stop offset is different from the current one, then
>       don't use the last_map_symbol to search.  We usually reset the
> @@ -1172,8 +1176,9 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
>    if (!found)
>      {
>        n = from_last_map_symbol ? last_map_symbol : info->symtab_pos;
> +      int start_n = n;
>

If we cannot find any map_symbol for this section, then
`from_last_map_symbol' will always be false, so `n' should always be
`info->symtab_pos', so ...


> -      for (; n >= 0; n--)
> +      for (; n >= no_mapping_symbols_before; n--)
>         {
>           bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
>           /* We have searched all possible symbols in the range.  */
> @@ -1187,6 +1192,10 @@ riscv_search_mapping_symbol (bfd_vma memaddr,
>               break;
>             }
>         }
> +      /* Remember that we already scanned all symbols before start_n and
> +       * didn't find any. Next time, do not scan those symbols again. */
> +      if (!found)
> +       no_mapping_symbols_before = start_n;
>

moved this to ...


>      }
>
>    if (found)
>


if (found)
{
..
}
else
  no_map_symbol_before = info->symtab_pos;


Thanks
Nelson
diff mbox series

Patch

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 8ab138e45ce..c754e1d9460 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -69,6 +69,7 @@  struct riscv_private_data
 /* Used for mapping symbols.  */
 static int last_map_symbol = -1;
 static bfd_vma last_stop_offset = 0;
+static int no_mapping_symbols_before = 0;
 static bfd_vma last_map_symbol_boundary = 0;
 static enum riscv_seg_mstate last_map_state = MAP_NONE;
 static asection *last_map_section = NULL;
@@ -1136,7 +1137,10 @@  riscv_search_mapping_symbol (bfd_vma memaddr,
 
   /* Reset the last_map_symbol if we start to dump a new section.  */
   if (memaddr <= 0)
-    last_map_symbol = -1;
+    {
+      last_map_symbol = -1;
+      no_mapping_symbols_before = 0;
+    }
 
   /* If the last stop offset is different from the current one, then
      don't use the last_map_symbol to search.  We usually reset the
@@ -1172,8 +1176,9 @@  riscv_search_mapping_symbol (bfd_vma memaddr,
   if (!found)
     {
       n = from_last_map_symbol ? last_map_symbol : info->symtab_pos;
+      int start_n = n;
 
-      for (; n >= 0; n--)
+      for (; n >= no_mapping_symbols_before; n--)
 	{
 	  bfd_vma addr = bfd_asymbol_value (info->symtab[n]);
 	  /* We have searched all possible symbols in the range.  */
@@ -1187,6 +1192,10 @@  riscv_search_mapping_symbol (bfd_vma memaddr,
 	      break;
 	    }
 	}
+      /* Remember that we already scanned all symbols before start_n and
+       * didn't find any. Next time, do not scan those symbols again. */
+      if (!found)
+	no_mapping_symbols_before = start_n;
     }
 
   if (found)