Bug 156889 – Speed up bound functions a bit

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 276996 [details]
work in progress

It doesn't compile yet.

[Filip Pizlo]

Created attachment 277008 [details]
it does things!

6x faster on my microbenchmark

[Saam Barati]

Comment on attachment 277008 [details]
it does things!

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

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:1099
> +    jit.and32(CCallHelpers::TrustedImm32(-stackAlignmentBytes()), GPRInfo::regT2);

This is nifty. I didn't know this worked for rounding up to a power two but it makes sense because of two's complement.

[Filip Pizlo]

Created attachment 277093 [details]
the patch

[WebKit Commit Bot]

Attachment 277093 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/ThunkGenerators.h:69:  The parameter name "vm" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "vm" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "function" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "intrinsic" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:300:  The parameter name "function" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/VM.h:413:  The parameter name "intrinsic" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/jit/ThunkGenerators.cpp:1100:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 7 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 277093 [details]
the patch

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

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:1066
> +    static std::once_flag dummyCallLinkInfoFlag;
> +    static CallLinkInfo* dummyCallLinkInfo;
> +    std::call_once(
> +        dummyCallLinkInfoFlag,
> +        [] () {
> +            dummyCallLinkInfo = new CallLinkInfo();
> +            dummyCallLinkInfo->setUpCall(CallLinkInfo::Call, CodeOrigin(), GPRInfo::regT3);
> +        });

I'll remove this.  It's not needed.

[Saam Barati]

Comment on attachment 277093 [details]
the patch

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

r=me

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:1099
> +    jit.add32(CCallHelpers::TrustedImm32(stackAlignmentBytes() - 1), GPRInfo::regT2);
> +    jit.and32(CCallHelpers::TrustedImm32(-stackAlignmentBytes()), GPRInfo::regT2);

It would be cool to make this a JIT method.

[Saam Barati]

looks like some debug tests might be broken.

[Build Bot]

Comment on attachment 277093 [details]
the patch

Attachment 277093 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1203810

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 277102 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Filip Pizlo]

(In reply to comment #8)
> looks like some debug tests might be broken.

Wow yeah, that's what I get for not testing debug.  Doing that now.

[Filip Pizlo]

Created attachment 277109 [details]
patch for landing

To fix debug failures I had to do more dejankification of the "OMG no JIT" path.

[WebKit Commit Bot]

Attachment 277109 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/ThunkGenerators.h:69:  The parameter name "vm" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "vm" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "function" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "intrinsic" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:300:  The parameter name "function" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/VM.h:413:  The parameter name "intrinsic" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 277109 [details]
patch for landing

Attachment 277109 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1204518

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 277121 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Filip Pizlo]

Created attachment 277122 [details]
patch for landing

Silenced a pointless null check.

[WebKit Commit Bot]

Attachment 277122 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/ThunkGenerators.h:69:  The parameter name "vm" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "vm" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "function" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:261:  The parameter name "intrinsic" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/Executable.h:300:  The parameter name "function" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/VM.h:413:  The parameter name "intrinsic" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/199946

[Csaba Osztrogonác]

(In reply to comment #18)
> Landed in http://trac.webkit.org/changeset/199946

It broke the cloop build.

[Filip Pizlo]

(In reply to comment #19)
> (In reply to comment #18)
> > Landed in http://trac.webkit.org/changeset/199946
> 
> It broke the cloop build.

Of course it did.  I'll fix it.

[Filip Pizlo]

Landed cloop fix in http://trac.webkit.org/changeset/199949