inline callbacks - #5081
Conversation
60e5471 to
529a833
Compare
|
|
||
| unsigned int inlineeCount; | ||
| unsigned int ldFldInlineeCount; | ||
| unsigned int callbackInlineeCount; |
There was a problem hiding this comment.
Could we put this int down in the X64_PAD4 space after callbackInlinees?
| Js::FunctionInfo * functionInfo = GetCallSiteCallbackInfo(inliner, profiledCallSiteId); | ||
| if (functionInfo) | ||
| { | ||
| return Inline(inliner, functionInfo, false, false, true, GetConstantArgInfo(inliner, profiledCallSiteId), profiledCallSiteId, recursiveInlineDepth, true); |
There was a problem hiding this comment.
false [](start = 45, length = 5)
How do we guarantee at this point that it's not a constructor call?
There was a problem hiding this comment.
constructor call is specific to the callsite, not the function. It is set true for calls of the form new foo(). At the point callback info is collected, we don't know how the callsite will use the callback function. The result of guessing wrong here is either failing to serialize data that could have been used, or serializing data that won't be used, but neither case is fatal
| Output::Print(__VA_ARGS__); \ | ||
| Output::Flush(); \ | ||
| } | ||
| #define INLINE_TRACE_AND_TESTTRACE(...) \ |
There was a problem hiding this comment.
Thanks for cleaning this up :)
| //------------------------------------------------------------------------------------------------------- | ||
| // Copyright (C) Microsoft. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. | ||
| //------------------------------------------------------------------------------------------------------- |
There was a problem hiding this comment.
Are there other configurations that might be worth testing? Maybe passing the callback through multiple layers like function Dispatch2(f) { Dispatch(f); } or a callback that calls things like function Dispatch3(f) { f(Foo); } where we call Dispatch3(Dispatch) after the warmup?
There was a problem hiding this comment.
or maybe a case where the callback isn't the first param
In reply to: 185833307 [](ancestors = 185833307)
| Field(uint) ldFldInlineeCount; | ||
|
|
||
| // Number of functions passed as arguments to be inlined. | ||
| Field(uint) callbackInlineeCount; |
There was a problem hiding this comment.
We currently have four unused bytes after globalObjTypeSpecFldInfoCount; could this go up there?
There was a problem hiding this comment.
This member isn't actually needed. I'm going to remove it.
6a75574 to
530b02e
Compare
| // False if there is more than one ArgIn that is a function object or a function object with arg number greater than MaxInlineeArgoutCount | ||
| Field(uint8) canInlineCallback : 1; | ||
|
|
||
| Field(uint8) isConstructor : 1; |
There was a problem hiding this comment.
I added that before looking into what/how that tracks. thanks for catching that.
…thout callbacks Rather than store the callback info in the CallSiteInfo, allocate it as needed stored on the function body, similar to how PolymorphicInlineData is handled.
Merge pull request #5081 from sigatrev:inlineCallbacks This commit adds tracking of up to 1 function passed as an argument to any given callsite, and provides the neccessary data to the JIT to inline it. The inliner is able to follow the single-def chain up multiple functions to find the approriate data, allowing inling to occur at megamorphic in cases where the function is a callback. This will allow more efficient javascript implementations of built-ins like filter and sort.
3 participants
This commit adds tracking of up to 1 function passed as an argument to any given callsite, and provides the neccessary data to the JIT to inline it. The inliner is able to follow the single-def chain up multiple functions to find the approriate data, allowing inling to occur at megamorphic in cases where the function is a callback. This will allow more efficient javascript implementations of built-ins like filter and sort.