[pulseaudio-discuss] Patch review status wiki page updated

[David Henningsson]

On 2014-09-22 06:28, Alexander E. Patrakov wrote:
> 22.09.2014 05:20, David Henningsson wrote:
>> On 2014-09-21 12:28, Alexander E. Patrakov wrote:
>>> 21.09.2014 16:19, Tanu Kaskinen wrote:
>>>> Patch review status updated:
>>>> http://www.freedesktop.org/wiki/Software/PulseAudio/PatchStatus/
>>>
>>> "Reduce hardware pointer update syscalls" is not exactly "waiting for
>>> review". I have tested it and found it to fail an assertion when the
>>> sink is autosuspended. Also I did some profiling using perf, but testing
>>> on one card does not give enough justification to reject the patch on
>>> the "no performance improvement" basis. It does look like a valid
>>> cleanup and simplification attempt, though.
>>>
>>> Of course this cannot count as a proper review, but, due to the failed
>>> assertion, the need for a new version of the patch is quite obvious at
>>> this point.
>>
>> I'm currently waiting for Peter to see if he notices any substantial
>> perfomance improvements on his card(s). If he does, it makes sense to
>> submit a new, and more robust, version. If not, maybe it's easier to
>> just drop the idea (no use optimising things that don't take much time
>> anyway).
>
> I think it may still be a valid optimization, but only when combined
> with the removal of the loop that increases j from mmap_write() and
> unix_write(). I.e., currently, there may be several attempts to write
> data, i.e. several syscalls that move the pointer (via writei or
> mmap_commit). In the worst case, by asking Peter to test the version
> that still has the loop about j, you are wasting his time.

The common case is for the loop to run once, which fills the buffer up, 
then go from the top of the loop and break out at "Not filling up, 
because too early.". This is true both with and without the 
optimisation, but even more true with the optimisation because _avail is 
not synchronised with the hardware in the second iteration.

Removing a loop that only runs once anyway is not much of an optimisation.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic