1205540 - skip processing of inactive audio nodes

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Karl Tomlinson (:karlt)]

For bug 1189562 we need to detect which streams have no input and so can be suspended.
These inactive streams must continue to produce output until the end of the MSG iteration but the output is known to be zero and so processing is simple.

In this bug, the streams are detected and the zero processing will be optimized.
The suspending for bug 1189562 is not covered by this bug.

Comment 1

[Karl Tomlinson (:karlt)]

MSG thread processing time from perf top -t <tid> on http://webaudiodemos.appspot.com/MIDIDrums/index.html initially is distributed mostly in number crunching:

    12.00%  liblgpllibs.so  [.] rdft_calc_c                                   
     5.45%  libxul.so       [.] WebCore::DirectConvolver::process             
     5.17%  libxul.so       [.] moz_speex_interpolate_product_single          
     4.89%  liblgpllibs.so  [.] ff_fft_permute_sse.loop                       
     4.50%  libxul.so       [.] resampler_basic_interpolate_single            
     4.42%  libxul.so       [.] mozilla::AudioBufferAddWithScale              
     4.34%  libxul.so       [.] mozilla::BufferComplexMultiply                
     4.33%  libxul.so       [.] mozilla::FFTBlock::GetInverseWithoutScaling   
     4.07%  liblgpllibs.so  [.] pass_sse.loop                                 
     3.86%  libxul.so       [.] WebCore::FFTConvolver::process                
     3.18%  libxul.so       [.] cubic_coef                                    
     2.93%  liblgpllibs.so  [.] fft16_sse                                     
     2.48%  libxul.so       [.] mozilla::DelayBuffer::ReadChannels            
     2.46%  liblgpllibs.so  [.] pass_interleave_sse.loop                      
     2.14%  libxul.so       [.] mozilla::FFTBlock::PerformFFT                 
     1.99%  libm-2.20.so    [.] __powf_finite                                 
     1.42%  liblgpllibs.so  [.] fft8_sse                                      
     1.22%  libm-2.20.so    [.] __logf_finite                                 
     1.21%  libxul.so       [.] WebCore::ReverbConvolverStage::process        
     1.09%  libxul.so       [.] mozilla::StreamBuffer::FindTrack              
     1.08%  libxul.so       [.] WebCore::DynamicsCompressorKernel::process    
     0.98%  firefox         [.] je_arena_dalloc_junk_small                    
     0.93%  libxul.so       [.] WebCore::ZeroPole::process                    
     0.92%  liblgpllibs.so  [.] ff_fft_permute_sse.loopcopy                   
     0.76%  libxul.so       [.] mozilla::AudioNodeStream::AdvanceOutputSegment

As inactive nodes sit waiting for GC things the distribution changes to be dominated by overhead:

    10.48%  libxul.so       [.] mozilla::StreamBuffer::FindTrack                
     8.38%  libxul.so       [.] mozilla::AudioNodeStream::ObtainInputBlock      
     7.52%  libxul.so       [.] mozilla::MediaStreamGraphImpl::ProduceDataForStr
     7.08%  libxul.so       [.] mozilla::AudioNodeStream::AdvanceOutputSegment  
     5.52%  libxul.so       [.] mozilla::AudioNodeStream::ProcessInput          
     5.42%  libxul.so       [.] mozilla::MediaSegmentBase<mozilla::AudioSegment,
     3.41%  libxul.so       [.] mozilla::AudioNodeStream::ComputedNumberOfChanne
     3.34%  libxul.so       [.] mozilla::MediaStreamGraphImpl::AddBlockingRelate
     3.12%  libxul.so       [.] mozilla::AudioBlock::SetBuffer                  
     2.90%  liblgpllibs.so  [.] rdft_calc_c                                     
     1.66%  libxul.so       [.] mozilla::TimeVarying<long, bool, 5u>::GetAt     
     1.64%  liblgpllibs.so  [.] ff_fft_permute_sse.loop                         
     1.50%  libxul.so       [.] WebCore::FFTConvolver::process                  
     1.44%  libxul.so       [.] mozilla::AudioBufferAddWithScale                
     1.40%  libxul.so       [.] WebCore::DirectConvolver::process               
     1.34%  libxul.so       [.] mozilla::FFTBlock::GetInverseWithoutScaling     
     1.30%  libxul.so       [.] mozilla::MediaStreamGraphImpl::UpdateStreamOrder
     1.28%  libxul.so       [.] mozilla::dom::PannerNodeEngine::ProcessBlock    
     1.26%  libxul.so       [.] mozilla::BufferComplexMultiply                  
     1.24%  libxul.so       [.] moz_speex_interpolate_product_single            
     1.13%  libxul.so       [.] resampler_basic_interpolate_single              
     1.12%  libxul.so       [.] mozilla::StreamBuffer::ForgetUpTo               
     1.05%  liblgpllibs.so  [.] pass_sse.loop                                   
     1.00%  libxul.so       [.] mozilla::MediaStream::EnsureTrack               
     0.84%  libxul.so       [.] cubic_coef                                      

