Bug 20982 - Speed up the apply method of functions by special-casing array and 'arguments' objects
Summary: Speed up the apply method of functions by special-casing array and 'arguments...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks: 20813
  Show dependency treegraph
 
Reported: 2008-09-21 20:49 PDT by Cameron Zwarich (cpst)
Modified: 2008-09-22 14:21 PDT (History)
1 user (show)

See Also:


Attachments
patch (10.70 KB, patch)
2008-09-21 23:18 PDT, Sam Weinig
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron Zwarich (cpst) 2008-09-21 20:49:54 PDT
Currently, functionProtoFuncApply() accesses the arguments via numerical indices using the generic path for lookup. We should really fix this. Sam has a patch to do this, so I'm assigning this to him.
Comment 1 Sam Weinig 2008-09-21 23:18:06 PDT
Created attachment 23643 [details]
patch
Comment 2 Sam Weinig 2008-09-21 23:34:38 PDT
Comment on attachment 23643 [details]
patch

Taking this out of the review queue.  I am afraid that this might break RegExpMatchesArray.
Comment 3 Cameron Zwarich (cpst) 2008-09-21 23:35:29 PDT
Comment on attachment 23643 [details]
patch

As we discussed on IRC, this code seems like it would break with the RegExpMatchesArray, since that array is lazily filled when a property is retrieved.
Comment 4 Darin Adler 2008-09-22 07:49:41 PDT
Should be straightforward to fix it for RegExpMatchesArray one of two ways:

    1) Make a call to fill out a RegExpMatchesArray before doing the rest.
    2) Do a check for JSArray with the exec->machine()->isJSArray() function that returns false for RegExpMatchesArray and keep a slow case for JSArray derived classes around for RegExpMatchesArray (and maybe WebCore/bridge bindings arrays?)
Comment 5 Sam Weinig 2008-09-22 11:36:22 PDT
Comment on attachment 23643 [details]
patch

Putting this back up for review since it doesn't break RegExpMatchesArray.  I have added new test cases for it however and will land them if this gets reviewed.
Comment 6 Darin Adler 2008-09-22 11:41:47 PDT
Comment on attachment 23643 [details]
patch

If speed really matters for Arguments, then we need to do various checks outside the loops rather than inside, so for example:

    - We need an entirely separate loop for the Arguments case where there are no deleted arguments.

    - The loop for the parameters should come first, then a separate loop to handle arguments.

r=me
Comment 7 Sam Weinig 2008-09-22 14:21:28 PDT
Landed in r36779.  There is still more work to be done to speed up Function.apply however.