vsprintf: uninline simple_strntoull(), reorder arguments
Commit Message
* uninline simple_strntoull(),
gcc overinlines and this function is not performance critical
* reorder arguments, so that appending INT_MAX as 4th argument
generates very efficient tail call
Space savings:
add/remove: 1/0 grow/shrink: 0/3 up/down: 27/-179 (-152)
Function old new delta
simple_strntoll - 27 +27
simple_strtoull 15 10 -5
simple_strtoll 41 7 -34
vsscanf 1930 1790 -140
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
lib/vsprintf.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
Comments
On Fri, Oct 27, 2023 at 05:13:58PM +0300, Alexey Dobriyan wrote:
> * uninline simple_strntoull(),
> gcc overinlines and this function is not performance critical
>
> * reorder arguments, so that appending INT_MAX as 4th argument
> generates very efficient tail call
>
> Space savings:
>
> add/remove: 1/0 grow/shrink: 0/3 up/down: 27/-179 (-152)
> Function old new delta
> simple_strntoll - 27 +27
> simple_strtoull 15 10 -5
> simple_strtoll 41 7 -34
> vsscanf 1930 1790 -140
Makes sense to me
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...
> if (is_sign)
> - val.s = simple_strntoll(str,
> - field_width >= 0 ? field_width : INT_MAX,
> - &next, base);
> + val.s = simple_strntoll(str, &next, base,
> + field_width >= 0 ? field_width : INT_MAX);
> else
> - val.u = simple_strntoull(str,
> - field_width >= 0 ? field_width : INT_MAX,
> - &next, base);
> + val.u = simple_strntoull(str, &next, base,
> + field_width >= 0 ? field_width : INT_MAX);
Looking at these, why do we even care about signedness? field_witdh IIRC is 16-bit or less
and if size_t is to big it's still fine. No?
On Fri 2023-10-27 17:13:58, Alexey Dobriyan wrote:
> * uninline simple_strntoull(),
> gcc overinlines and this function is not performance critical
>
> * reorder arguments, so that appending INT_MAX as 4th argument
> generates very efficient tail call
>
> Space savings:
>
> add/remove: 1/0 grow/shrink: 0/3 up/down: 27/-179 (-152)
> Function old new delta
> simple_strntoll - 27 +27
> simple_strtoull 15 10 -5
> simple_strtoll 41 7 -34
> vsscanf 1930 1790 -140
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
I am usually quite skeptical about these micro-optimizations. They might
help only on some architectures with some compiler versions. And
they might make it worse for others.
Well, I could imagine that passing constant via the last parameter
might help in most cases. And the new ordering of the parameters
is fine. So let's get it in.
Reviewed-by: Petr Mladek <pmladek@suse.com>
I have just pushed the patch into printk/linux.git, branch for-6.7.
Best Regards,
Petr
@@ -60,7 +60,8 @@
bool no_hash_pointers __ro_after_init;
EXPORT_SYMBOL_GPL(no_hash_pointers);
-static noinline unsigned long long simple_strntoull(const char *startp, size_t max_chars, char **endp, unsigned int base)
+noinline
+static unsigned long long simple_strntoull(const char *startp, char **endp, unsigned int base, size_t max_chars)
{
const char *cp;
unsigned long long result = 0ULL;
@@ -95,7 +96,7 @@ static noinline unsigned long long simple_strntoull(const char *startp, size_t m
noinline
unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
{
- return simple_strntoull(cp, INT_MAX, endp, base);
+ return simple_strntoull(cp, endp, base, INT_MAX);
}
EXPORT_SYMBOL(simple_strtoull);
@@ -130,8 +131,8 @@ long simple_strtol(const char *cp, char **endp, unsigned int base)
}
EXPORT_SYMBOL(simple_strtol);
-static long long simple_strntoll(const char *cp, size_t max_chars, char **endp,
- unsigned int base)
+noinline
+static long long simple_strntoll(const char *cp, char **endp, unsigned int base, size_t max_chars)
{
/*
* simple_strntoull() safely handles receiving max_chars==0 in the
@@ -140,9 +141,9 @@ static long long simple_strntoll(const char *cp, size_t max_chars, char **endp,
* and the content of *cp is irrelevant.
*/
if (*cp == '-' && max_chars > 0)
- return -simple_strntoull(cp + 1, max_chars - 1, endp, base);
+ return -simple_strntoull(cp + 1, endp, base, max_chars - 1);
- return simple_strntoull(cp, max_chars, endp, base);
+ return simple_strntoull(cp, endp, base, max_chars);
}
/**
@@ -155,7 +156,7 @@ static long long simple_strntoll(const char *cp, size_t max_chars, char **endp,
*/
long long simple_strtoll(const char *cp, char **endp, unsigned int base)
{
- return simple_strntoll(cp, INT_MAX, endp, base);
+ return simple_strntoll(cp, endp, base, INT_MAX);
}
EXPORT_SYMBOL(simple_strtoll);
@@ -3648,13 +3649,11 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
break;
if (is_sign)
- val.s = simple_strntoll(str,
- field_width >= 0 ? field_width : INT_MAX,
- &next, base);
+ val.s = simple_strntoll(str, &next, base,
+ field_width >= 0 ? field_width : INT_MAX);
else
- val.u = simple_strntoull(str,
- field_width >= 0 ? field_width : INT_MAX,
- &next, base);
+ val.u = simple_strntoull(str, &next, base,
+ field_width >= 0 ? field_width : INT_MAX);
switch (qualifier) {
case 'H': /* that's 'hh' in format */