Skip to content

Commit

Permalink
[Perf] Dont duplicate the function lookup in fb_call_user_func_safe*
Browse files Browse the repository at this point in the history
Summary: fb_call_user_func_safe etc were implemented via is_callable, then
call_user_func_array. Instead, we can do a single lookup of the CallInfo.

Test Plan: fast_tests slow_tests

Reviewers: myang, qigao

Reviewed By: myang

CC: ps, mwilliams, myang

Differential Revision: 343223
  • Loading branch information
mwilliams authored and macvicar committed Oct 18, 2011
1 parent d8d89af commit 7f8439a
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 9 deletions.
19 changes: 19 additions & 0 deletions src/runtime/base/builtin_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,25 @@ bool get_user_func_handler(CVarRef function, bool skip,
return false;
}

bool get_callable_user_func_handler(CVarRef function,
MethodCallPackage &mcp,
String &classname, String &methodname,
bool &doBind) {
bool ret = get_user_func_handler(function, true, mcp,
classname, methodname, doBind, false);
if (ret && mcp.ci->m_flags & (CallInfo::Protected|CallInfo::Private)) {
classname = mcp.getClassName();
if (!ClassInfo::HasAccess(classname, *mcp.name,
mcp.ci->m_flags & CallInfo::StaticMethod ||
!mcp.obj,
mcp.obj)) {
ret = false;
}
}

return ret;
}

Variant f_call_user_func_array(CVarRef function, CArrRef params,
bool bound /* = false */) {
MethodCallPackage mcp;
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/base/builtin_functions.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@ bool get_user_func_handler(CVarRef function, bool skip,
MethodCallPackage& mcp,
String &classname, String &methodname,
bool &doBind, bool warn = true);
bool get_callable_user_func_handler(CVarRef function,
MethodCallPackage& mcp,
String &classname, String &methodname,
bool &doBind);

Variant invoke_func_few_handler(void *extra, CArrRef params,
Variant (*few_args)(
Expand Down
39 changes: 31 additions & 8 deletions src/runtime/ext/ext_fb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -839,20 +839,43 @@ Array f_fb_call_user_func_safe(int _argc, CVarRef function,
return f_fb_call_user_func_array_safe(function, _argv);
}

static Variant fb_call_user_func_safe(CVarRef function, CArrRef params,
bool &ok) {
MethodCallPackage mcp;
String classname, methodname;
bool doBind;
if (get_callable_user_func_handler(function,
mcp, classname, methodname, doBind)) {
ok = true;
if (doBind) {
FrameInjection::StaticClassNameHelper scn(
ThreadInfo::s_threadInfo.getNoCheck(), classname);
ASSERT(!mcp.m_isFunc);
return mcp.ci->getMeth()(mcp, params);
} else {
if (mcp.m_isFunc) {
return mcp.ci->getFunc()(mcp.extra, params);
} else {
return mcp.ci->getMeth()(mcp, params);
}
}
}
ok = false;
return null;
}

Variant f_fb_call_user_func_safe_return(int _argc, CVarRef function,
CVarRef def,
CArrRef _argv /* = null_array */) {
if (f_is_callable(function)) {
return f_call_user_func_array(function, _argv);
}
return def;
bool ok;
Variant ret = fb_call_user_func_safe(function, _argv, ok);
return ok ? ret : def;
}

Array f_fb_call_user_func_array_safe(CVarRef function, CArrRef params) {
if (f_is_callable(function)) {
return CREATE_VECTOR2(true, f_call_user_func_array(function, params));
}
return CREATE_VECTOR2(false, null);
bool ok;
Variant ret = fb_call_user_func_safe(function, params, ok);
return CREATE_VECTOR2(ok, ret);
}

///////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/ext/ext_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ bool f_is_callable(CVarRef v, bool syntax /* = false */,
String classname, methodname;
bool doBind;
ret = get_user_func_handler(v, true, mcp,
classname, methodname, doBind, false);
classname, methodname, doBind, false);
if (ret && mcp.ci->m_flags & (CallInfo::Protected|CallInfo::Private)) {
classname = mcp.getClassName();
if (!ClassInfo::HasAccess(classname, *mcp.name,
Expand Down

0 comments on commit 7f8439a

Please sign in to comment.