Bug 46207 - Speed up function.apply(..., arguments)
Summary: Speed up function.apply(..., arguments)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-21 12:04 PDT by Oliver Hunt
Modified: 2010-09-21 16:21 PDT (History)
0 users

See Also:


Attachments
Patch (5.68 KB, patch)
2010-09-21 12:07 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (7.85 KB, patch)
2010-09-21 14:55 PDT, Oliver Hunt
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2010-09-21 12:04:44 PDT
Speed up function.apply(..., arguments)
Comment 1 Oliver Hunt 2010-09-21 12:07:43 PDT
Created attachment 68271 [details]
Patch
Comment 2 Geoffrey Garen 2010-09-21 12:27:53 PDT
Comment on attachment 68271 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68271&action=review

r=me, but I think you should make these changes.

> JavaScriptCore/jit/JITOpcodes.cpp:1533
> +        emitLoad(argsOffset, regT1, regT0);
> +        slowJumps.append(branchTestPtr(NonZero, regT0));
> +        slowJumps.append(branchTestPtr(NonZero, regT1));

To test for JSValue(), test the tag against JSValue::EmptyValueTag.

> JavaScriptCore/jit/JITOpcodes.cpp:1541
> +        slowJumps.append(branch32(Above, regT0, Imm32(Arguments::MaxArguments)));

Do you need this check on the hot path? In theory, the only limit to argument use on the hot path is the size of the register file, which you check below.

> JavaScriptCore/jit/JITOpcodes.cpp:1560
> +        if (sizeof(Register) == 2 * ScalePtr) {
> +            loadPtr(BaseIndex(regT1, regT0, ScalePtr, static_cast<unsigned>(-sizeof(Register) + sizeof(void*))), regT3);
> +            storePtr(regT3, BaseIndex(regT2, regT0, ScalePtr, sizeof(void*)));
> +        }

Might be clearer just to use #if USE(JSVALUE32_64) here instead.

> JavaScriptCore/jit/JITOpcodes.cpp:1561
> +        jump(loopStart);

This will "jump to jump" each time through the loop. Might be faster to do "branchTest32(Zero, regT0)" here, branching back to the sub32 above.
Comment 3 Oliver Hunt 2010-09-21 14:55:01 PDT
Created attachment 68294 [details]
Patch
Comment 4 Geoffrey Garen 2010-09-21 16:18:08 PDT
Comment on attachment 68294 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68294&action=review

r=me

> JavaScriptCore/jit/JITOpcodes.cpp:1572
> +    JumpList slowJumps;
> +    JumpList endBranches;

These variables are unused.
Comment 5 Oliver Hunt 2010-09-21 16:21:26 PDT
Committed r67990: <http://trac.webkit.org/changeset/67990>