[webkit-reviews] review denied: [Bug 101187] Reduce the memory usage of ConvolverNode : [Attachment 172275] Patch

[bugzilla-daemon at webkit.org]

Chris Rogers <crogers at google.com> has denied Xingnan Wang
<xingnan.wang at intel.com>'s request for review:
Bug 101187: Reduce the memory usage of ConvolverNode
https://bugs.webkit.org/show_bug.cgi?id=101187

Attachment 172275: Patch
https://bugs.webkit.org/attachment.cgi?id=172275&action=review

------- Additional Comments from Chris Rogers <crogers at google.com>
Xingnan, thanks for looking into reducing memory usage for ConvolverNode.  I
think there's a potentially much better way to achieve even much higher memory
savings and not impact performance.  Even if your tests show little performance
hit with the interleaving/deinterleaving, there are a range of processor
architectures and chip models that this runs on.  We'd have to test them all
very carefully.  Plus we don't want to take *any* performance hit at all.

Here's my idea:
FFTConvolver has a "m_frame" member variable which could be shared among all
similar FFTConvolver objects of the same size (and which are running on the
same thread).  The process() method could be changed to take an additional
argument which is the FFTFrame (shared).  It could even be made more fancy
where this additional argument is optional, and the FFTConvolver would lazily
create its own "m_frame" if one wasn't provided.

As an example, you can see where FFTConvolver is used in HRTFPanner which has
four of them.  Not only could a single FFTFrame be shared for all four, but it
could also be shared among other HRTFPanner objects running in that same
thread.

Similarly, for ConvolverNode, all the ReverbConvolverStage's of the same size
(and running on the same thread) can be given a shared FFTFrame.

The memory savings this way could be very significant and would help ALL the
different FFT back-ends, instead of just the FFmpeg one.