Speedometer spends about 10% of its time in this function. I have a 10-15% speedup by rewriting it as a builtin. This isn't going to be a huge win for speedometer, but since 10-15% is significant, we might as well land it.
There's an older bug somewhere where i did this with a bunch of the required logic. There were issues i was having making the logic universally faster at that time. (Part of the reason for .@call and .@apply exist :D )
(In reply to comment #1) > There's an older bug somewhere where i did this with a bunch of the required > logic. There were issues i was having making the logic universally faster at > that time. > > (Part of the reason for .@call and .@apply exist :D ) I don't think this needs to @call or @apply anything. It's basically just inspecting arguments, and maybe building an array. And doing some property accesses. So I think property accesses, argument length introspection, happen to be faster in JS, especially once we get to the JITs and the call may be inlined. I needed to make a few helper functions for a few operations that Function.prototype.bind does.
Created attachment 288640 [details] patch
Created attachment 288641 [details] patch
Comment on attachment 288641 [details] patch Attachment 288641 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2062262 New failing tests: js/dom/function-bind.html
Created attachment 288649 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 288641 [details] patch Oh, i see what you're doing - the prior work endeavored to avoid a custom object for jsboundfunction.
(In reply to comment #7) > Comment on attachment 288641 [details] > patch > > Oh, i see what you're doing - the prior work endeavored to avoid a custom > object for jsboundfunction. @Oliver: not sure if this helps add context, but JSC already has super fast bound function calls for the common case that you didn't add any additional args. It's the actual function.bind - the creation of the bound function object - that is slow right now.
Created attachment 288651 [details] patch update test results for new error message.
(In reply to comment #7) > Comment on attachment 288641 [details] > patch > > Oh, i see what you're doing - the prior work endeavored to avoid a custom > object for jsboundfunction. Yeah it might be worth it to make this even faster. This was just low hanging fruit, so I decided to translate the C function into JS. However, to make this faster, I think we'd ideally want to be able to inline the allocation of the actual bound function cell into the JITs. This would be nice. There are some weird things that constructor does, so it would probably require more work than just translating the creation into assembly.
Comment on attachment 288651 [details] patch Clearing flags on attachment: 288651 Committed r205848: <http://trac.webkit.org/changeset/205848>
All reviewed patches have been landed. Closing bug.
(In reply to comment #10) > (In reply to comment #7) > > Comment on attachment 288641 [details] > > patch > > > > Oh, i see what you're doing - the prior work endeavored to avoid a custom > > object for jsboundfunction. > > Yeah it might be worth it to make this even faster. This was just low > hanging fruit, so I decided to translate the C function into JS. > However, to make this faster, I think we'd ideally want to be able > to inline the allocation of the actual bound function cell into the > JITs. This would be nice. There are some weird things that constructor does, > so it would probably require more work than just translating the creation > into > assembly. The old patch literally had code along the lines of if (arguments.length == 1) { let thisValue = arguments[0]; let self = this; return function() { return self.@apply(thisValue, arguments); } } let args = [...arguments] let thisValue = args.shift(); // i think? return function() { return function() { if (arguments.length == 0) return self.@apply(thisValue, args); return self.@apply(thisValue, [...args, ...arguments]); } }
(In reply to comment #13) > (In reply to comment #10) > > (In reply to comment #7) > > > Comment on attachment 288641 [details] > > > patch > > > > > > Oh, i see what you're doing - the prior work endeavored to avoid a custom > > > object for jsboundfunction. > > > > Yeah it might be worth it to make this even faster. This was just low > > hanging fruit, so I decided to translate the C function into JS. > > However, to make this faster, I think we'd ideally want to be able > > to inline the allocation of the actual bound function cell into the > > JITs. This would be nice. There are some weird things that constructor does, > > so it would probably require more work than just translating the creation > > into > > assembly. > > The old patch literally had code along the lines of > > if (arguments.length == 1) { > let thisValue = arguments[0]; > let self = this; > return function() { return self.@apply(thisValue, arguments); } > } > > let args = [...arguments] > let thisValue = args.shift(); // i think? > return function() { > return function() { > if (arguments.length == 0) > return self.@apply(thisValue, args); > return self.@apply(thisValue, [...args, ...arguments]); > } > } I don't think this is as good though. For example: return function() { return self.@apply(thisValue, arguments); } This is worse than the current special bound function with its special JIT thunk. That thunk means that even before any warmup, bound function calls are fast. This would mean that bound function calls would only become fast if you did enough of them.