[PATCH 1/2] lib: vsprintf: optimised put_dec() function
mnazarewicz at google.com
Thu Feb 24 17:10:09 EST 2011
On Thu, 24 Feb 2011 22:51:01 +0100, Andrew Morton wrote:
>> Cc: linux-arm at lists.arm.linux.org.uk, Michal Nazarewicz
>> <mina86 at mina86.com>, "Douglas W. Jones" <jones at cs.uiowa.edu>, Denis
>> Vlasenko <vda.linux at googlemail.com
> linux-arm-kernel is for arm patches. linux-kernel at vger.kernel.org
> would be an appropriate cc for patches against lib/vsprintf.c.
Yes, I realised I forgot about LKML just after I submitted the patch.
I included ARM because of the benchmark I did on ARM I mentioned.
> On Tue, 22 Feb 2011 15:26:26 +0100 Michal Nazarewicz wrote:
>> The put_dec() and family of functions were based on a code
>> optimised for processors with 8-bit ALU but since we don't
>> need to limit ourselves to such small ALUs, the code was
>> optimised and used capacities of an 16-bit ALU anyway.
> I don't think this was worth optimising! If any kernel workload is
> calling sprintf("%d") with any frequency then something surely is
>> lib/vsprintf.c | 269
>> 1 files changed, 185 insertions(+), 84 deletions(-)
> Despite the code bloat, the patch somehow reduces the vsprintf.o
> footprint by 150 bytes (1%).
> I'm not really sure that we should apply this. But given that the
> existing code is an incomprehensible over-optimised mess I guess this
> patch doesn't make things much worse.
> However I think that a better approach would be to rip out the existing
> stuff and go for some small, simple and slow thing. Reduce the kernel
> footprint and increase understandability and hence maintainability.
> Unless I'm all wrong and you're aware of a workload which calls these
> things with any frequency?
Oh, no, I just felt like hacking this piece of code one evening. ;)
I don't have any strong feelings about this patch so feel free to
ignore it. I just wanted decision to include it or not to include
it be made by someone with experience rather than be a consequence
of me forgetting about this patch.
>> + r = (q * (uint64_t)0xcccd) >> 19;
>> + *buf++ = (q - 10 * r) + '0';
On Thu, 24 Feb 2011 22:52:42 +0100, Andrew Morton wrote:
> Also, the funky indenting to align on the "=" is atypical for kernel
> code and is inconsistent with the rest of vsprintf.c. Just a single
> space, please.
Want me to resubmit with spaces fixed?
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: mnazarewicz at google.com>-----ooO--(_)--Ooo--
More information about the linux-arm-kernel