diff mbox series

abg-reader: optimize if construction

Message ID 20221016064703.67180-1-hexiaole1994@126.com
State New
Headers
Series abg-reader: optimize if construction |

Commit Message

Xiaole He Oct. 16, 2022, 6:47 a.m. UTC 
  In 'build_enum_type_decl' function of 'src/abg-reader.cc', the
'for loop' walk through all the child nodes of the '<enum-decl>' for
seeking '<underlying-type>' and '<enumerator>':

/* original src/abg-reader.cc begin */
static enum_type_decl_sptr
build_enum_type_decl(read_context& ctxt,
             const xmlNodePtr node,
             bool add_to_current_scope)
{
  ...
  for (xmlNodePtr n = xmlFirstElementChild(node);
       n;
       n = xmlNextElementSibling(n))
    {
      if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
      {
         ...
      }

      if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
      {
         ...
      }
    }
  ...
}
/* original src/abg-reader.cc end */

Here uses 2 separate 'if' statements for seeking, that is, for any
child node of the '<enum-decl>', there involves 2 'if' comparations.
Because the child node of the '<enum-decl>' is either
'<underlying-type>' or '<enumerator>', there would be a slight
optimization when use 'if-else if' construction instead, like below:

/* optimized src/abg-reader.cc begin */
for (xmlNodePtr n = xmlFirstElementChild(node);
     n;
     n = xmlNextElementSibling(n))
  {
    if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
    {
       ...
    }
    else if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
    {
       ...
    }
  }
/* optimized src/abg-reader.cc end */

Supposing there has the test case:

/* test case begin */
<abi-instr version='1.0'>
  <enum-decl name='E' filepath='../../abitests/test-enum0-v0.cc' line='1' column='6' id='type-id-2'>
    <underlying-type type-id='type-id-1'/>
    <enumerator name='e0' value='0'/>
    <enumerator name='e2' value='1'/>
  </enum-decl>
</abi-instr>
/* test case end */

When parsing the '<underlying-type>' xml tag, for the original
'src/abg-reader.cc', there involves 2 'if' comparations. But involves
only 1 'if' comparation for the optimized 'src/abg-reader.cc'.

Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
Tested-by: Xiaole He <hexiaole@kylinos.cn>
---
 src/abg-reader.cc | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Dodji Seketeli Oct. 17, 2022, 2:14 p.m. UTC | #1 
Hello Xiaole,

Xiaole He via Libabigail <libabigail@sourceware.org> a écrit:

> In 'build_enum_type_decl' function of 'src/abg-reader.cc', the
> 'for loop' walk through all the child nodes of the '<enum-decl>' for
> seeking '<underlying-type>' and '<enumerator>':
>
> /* original src/abg-reader.cc begin */
> static enum_type_decl_sptr
> build_enum_type_decl(read_context& ctxt,
>              const xmlNodePtr node,
>              bool add_to_current_scope)
> {
>   ...
>   for (xmlNodePtr n = xmlFirstElementChild(node);
>        n;
>        n = xmlNextElementSibling(n))
>     {
>       if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
>       {
>          ...
>       }
>
>       if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
>       {
>          ...
>       }
>     }
>   ...
> }
> /* original src/abg-reader.cc end */
>
> Here uses 2 separate 'if' statements for seeking, that is, for any
> child node of the '<enum-decl>', there involves 2 'if' comparations.
> Because the child node of the '<enum-decl>' is either
> '<underlying-type>' or '<enumerator>', there would be a slight
> optimization when use 'if-else if' construction instead, like below:
>
> /* optimized src/abg-reader.cc begin */
> for (xmlNodePtr n = xmlFirstElementChild(node);
>      n;
>      n = xmlNextElementSibling(n))
>   {
>     if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
>     {
>        ...
>     }
>     else if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
>     {
>        ...
>     }
>   }
> /* optimized src/abg-reader.cc end */
>
> Supposing there has the test case:
>
> /* test case begin */
> <abi-instr version='1.0'>
>   <enum-decl name='E' filepath='../../abitests/test-enum0-v0.cc' line='1' column='6' id='type-id-2'>
>     <underlying-type type-id='type-id-1'/>
>     <enumerator name='e0' value='0'/>
>     <enumerator name='e2' value='1'/>
>   </enum-decl>
> </abi-instr>
> /* test case end */
>
> When parsing the '<underlying-type>' xml tag, for the original
> 'src/abg-reader.cc', there involves 2 'if' comparations. But involves
> only 1 'if' comparation for the optimized 'src/abg-reader.cc'.
>
> Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
> Tested-by: Xiaole He <hexiaole@kylinos.cn>