Patches that I'll attach here to skip ProcessBlock and ObtainInputBlock on inactive nodes change the distribution to:

    14.77%  libxul.so       [.] mozilla::StreamBuffer::FindTrack                
     9.53%  libxul.so       [.] mozilla::MediaStreamGraphImpl::ProduceDataForStr
     7.79%  libxul.so       [.] mozilla::AudioNodeStream::AdvanceOutputSegment  
     6.73%  libxul.so       [.] mozilla::AudioNodeStream::ProcessInput          
     5.93%  libxul.so       [.] mozilla::MediaSegmentBase<mozilla::AudioSegment,
     3.75%  libxul.so       [.] mozilla::AudioNodeStream::ObtainInputBlock      
     3.05%  liblgpllibs.so  [.] rdft_calc_c                                     
     2.81%  libxul.so       [.] mozilla::MediaStreamGraphImpl::AddBlockingRelate
     2.21%  libxul.so       [.] mozilla::AudioBlock::SetBuffer                  
     2.06%  libxul.so       [.] mozilla::TimeVarying<long, bool, 5u>::GetAt     
     1.62%  liblgpllibs.so  [.] ff_fft_permute_sse.loop                         
     1.51%  libxul.so       [.] mozilla::MediaStreamGraphImpl::UpdateStreamOrder
     1.49%  libxul.so       [.] mozilla::AudioBufferAddWithScale                
     1.45%  libxul.so       [.] WebCore::DirectConvolver::process               
     1.42%  libxul.so       [.] WebCore::FFTConvolver::process                  
     1.38%  libxul.so       [.] mozilla::MediaStreamGraphImpl::RecomputeBlocking
     1.25%  libxul.so       [.] mozilla::StreamBuffer::ForgetUpTo               
     1.24%  libxul.so       [.] moz_speex_interpolate_product_single            
     1.23%  libxul.so       [.] mozilla::BufferComplexMultiply                  
     1.15%  liblgpllibs.so  [.] pass_sse.loop                                   
     1.15%  libxul.so       [.] mozilla::FFTBlock::GetInverseWithoutScaling     
     1.13%  libxul.so       [.] resampler_basic_interpolate_single              
     0.88%  libxul.so       [.] mozilla::MediaStreamGraphImpl::StreamSet::iterat
     0.83%  libxul.so       [.] cubic_coef                                      
     0.80%  libxul.so       [.] mozilla::MediaStreamGraphImpl::NotifyHasCurrentD

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1205540 don't send more null chunks than necessary to AnalyserNode r?padenot

bug 1205540 don't send more null chunks than necessary to AnalyserNode r?padenot

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1205540 provide querying whether engines need to continue processing even without input r?padenot

bug 1205540 provide querying whether engines need to continue processing even without input r?padenot

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1205540 make source stream available during RemoveInput r?padenot

bug 1205540 make source stream available during RemoveInput r?padenot

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1205540 mark BufferSource finished only when producing silent output block r?padenot

bug 1205540 account for active inputs and skip processing when streams are inactive r?padenot

Comment 6

[Karl Tomlinson (:karlt)]

https://reviewboard.mozilla.org/r/19569/#review17513

