* [PATCH] abg-reader: optimize if construction
@ 2022-10-16 6:47 Xiaole He
2022-10-17 14:14 ` Dodji Seketeli
0 siblings, 1 reply; 3+ messages in thread
From: Xiaole He @ 2022-10-16 6:47 UTC (permalink / raw)
To: libabigail; +Cc: Xiaole He, Xiaole He
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(-)
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;
--
2.27.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] abg-reader: optimize if construction
2022-10-16 6:47 [PATCH] abg-reader: optimize if construction Xiaole He
@ 2022-10-17 14:14 ` Dodji Seketeli
2022-10-18 3:25 ` Xiaole He
0 siblings, 1 reply; 3+ messages in thread
From: Dodji Seketeli @ 2022-10-17 14:14 UTC (permalink / raw)
To: Xiaole He via Libabigail; +Cc: Xiaole He, Xiaole He
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re:Re: [PATCH] abg-reader: optimize if construction
2022-10-17 14:14 ` Dodji Seketeli
@ 2022-10-18 3:25 ` Xiaole He
0 siblings, 0 replies; 3+ messages in thread
From: Xiaole He @ 2022-10-18 3:25 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Xiaole He via Libabigail, Xiaole He
[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-18 3:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-16 6:47 [PATCH] abg-reader: optimize if construction Xiaole He
2022-10-17 14:14 ` Dodji Seketeli
2022-10-18 3:25 ` Xiaole He
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).