Applied to the master branch.

Cheers,
Xiaole He Oct. 18, 2022, 3:25 a.m. UTC | #2 
Thank you for reviewing.

















At 2022-10-17 22:14:35, "Dodji Seketeli" <dodji@seketeli.org> wrote:
>Hello Xiaole,
>
>Xiaole He via Libabigail <libabigail@sourceware.org> a écrit:
>
>> In 'build_enum_type_decl' function of 'src/abg-reader.cc', the
>> 'for loop' walk through all the child nodes of the '<enum-decl>' for
>> seeking '<underlying-type>' and '<enumerator>':
>>
>> /* original src/abg-reader.cc begin */
>> static enum_type_decl_sptr
>> build_enum_type_decl(read_context& ctxt,
>>              const xmlNodePtr node,
>>              bool add_to_current_scope)
>> {
>>   ...
>>   for (xmlNodePtr n = xmlFirstElementChild(node);
>>        n;
>>        n = xmlNextElementSibling(n))
>>     {
>>       if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
>>       {
>>          ...
>>       }
>>
>>       if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
>>       {
>>          ...
>>       }
>>     }
>>   ...
>> }
>> /* original src/abg-reader.cc end */
>>
>> Here uses 2 separate 'if' statements for seeking, that is, for any
>> child node of the '<enum-decl>', there involves 2 'if' comparations.
>> Because the child node of the '<enum-decl>' is either
>> '<underlying-type>' or '<enumerator>', there would be a slight
>> optimization when use 'if-else if' construction instead, like below:
>>
>> /* optimized src/abg-reader.cc begin */
>> for (xmlNodePtr n = xmlFirstElementChild(node);
>>      n;
>>      n = xmlNextElementSibling(n))
>>   {
>>     if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
>>     {
>>        ...
>>     }
>>     else if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
>>     {
>>        ...
>>     }
>>   }
>> /* optimized src/abg-reader.cc end */
>>
>> Supposing there has the test case:
>>
>> /* test case begin */
>> <abi-instr version='1.0'>
>>   <enum-decl name='E' filepath='../../abitests/test-enum0-v0.cc' line='1' column='6' id='type-id-2'>
>>     <underlying-type type-id='type-id-1'/>
>>     <enumerator name='e0' value='0'/>
>>     <enumerator name='e2' value='1'/>
>>   </enum-decl>
>> </abi-instr>
>> /* test case end */
>>
>> When parsing the '<underlying-type>' xml tag, for the original
>> 'src/abg-reader.cc', there involves 2 'if' comparations. But involves
>> only 1 'if' comparation for the optimized 'src/abg-reader.cc'.
>>
>> Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
>> Tested-by: Xiaole He <hexiaole@kylinos.cn>
>
>Applied to the master branch.
>
>Cheers,
>
>-- 
>		Dodji
diff mbox series

Patch

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index b55808b9..4afa427a 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -4602,8 +4602,7 @@  build_enum_type_decl(read_context&	ctxt,
 	    base_type_id = CHAR_STR(a);
 	  continue;
 	}
-
-      if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
+      else if (xmlStrEqual(n->name, BAD_CAST("enumerator")))
 	{
 	  string name;
 	  int64_t value = 0;