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
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to