::: dom/media/webaudio/AudioNodeStream.cpp:736
(Diff revision 1)
> +  if (((mActiveInputCount >= 0 || mEngine->IsActive()) &&

This should be strictly mActiveInputCount > 0.

With that change there are some test failures so feel free to hold off review until I sort them out.

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8662209 [details]
MozReview Request: bug 1205540 mark BufferSource finished only when producing silent output block r?padenot

bug 1205540 mark BufferSource finished only when producing silent output block r?padenot

This allows simpler processing of the finished state to mark the node as an
inactive input of any downstream nodes.  Otherwise the input could not be
considered inactive until after downstream nodes have finished processing,
but ProcessInput() may not be called again on finished streams.

AudioBufferSourceNode now behaves the same as OscillatorNode and similarly
to nodes that release a playing ref.

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1205540 initialize mBufferFormat when constructing silent block r?padenot

bug 1205540 account for active inputs and skip processing when streams are inactive r?padenot

Comment 9

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c8c46376357

Comment 10

[Paul Adenot (:padenot)]

Comment on attachment 8662206 [details]
MozReview Request: bug 1205540 don't send more null chunks than necessary to AnalyserNode r?padenot

https://reviewboard.mozilla.org/r/19563/#review17657

Comment 11

[Paul Adenot (:padenot)]

Comment on attachment 8662207 [details]
MozReview Request: bug 1205540 provide querying whether engines need to continue processing even without input r?padenot

https://reviewboard.mozilla.org/r/19565/#review17581

::: dom/media/webaudio/AudioBufferSourceNode.cpp:514
(Diff revision 1)
> +    return mBufferSampleRate != 0;

Comment 12

[Paul Adenot (:padenot)]

Comment on attachment 8662208 [details]
MozReview Request: bug 1205540 make source stream available during RemoveInput r?padenot

https://reviewboard.mozilla.org/r/19567/#review17659

Comment 13

[Paul Adenot (:padenot)]

Comment on attachment 8662209 [details]
MozReview Request: bug 1205540 mark BufferSource finished only when producing silent output block r?padenot

https://reviewboard.mozilla.org/r/19569/#review17661

Comment 14

[Paul Adenot (:padenot)]

Comment on attachment 8662345 [details]
MozReview Request: bug 1205540 initialize mBufferFormat when constructing silent block r?padenot

https://reviewboard.mozilla.org/r/19581/#review17663

Comment 15

[Paul Adenot (:padenot)]

(In reply to Paul Adenot (:padenot) from comment #11)
> Comment on attachment 8662207 [details]
> MozReview Request: bug 1205540 provide querying whether engines need to
> continue processing even without input r?padenot
> 
> https://reviewboard.mozilla.org/r/19565/#review17581
> 
> ::: dom/media/webaudio/AudioBufferSourceNode.cpp:514
> (Diff revision 1)
> > +    return mBufferSampleRate != 0;

Not sure what happened with review board, I don't have anything in particular to say about this line.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4de5f87180fb
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d122cb34921
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3f6e1db7233
https://hg.mozilla.org/integration/mozilla-inbound/rev/abace4cdec06
https://hg.mozilla.org/integration/mozilla-inbound/rev/e89d8182d588

Comment 17

[Carsten Book [:Tomcat]]

sorry had to back this out for assertion failures like https://treeherder.mozilla.org/logviewer.html#?job_id=14327934&repo=mozilla-inbound

Comment 18

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb30e5ee32f0

Comment 23

[Karl Tomlinson (:karlt)]

Comment on attachment 8662345 [details]
MozReview Request: bug 1205540 initialize mBufferFormat when constructing silent block r?padenot

bug 1205540 initialize mBufferFormat when constructing silent block r?padenot

This makes an AudioBlock valid for code testing mBufferFormat without IsNull(),
without the need for explicit SetNull().

This is useful so that setting AudioNodeStream::mLastChunks each iteration is
not required for inactive nodes.

Comment 25

[Paul Adenot (:padenot)]

Comment on attachment 8663473 [details]
MozReview Request: bug 1205540 account for active inputs and skip processing when streams are inactive r=padenot

https://reviewboard.mozilla.org/r/19815/#review17819

Comment 26

[Karl Tomlinson (:karlt)]

Thanks for all the reviews, Paul.

I'm going to assume that comment 25 was aimed at the latest change
https://reviewboard.mozilla.org/r/19561/diff/2-3/
https://reviewboard.mozilla.org/r/19581/diff/2#index_header

Inserting a changeset, as I did, makes changeset evolution hard to track and
the reviewboard UI looks very similar regardless of which particular changeset
is being reviewed.

Let me know if I'm jumping to conclusions.

Comment 27

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c4fb8ae2b14a
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7004e27c6a2
https://hg.mozilla.org/integration/mozilla-inbound/rev/99cfd412946e
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c607f3f39d5
https://hg.mozilla.org/integration/mozilla-inbound/rev/75a8cb37b6bc
https://hg.mozilla.org/integration/mozilla-inbound/rev/28c008effea1

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c4fb8ae2b14a
https://hg.mozilla.org/mozilla-central/rev/e7004e27c6a2
https://hg.mozilla.org/mozilla-central/rev/99cfd412946e
https://hg.mozilla.org/mozilla-central/rev/5c607f3f39d5
https://hg.mozilla.org/mozilla-central/rev/75a8cb37b6bc
https://hg.mozilla.org/mozilla-central/rev/28c008effea1

Comment 29

[Paul Adenot (:padenot)]

(In reply to Karl Tomlinson (ni?:karlt) from comment #26)
> Thanks for all the reviews, Paul.
> 
> I'm going to assume that comment 25 was aimed at the latest change
> https://reviewboard.mozilla.org/r/19561/diff/2-3/
> https://reviewboard.mozilla.org/r/19581/diff/2#index_header
> 
> Inserting a changeset, as I did, makes changeset evolution hard to track and
> the reviewboard UI looks very similar regardless of which particular
> changeset
> is being reviewed.
> 
> Let me know if I'm jumping to conclusions.

No you're right, I'm really struggling with ReviewBoard, I need to invest a bit more time in it.

Comment 30

[Paul Adenot (:padenot)]

Comment on attachment 8662345 [details]
MozReview Request: bug 1205540 initialize mBufferFormat when constructing silent block r?padenot

(clearing the review request)