Consistent 30% improvement in audio mixer running time.

Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=chromium:687502
Review-Url: https://codereview.webrtc.org/2659423002
Cr-Commit-Position: refs/heads/master@{#16396}
Committed: https://chromium.googlesource.com/external/webrtc/+/4637b6afca431be8caf1d22151a2cbb2e86a33d5

[aleloi, 2017-01-30 15:42:31]

Description was changed from

==========
Consistent 30% improvement in audio mixer running time.

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample of an audio
frame with a floating point number. This number is almost always
exactly 1.0f. The only situation when it's not 1 is when an audio
steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build. There can be up
to 3 streams. Then the improvement is larger.

BUG=None
==========

to

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample of an audio
frame with a floating point number. This number is almost always
exactly 1.0f. The only situation when it's not 1 is when an audio
steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build. There can be up
to 3 streams. Then the improvement is larger.

BUG=None
==========

[aleloi, 2017-01-30 15:42:31]

aleloi@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[aleloi, 2017-01-30 15:45:25]

Description was changed from

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample of an audio
frame with a floating point number. This number is almost always
exactly 1.0f. The only situation when it's not 1 is when an audio
steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build. There can be up
to 3 streams. Then the improvement is larger.

BUG=None
==========

to

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample of an audio
frame with a floating point number. This number is almost always
exactly 1.0f. The only situation when it's not 1 is when an audio
steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=None
==========

[aleloi, 2017-01-30 15:45:36]

The CQ bit was checked by aleloi@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 15:45:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[aleloi, 2017-01-30 15:46:39]

Really short one, PTAL!

I did some profiling of mixing in floats/int32 and made some interesting
findings...

[aleloi, 2017-01-30 16:09:08]

Description was changed from

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample of an audio
frame with a floating point number. This number is almost always
exactly 1.0f. The only situation when it's not 1 is when an audio
steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=None
==========

to

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=None
==========

[commit-bot: I haz the power, 2017-01-30 16:26:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 16:26:02]

Dry run: This issue passed the CQ dry run.

[hlundin-webrtc, 2017-02-01 08:33:05]

Sorry for the delayed review. LGTM!

[hlundin-webrtc, 2017-02-01 08:34:40]

On 2017/02/01 08:33:05, hlundin-webrtc wrote:
> Sorry for the delayed review. LGTM!

Before landing, file a Chromium bug for this and attach to this CL.
Not LGTM until fixed.

[aleloi, 2017-02-01 09:53:59]

Description was changed from

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=None
==========

to

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=webrtc:7092
==========

[aleloi, 2017-02-01 09:54:17]

Done!

[aleloi, 2017-02-01 10:13:40]

Description was changed from

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=webrtc:7092
==========

to

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=chromium:687502
==========

[hlundin-webrtc, 2017-02-01 10:15:35]

lgtm

[aleloi, 2017-02-01 10:18:40]

The CQ bit was checked by aleloi@webrtc.org

[commit-bot: I haz the power, 2017-02-01 10:18:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-01 11:43:32]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1485944320476640, "parent_rev":
"35fc2aa82fb5a562f3f76f2b91a55f05ebfd4874", "commit_rev":
"4637b6afca431be8caf1d22151a2cbb2e86a33d5"}

[commit-bot: I haz the power, 2017-02-01 11:43:34]

Description was changed from

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=chromium:687502
==========

to

==========
Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced 
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of 
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no 
APM limiter. There can be up to 3 streams.

BUG=chromium:687502

Review-Url: https://codereview.webrtc.org/2659423002
Cr-Commit-Position: refs/heads/master@{#16396}
Committed:
https://chromium.googlesource.com/external/webrtc/+/4637b6afca431be8caf1d2215...
==========

[commit-bot: I haz the power, 2017-02-01 11:43:35]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/webrtc/+/4637b6afca431be8caf1d2215...

[the sun, 2017-02-07 08:53:22]

solenberg@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[the sun, 2017-02-07 08:53:22]

https://codereview.webrtc.org/2659423002/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_frame_manipulator.cc (right):

https://codereview.webrtc.org/2659423002/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_frame_manipulator.cc:32: if (start_gain ==
target_gain) {
Comparing floats is fuzzy and operator == is not to be trusted to do what you
intend. See e.g.:

http://stackoverflow.com/questions/41989570/in-c-is-exactly-one-of-and-guaran...
http://stackoverflow.com/questions/17333/what-is-the-most-effective-way-for-f...

hlundin@, do you know if we have any function in the codebase (outside of gtest)
for this?

[hlundin-webrtc, 2017-02-07 21:34:51]

https://codereview.webrtc.org/2659423002/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_frame_manipulator.cc (right):

https://codereview.webrtc.org/2659423002/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_frame_manipulator.cc:32: if (start_gain ==
target_gain) {
On 2017/02/07 08:53:22, the sun wrote:
> Comparing floats is fuzzy and operator == is not to be trusted to do what you
> intend. See e.g.:
> 
>
http://stackoverflow.com/questions/41989570/in-c-is-exactly-one-of-and-guaran...
>
http://stackoverflow.com/questions/17333/what-is-the-most-effective-way-for-f...
> 
> hlundin@, do you know if we have any function in the codebase (outside of
gtest)
> for this?

No, I know of no such function in our codebase.

[aleloi, 2017-02-08 08:43:18]

On 2017/02/07 08:53:22, the sun wrote:
>
https://codereview.webrtc.org/2659423002/diff/1/webrtc/modules/audio_mixer/au...
> File webrtc/modules/audio_mixer/audio_frame_manipulator.cc (right):
> 
>
https://codereview.webrtc.org/2659423002/diff/1/webrtc/modules/audio_mixer/au...
> webrtc/modules/audio_mixer/audio_frame_manipulator.cc:32: if (start_gain ==
> target_gain) {
> Comparing floats is fuzzy and operator == is not to be trusted to do what you
> intend. See e.g.:
> 
>
http://stackoverflow.com/questions/41989570/in-c-is-exactly-one-of-and-guaran...
>
http://stackoverflow.com/questions/17333/what-is-the-most-effective-way-for-f...
> 
> hlundin@, do you know if we have any function in the codebase (outside of
gtest)
> for this?

Ramp() is only ever called with 0.0 and 1.0. == works as long as no number is
NaN.