Restructure FunctionBody hierarchy for re-deferral #1278
Conversation
| @@ -583,7 +583,7 @@ namespace Js | |||
| bool IsPolymorphicCallSite(const ProfileId profiledCallSiteId) const; | |||
| // This function walks all the chained jittimedata and returns the one which match the functionInfo. | |||
| // This can return null, if the functionInfo doesn't match. | |||
| const FunctionCodeGenJitTimeData *GetJitTimeDataFromFunctionInfo(FunctionInfo *polyFunctionInfo) const; | |||
| const FunctionCodeGenJitTimeData *GetJitTimeDataFromFunctionInfo(FunctionInfo *polyFunctioInfoy) const; | |||
|
Looks good to me other than a couple minor questions. |
pleath
force-pushed
the
functioninfo
branch
2 times, most recently
from
629a1e1 to
b84eb2b
Compare
| inline void FunctionProxy::SetLocalFunctionId(LocalFunctionId functionId) | ||
| { | ||
| Assert(GetFunctionInfo()); | ||
| Assert(GetFunctionInfo()->GetFunctionProxy() == this); |
There was a problem hiding this comment.
Can these asserts be moved to GetFunctionInfo itself? Also was curious why this particular assert wasn't there in GetLocalFunctionId (another reason I suggested centralizing the asserts)
There was a problem hiding this comment.
There are cases where it’s awkward to assert in GetFunctionInfo() because we’re in the midst of swapping the functionBodyImpl. I suppose we could have a GetFunctionInfoUnchecked() for those.
--Paul
From: Hitesh Kanwathirtha [mailto:[email protected]]
Sent: Thursday, July 21, 2016 11:04 AM
To: Microsoft/ChakraCore [email protected]
Cc: Paul Leathers [email protected]; Author [email protected]
Subject: Re: [Microsoft/ChakraCore] Restructure FunctionBody hierarchy for re-deferral (#1278)
In lib/Runtime/Base/FunctionBody.hhttps://github.com//pull/1278#discussion_r71755941:
@@ -1348,16 +1393,180 @@ namespace Js
ScriptFunctionType * AllocDeferredPrototypeType(); };
- inline Js::LocalFunctionId FunctionProxy::GetLocalFunctionId() const
- {
Assert(GetFunctionInfo());return GetFunctionInfo()->GetLocalFunctionId();- }
- inline void FunctionProxy::SetLocalFunctionId(LocalFunctionId functionId)
- {
Assert(GetFunctionInfo());Assert(GetFunctionInfo()->GetFunctionProxy() == this);
Can these asserts be moved to GetFunctionInfo itself? Also was curious why this particular assert wasn't there in GetLocalFunctionId (another reason I suggested centralizing the asserts)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//pull/1278/files/b84eb2bf59a60a0647d0efcece9bc741d4855b53#r71755941, or mute the threadhttps://github.com/notifications/unsubscribe-auth/APF8RLQXTH-jjUOD-IA2WsNpgLf4uipiks5qX7SOgaJpZM4JM3EN.
pleath
force-pushed
the
functioninfo
branch
2 times, most recently
from
f071999 to
746941b
Compare
|
@pleath any update on this PR? |
|
Not today. I’ll be bringing it up to date soon. -- Paul From: Doug Ilijev [mailto:[email protected]] @pleathhttps://github.com/pleath any update on this PR? — |
…dalone proxy from which FunctionProxy does not inherit, and FunctionProxy is the basis for all the representations of user functions (FunctionBody, etc.). FunctionInfo still points to the FunctionProxy that implements the function, and FunctionProxy points to FunctionInfo. Do this to facilitate re-deferral and to maximize the memory benefit.
|
Fully synched. |
|
Does #1585 depend on this PR or are they independent changes? |
|
Subsumed by #1585. |
Restructure the FunctionBody hierarchy so that FunctionInfo is a standalone proxy from which FunctionProxy does not inherit, and FunctionProxy is the basis for all the representations of user functions (FunctionBody, etc.). FunctionInfo still points to the FunctionProxy that implements the function, and FunctionProxy points to FunctionInfo. Do this to facilitate re-deferral and to maximize the memory benefit.