[Webkit-unassigned] [Bug 63644] [v8] Improve performance of typed array set() taking Array

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63644


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #106509|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #3 from David Levin <levin at chromium.org>  2011-09-07 14:31:05 PST ---
(From update of attachment 106509)
View in context: https://bugs.webkit.org/attachment.cgi?id=106509&action=review

Just a few comments. Also thanks to dslomov who helped.

> ChangeLog:19
> +        * ../../Source/WebCore/bindings/v8/custom/V8ArrayBufferViewCustomScript.js: Added.

This ChangeLog is messed up. See other entries in this file.

> Source/WebCore/WebCore.gyp/WebCore.gyp:483
> +          'action_name': 'generateV8ArrayBufferViewCustomScript.h',

Similar items are named like this: generateV8ArrayBufferViewCustomScript

> Source/WebCore/WebCore.gyp/WebCore.gyp:497
> +          'message': 'Generating V8ArrayBufferViewCustomScript.h from V8ArrayBuferViewCustomScript.js',

typo: V8ArrayBuferViewCustomScript

> Source/WebCore/bindings/v8/V8BindingScripts.cpp:32
> +namespace WebCore {

add blank line.

> Source/WebCore/bindings/v8/V8BindingScripts.cpp:33
> +void V8BindingScripts::runScriptsInGlobalContext(v8::Handle<v8::Context> v8Context)

s/runScriptsInGlobalContext/runScript/ ?

> Source/WebCore/bindings/v8/V8BindingScripts.cpp:37
> +                        sizeof(V8ArrayBufferViewCustomScript_js));

indentation is wrong.

> Source/WebCore/bindings/v8/V8BindingScripts.cpp:40
> +}

add blank line.

> Source/WebCore/bindings/v8/V8BindingScripts.h:37
> +    static void runScriptsInGlobalContext(v8::Handle<v8::Context> v8Context);

param name adds no info. Omit it.

> Source/WebCore/bindings/v8/V8DOMWindowShell.cpp:2
>   * Copyright (C) 2008, 2009 Google Inc. All rights reserved.

Add 2011.

> Source/WebCore/bindings/v8/custom/V8ArrayBufferViewCustomScript.js:34
> +                var n = source.length;

Please use descriptive variable names. (not "n")

> Source/WebCore/bindings/v8/custom/V8ArrayBufferViewCustomScript.js:41
> +            } else {

Don't use {} for single line statements.

> LayoutTests/fast/canvas/webgl/array-unit-tests.html:647
> +function testSettingFromArrayWithMinusZeroOffset(type, name) {

This test isn't used.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.