Inline the only callers of TypedArrayBase::{setRange,zeroRange}

Inline the only callers of TypedArrayBase::{setRange,zeroRange}

These two methods are only called by WebAudio code, and only
on Float32Arrays. Replace those calls by inlining the appropriate
calls to memmove/memset.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197058

[adamk, 2015-06-12 00:43:47]

adamk@chromium.org changed reviewers:
+ jochen@chromium.org, kbr@chromium.org

[adamk, 2015-06-12 00:43:48]

This is barely worthwhile, but I ran into it while cleaning up other stuff in
Blink TypedArray code. Feel free to disregard if you think it is, indeed, not
worthwhile.

[Ken Russell (switch to Gerrit), 2015-06-12 01:09:48]

kbr@chromium.org changed reviewers:
+ hongchan@chromium.org, rtoy@chromium.org

[Ken Russell (switch to Gerrit), 2015-06-12 01:09:48]

lgtm if rtoy and/or hongchan are happy

[jochen (gone - plz use gerrit), 2015-06-12 08:35:09]

lgtm

[adamk, 2015-06-12 17:59:25]

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
File Source/modules/webaudio/AudioBuffer.cpp (right):

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
Source/modules/webaudio/AudioBuffer.cpp:181: memmove(dst, src, m_length *
sizeof(*dst));
For WebAudio reviewers: this memmove is from the original, generic code. I
suspect this could be memcpy here, though I also don't know how important perf
is here.

[Raymond Toy, 2015-06-12 18:05:50]

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
File Source/modules/webaudio/AudioBuffer.cpp (right):

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
Source/modules/webaudio/AudioBuffer.cpp:181: memmove(dst, src, m_length *
sizeof(*dst));
On 2015/06/12 17:59:25, adamk wrote:
> For WebAudio reviewers: this memmove is from the original, generic code. I
> suspect this could be memcpy here, though I also don't know how important perf
> is here.

If the original did this, I'm happy with leaving it as is.

Also, the original had some checks on the parameters.  I think these checks
should be here. (Unless we know the checks will always pass. I didn't look at
that.)

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
Source/modules/webaudio/AudioBuffer.cpp:324: memset(data, 0, length() *
sizeof(*data));
The original had some checks on the parameters. Are we guaranteed we're safe
here?

[adamk, 2015-06-12 18:14:28]

The previous checks on the parameters always passed trivially, but I can make
the code _look_ safer if you wish (see below).

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
File Source/modules/webaudio/AudioBuffer.cpp (right):

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
Source/modules/webaudio/AudioBuffer.cpp:181: memmove(dst, src, m_length *
sizeof(*dst));
On 2015/06/12 18:05:50, Raymond Toy wrote:
> On 2015/06/12 17:59:25, adamk wrote:
> > For WebAudio reviewers: this memmove is from the original, generic code. I
> > suspect this could be memcpy here, though I also don't know how important
perf
> > is here.
> 
> If the original did this, I'm happy with leaving it as is.
> 
> Also, the original had some checks on the parameters.  I think these checks
> should be here. (Unless we know the checks will always pass. I didn't look at
> that.)

The destination of this memmove was just created with size m_length, so this is
safe. I could make it look safer by calling
channelDataArray->view()->byteLength() (which will do the same math internally
that I'm doing here). Would you prefer that?

https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
Source/modules/webaudio/AudioBuffer.cpp:324: memset(data, 0, length() *
sizeof(*data));
On 2015/06/12 18:05:50, Raymond Toy wrote:
> The original had some checks on the parameters. Are we guaranteed we're safe
> here?

Same as above, all the channel data arrays have the same length as m_len gth.
But I can make this look safer by calling byteLength() if you'd prefer.

[Raymond Toy, 2015-06-12 18:19:56]

On 2015/06/12 18:14:28, adamk wrote:
> The previous checks on the parameters always passed trivially, but I can make
> the code _look_ safer if you wish (see below).
> 
>
https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
> File Source/modules/webaudio/AudioBuffer.cpp (right):
> 
>
https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
> Source/modules/webaudio/AudioBuffer.cpp:181: memmove(dst, src, m_length *
> sizeof(*dst));
> On 2015/06/12 18:05:50, Raymond Toy wrote:
> > On 2015/06/12 17:59:25, adamk wrote:
> > > For WebAudio reviewers: this memmove is from the original, generic code. I
> > > suspect this could be memcpy here, though I also don't know how important
> perf
> > > is here.
> > 
> > If the original did this, I'm happy with leaving it as is.
> > 
> > Also, the original had some checks on the parameters.  I think these checks
> > should be here. (Unless we know the checks will always pass. I didn't look
at
> > that.)
> 
> The destination of this memmove was just created with size m_length, so this
is
> safe. I could make it look safer by calling
> channelDataArray->view()->byteLength() (which will do the same math internally
> that I'm doing here). Would you prefer that?
> 
>
https://codereview.chromium.org/1180023002/diff/1/Source/modules/webaudio/Aud...
> Source/modules/webaudio/AudioBuffer.cpp:324: memset(data, 0, length() *
> sizeof(*data));
> On 2015/06/12 18:05:50, Raymond Toy wrote:
> > The original had some checks on the parameters. Are we guaranteed we're safe
> > here?
> 
> Same as above, all the channel data arrays have the same length as m_len gth.
> But I can make this look safer by calling byteLength() if you'd prefer.

No, everything is fine.

lgtm.

[adamk, 2015-06-12 18:20:48]

One more note about the length stuff: there are already other bits of code that
memcpy with this assumption in place.

[hongchan, 2015-06-12 18:23:38]

lgtm

[adamk, 2015-06-12 18:23:48]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2015-06-12 18:24:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1180023002/1

[commit-bot: I haz the power, 2015-06-12 19:55:13]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197058