Re: [Qemu-devel] [PATCH v3 4/5] qtest: precompute hex nibs

[John Snow]

On 05/07/2015 02:13 AM, Markus Armbruster wrote:
> John Snow <address@hidden> writes:
> 
>> On 05/06/2015 11:19 AM, Markus Armbruster wrote:
>>> John Snow <address@hidden> writes:
>>>
>>>> On 05/06/2015 02:25 AM, Markus Armbruster wrote:
>>>>> John Snow <address@hidden> writes:
>>>>>
>>>>>> Instead of letting printf and friends do this for us
>>>>>> one byte at a time, fill a buffer ourselves and then
>>>>>> send the entire buffer in one go.
>>>>>>
>>>>>> This gives a moderate speed improvement over the old
>>>>>> method.
>>>>>
>>>>> Out of curiosity: how much of the improvement is due to doing our own
>>>>> buffering instead of printf()'s (assuming the stream is buffered), and
>>>>> how much is due to doing our own hex formatting instead of printf()'s?
>>>>>
>>>>
>>>> Out of ignorance: How would I measure?
>>>
>>> Heh, well played!
>>>
>>> The code before the series uses chr unbuffered:
>>>
>>>         for (i = 0; i < len; i++) {
>>>             qtest_send(chr, "%02x", data[i]);
>>>         }
>>>
>>> qtest_send() formats into two bytes, passes them to
>>> qemu_chr_fe_write_all(), which writes them to chr.
>>>
>>> The chr are typically unbuffered, so this could well produce a series of
>>> two-byte write() system calls.
>>>
>>> Adding some buffering will obviously make a difference for larger len.
>>>
>>> Whether formatting hex digits by hands can make a difference is not
>>> obvious.
>>>
>>> To find out, add just buffering.  Something like this in your patch
>>> instead of byte2hex():
>>>
>>>          for (i = 0; i < len; i++) {
>>> -            qtest_sendf(chr, "%02x", data[i]);
>>> +            snprintf(&enc[i * 2], 2, "%02x", data[i]);
>>>          }
>>>
>>> If the speedup is pretty much entirely due to buffering (which I
>>> suspect), then your commit message could use a bit of love :)
>>>
>>
>> When you're right, you're right. The difference may not be statistically
>> meaningful, but with today's current planetary alignment, using
>> sprintf() to batch the sends instead of my home-rolled nib computation
>> function, I can eke out a few more tenths of a second.
> 
> :)
> 
>> If there are no objections, I will stage patches 1-3 and 5, and resubmit
>> a quick v2 of just this single patch, unless you want to go ahead and
>> say that making the edit will be fine, then I will just edit it before
>> sending the pullreq.
> 
> Recommend to post the revised patch, with an updated commit message.
> 

Staged patches and dropped patch 4, thanks.

--js