Defer functions enclosed in all contexts. #2666
Conversation
pleath
force-pushed
the
deferfib
branch
3 times, most recently
from
914cf59
to
4fb8341
Compare
|
@aneeshdk This is the feature work we discussed last week. Can you take a look when you get a chance? (Thanks.) |
| { | ||
| func->bodyScope = this->currentScope; | ||
| func = Anew(alloc, FuncInfo, pfi->GetDisplayName(), alloc, nullptr, nullptr, nullptr, pfi); | ||
| newFunc = true; |
There was a problem hiding this comment.
Sorry -- can you explain what you mean?
| @@ -2023,7 +2041,7 @@ void ByteCodeGenerator::CheckDeferParseHasMaybeEscapedNestedFunc() | |||
| else | |||
| { | |||
| // We have to wait until it is parsed before we populate the stack nested func parent. | |||
| FuncInfo * parentFunc = top->GetBodyScope()->GetEnclosingFunc(); | |||
| FuncInfo * parentFunc = top->GetParamScope() ? top->GetParamScope()->GetEnclosingFunc() : top->GetBodyScope()->GetEnclosingFunc(); | |||
There was a problem hiding this comment.
top->GetParamScope()->GetEnclosingFunc() : top->GetBodyScope()->GetEnclosingFunc() [](start = 55, length = 82)
Aren't these two the same?
| bool scopeIsEmpty = scope->IsEmpty(); | ||
| scope->SetIsObject(); | ||
| scope->SetCapturesAll(true); | ||
| scope->SetMustInstantiate(!scopeIsEmpty); | ||
| } | ||
|
|
||
| if (scope->GetHasOwnLocalInClosure()) | ||
| { | ||
| byteCodeGenerator->ProcessScopeWithCapturedSym(scope); |
There was a problem hiding this comment.
byteCodeGenerator->ProcessScopeWithCapturedSym(scope); [](start = 8, length = 54)
Why do we have to do this here? Doesn't this happen when we find the captured symbol itself? Or is it to cover the case where the scope is recreated from the ScopeInfo so we have to mark it again? If that is the case should we do this while Restoring the scope?
There was a problem hiding this comment.
We've been doing this when we find the capturing reference to the symbol. Now that reference may be deferred, so byte code gen won't see it. It would be great to go through and remove the calls at the points where they're no longer needed, but I'm not doing that cleanup as part of this change.
lib/Runtime/ByteCode/ScopeInfo.cpp
Outdated
| TRACE_BYTECODE(_u("\nSave ScopeInfo: %s parent: %s #symbols: %d %s\n"), | ||
| scope->GetFunc()->name, parent ? parent->GetDisplayName() : _u(""), count, | ||
| TRACE_BYTECODE(_u("\nSave ScopeInfo: %s #symbols: %d %s\n"), | ||
| scope->GetFunc()->name, /*parent ? parent->GetDisplayName() : _u(""),*/ count, |
There was a problem hiding this comment.
/parent ? parent->GetDisplayName() : _u(""),/ [](start = 36, length = 47)
Don't need this
There was a problem hiding this comment.
Thanks for spotting these.
lib/Runtime/ByteCode/ScopeInfo.cpp
Outdated
| scopeInfo->isObject ? _u("isObject") : _u("")); | ||
|
|
||
| MapSymbolData mapSymbolData = { byteCodeGenerator, scope->GetFunc(), 0 }; | ||
| MapSymbolData mapSymbolData = { /*byteCodeGenerator,*/ scope->GetFunc(), 0 }; |
There was a problem hiding this comment.
/byteCodeGenerator,/ [](start = 40, length = 22)
Don't need this
lib/Runtime/ByteCode/ScopeInfo.cpp
Outdated
| } | ||
|
|
||
| void ScopeInfo::SaveScopeInfoForDeferParse(ByteCodeGenerator* byteCodeGenerator, FuncInfo* parentFunc, FuncInfo* funcInfo) | ||
| void ScopeInfo::SaveEnclosingScopeInfo(ByteCodeGenerator* byteCodeGenerator, /*FuncInfo* parentFunc,*/ FuncInfo* funcInfo) |
There was a problem hiding this comment.
/FuncInfo parentFunc,*/ [](start = 81, length = 25)
Remove this
|
|
…ns enclosed in scopes other than function body and function expression scope -- in particular, ES6-style lexical scopes and parameter scopes. This requires changing ScopeInfo so that it is associated with a single scope rather than a function body. Instead of carrying per-function information for body/var scope, parameter scope, and function expression scope, ScopeInfo will describe one scope, with a value identifying the scope type, a pointer to the FunctionInfo that contains it, and a pointer to the enclosing ScopeInfo.. A FunctionProxy will point to the ScopeInfo that is the nearest enclosing scope. At parse time, we will reconstitute the closure environment by walking the list of enclosing ScopeInfo's. The code that allowed redeferral to work around the context limitation of deferring parsing is deleted, and the OptimizeBlockScope feature is off by default, as it doesn't play well with this new logic and isn't required to make (re-)deferral effective. (We will want to revisit it.)
Merge pull request #2666 from pleath:deferfib Support deferral of functions enclosed in scopes other than function body and function expression scope -- in particular, ES6-style lexical scopes and parameter scopes. This requires changing ScopeInfo so that it is associated with a single scope rather than a function body. Instead of carrying per-function information for body/var scope, parameter scope, and function expression scope, ScopeInfo will describe one scope, with a value identifying the scope type, a pointer to the FunctionInfo that contains it, and a pointer to the enclosing ScopeInfo. A FunctionProxy will point to the ScopeInfo that is the nearest enclosing scope. At parse time, we will reconstitute the closure environment by walking the list of enclosing ScopeInfo's. The code that allowed redeferral to work around the context limitation of deferring parsing is deleted, and the OptimizeBlockScope feature is off by default, as it doesn't play well with this new logic and isn't required to make (re-)deferral effective. (We will want to revisit it, though, so I've left it there.)
Support deferral of functions enclosed in scopes other than function body and function expression scope -- in particular, ES6-style lexical scopes and parameter scopes. This requires changing ScopeInfo so that it is associated with a single scope rather than a function body. Instead of carrying per-function information for body/var scope, parameter scope, and function expression scope, ScopeInfo will describe one scope, with a value identifying the scope type, a pointer to the FunctionInfo that contains it, and a pointer to the enclosing ScopeInfo. A FunctionProxy will point to the ScopeInfo that is the nearest enclosing scope. At parse time, we will reconstitute the closure environment by walking the list of enclosing ScopeInfo's. The code that allowed redeferral to work around the context limitation of deferring parsing is deleted, and the OptimizeBlockScope feature is off by default, as it doesn't play well with this new logic and isn't required to make (re-)deferral effective. (We will want to revisit it, though, so I've left it there.)