Skip to content

Comments

fix: fix CountStats performance#626

Merged
boyter merged 1 commit intoboyter:masterfrom
apocelipes:fix-regression
Jun 23, 2025
Merged

fix: fix CountStats performance#626
boyter merged 1 commit intoboyter:masterfrom
apocelipes:fix-regression

Conversation

@apocelipes
Copy link
Contributor

The compiler is still not smart, branch-less code in CountStats casused a performance regression. I'll revert this part.

Other parts won't impact the speed.

Before:
slow

Fixes:
fix

After the revert the performance issue is fixed and we even become a little bit faster.

The compiler reports that these functions are inlined (I also detected the binary file, printXXX functions actually inlined and they were not in the symbol table), and theoretically there should be no performance difference. This is so wierd, so I left some comments on it.

@boytertesting boytertesting bot added M/complexity Normal or medium complexity M/size Normal or medium sized change labels Jun 23, 2025
@boyter boyter merged commit ef7c27c into boyter:master Jun 23, 2025
4 checks passed
@boyter
Copy link
Owner

boyter commented Jun 23, 2025

Yeah annoying.

@apocelipes
Copy link
Contributor Author

apocelipes commented Jun 24, 2025

Yeah annoying.

@boyter Finally, I found what caused this. Even if the functions are inlined, their arguments are evaluated, so that we need to pay time to convert arguments to interface any. Converting to any is slow but if we use outside if-statements to skip the whole block, the cost will not be paied. The Golang compiler currently has only very limited optimizations, and it doesn't seem to be able to handle the above situation.

P.S. Even though converting arguments to any is slow, printXXXF(“%s %d”, s1, i1) is still much faster than printXXX(fmt.Sprintf(“%s %d”, s1, i1)).

@apocelipes apocelipes deleted the fix-regression branch June 24, 2025 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

M/complexity Normal or medium complexity M/size Normal or medium sized change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants