On 5/19/26 7:02 AM, Tao Liu wrote:
Hi Rui,
Thanks for your work on the performance improvement.
Frankly I doubt the rate of improvement, could you please provide a
time consumption measurement with contrast, so we can understand how
much benefit your patch will introduce? I think strcmp() is a common c
library that already done the optimization as much as possible.
Thanks,
Tao Liu
Hi Tao,
Thank you for the suggestion and for questioning the improvement claim.
I went back and did a direct end-to-end measurement for this patch on
the following workflow:
1. start ./crash /proc/kcore
2. run sym -l
3. quit crash
I compared commit df23d5b7c9e0 against its parent commit on the same
machine and kernel environment, using /proc/kcore as the dumpfile and
driving the interaction through a pseudo-terminal to stay close to
actual crash console usage.
Based on the measurements I got, this patch did not show a meaningful
improvement for that workflow. On my test setup, the patched version was
actually slightly slower overall, so I agree that the performance
benefit is not sufficiently justified here.
Given that result, I think it is better for me to withdraw this patch
rather than keep the claim without solid evidence.
That said, I would appreciate it if you could take a closer look at the
previous patch in the series:
crash: symbols: Optimize symbol string matching overhead in
numeric_forward()
That patch targets a more focused hot path, and in my testing it did
show measurable improvement, so I believe it is the one more worth
considering on its own.
Thanks again for the careful review and for asking for the measurement data.
Best regards,
Rui
> On Tue, May 19, 2026 at 5:27 AM Rui Qi <qirui.001(a)bytedance.com> wrote:
>>
>> In a modern Linux environment with numerous kernel modules loaded, the
>> crash utility must process and sort tens of thousands (often 50,000+) of
>> module symbols. The symbol sorting function (compare_syms) and module
>> loading loops frequently check for pseudo-symbols using macros like
>> MODULE_PSEUDO_SYMBOL(), MODULE_START(), and others.
>>
>> These macros heavily rely on the STREQ()/STRNEQ() macros, which
>> internally expand to string_exists() checks followed by a full strcmp()
>> or strncmp(). Because these macros are evaluated millions of times during
>> module initialization and sorting (O(N log N) complexity), the function
>> call overhead of strcmp() becomes a noticeable performance bottleneck.
>>
>> By explicitly checking the first character of the symbol name
>> (e.g., (sp)->name[0] == '_') before invoking STRNEQ(), we introduce a
>> lightweight "early reject" mechanism. Since most regular module
symbols
>> do not start with an underscore, this short-circuits the evaluation for
>> the vast majority of symbols, completely avoiding the overhead of
>> strncmp() macro expansion.
>>
>> Additionally, since the symbol name could potentially be NULL, we safely
>> guard the character access with an explicit non-null check
>> ((sp)->name &&) to prevent segmentation faults.
>>
>> This patch applies the early reject optimization to:
>> 1. All MODULE_* pseudo-symbol macros.
>> 2. The compare_syms() function for "__insmod" and
"_MODULE_START_" checks.
>>
>> This cumulative optimization yields a measurable performance improvement
>> in module symbol sorting and initialization speed.
>>
>> Signed-off-by: Rui Qi <qirui.001(a)bytedance.com>
>> ---
>> symbols.c | 20 ++++++++++----------
>> 1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/symbols.c b/symbols.c
>> index 736d9d96d606..9b0de45e9a95 100644
>> --- a/symbols.c
>> +++ b/symbols.c
>> @@ -1314,14 +1314,14 @@ symname_hash_search(struct syment *table[], char *name)
>> * Output for sym -[lL] command.
>> */
>>
>> -#define MODULE_PSEUDO_SYMBOL(sp) (STRNEQ((sp)->name,
"_MODULE_"))
>> +#define MODULE_PSEUDO_SYMBOL(sp) ((sp)->name &&
(sp)->name[0] == '_' && STRNEQ((sp)->name, "_MODULE_"))
>>
>> -#define MODULE_START(sp) (STRNEQ((sp)->name, "_MODULE_START_"))
>> -#define MODULE_END(sp) (STRNEQ((sp)->name, "_MODULE_END_"))
>> -#define MODULE_INIT_START(sp) (STRNEQ((sp)->name,
"_MODULE_INIT_START_"))
>> -#define MODULE_INIT_END(sp) (STRNEQ((sp)->name,
"_MODULE_INIT_END_"))
>> -#define MODULE_SECTION_START(sp) (STRNEQ((sp)->name,
"_MODULE_SECTION_START"))
>> -#define MODULE_SECTION_END(sp) (STRNEQ((sp)->name,
"_MODULE_SECTION_END"))
>> +#define MODULE_START(sp) ((sp)->name && (sp)->name[0] ==
'_' && STRNEQ((sp)->name, "_MODULE_START_"))
>> +#define MODULE_END(sp) ((sp)->name && (sp)->name[0] ==
'_' && STRNEQ((sp)->name, "_MODULE_END_"))
>> +#define MODULE_INIT_START(sp) ((sp)->name && (sp)->name[0] ==
'_' && STRNEQ((sp)->name, "_MODULE_INIT_START_"))
>> +#define MODULE_INIT_END(sp) ((sp)->name && (sp)->name[0] ==
'_' && STRNEQ((sp)->name, "_MODULE_INIT_END_"))
>> +#define MODULE_SECTION_START(sp) ((sp)->name && (sp)->name[0] ==
'_' && STRNEQ((sp)->name, "_MODULE_SECTION_START"))
>> +#define MODULE_SECTION_END(sp) ((sp)->name && (sp)->name[0] ==
'_' && STRNEQ((sp)->name, "_MODULE_SECTION_END"))
>>
>> #define MODULE_MEM_START(sp,t) (STRNEQ((sp)->name, module_tag[t].start))
>> #define MODULE_MEM_END(sp,t) (STRNEQ((sp)->name, module_tag[t].end))
>> @@ -3419,12 +3419,12 @@ compare_syms(const void *v1, const void *v2)
>> s2 = (struct syment *)v2;
>>
>> if (s1->value == s2->value) {
>> - if (STRNEQ(s1->name, "__insmod"))
>> + if (s1->name && s1->name[0] == '_'
&& STRNEQ(s1->name, "__insmod"))
>> return -1;
>> - if (STRNEQ(s2->name, "__insmod"))
>> + if (s2->name && s2->name[0] == '_'
&& STRNEQ(s2->name, "__insmod"))
>> return 1;
>> if (MODULE_MEM_START(s2, MOD_TEXT) ||
>> - STRNEQ(s2->name, "_MODULE_START_"))
>> + s2->name && s2->name[0] == '_'
&& STRNEQ(s2->name, "_MODULE_START_"))
>> return 1;
>> /* Get pseudo section name. */
>> if (MODULE_SECTION_START(s1))
>> --
>> 2.20.1
>> --
>> Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io
>> To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io
>> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
>> Contribution Guidelines:
https://github.com/crash-utility/crash/wiki
>>
>