Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache result of slow function resolution on Any that is present on a hot path #6536

Merged
merged 4 commits into from
May 4, 2023
Merged

Cache result of slow function resolution on Any that is present on a hot path #6536

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
317ef5c
Cache resolution of symbol for Any
hubertp May 3, 2023
06f98f7
cache unresolvedsymbol
hubertp May 3, 2023
f1f4b31
missing any condition in uncached
hubertp May 3, 2023
cafffaa
Rewrite caching of the resolved function
hubertp May 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 45 additions & 9 deletions engine/runtime/src/main/java/org/enso/interpreter/node/callable/InvokeMethodNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@

@ImportStatic({HostMethodCallNode.PolyglotCallType.class, HostMethodCallNode.class})
public abstract class InvokeMethodNode extends BaseNode {
protected static final int CACHE_SIZE = 10;
private @Child InvokeFunctionNode invokeFunctionNode;
private final ConditionProfile errorReceiverProfile = ConditionProfile.createCountingProfile();
private @Child InvokeMethodNode childDispatch;
Expand Down Expand Up @@ -104,30 +105,46 @@ public void setTailStatus(TailStatus tailStatus) {
public abstract Object execute(
VirtualFrame frame, State state, UnresolvedSymbol symbol, Object self, Object[] arguments);

@Specialization(guards = {"typesLibrary.hasType(self)", "!typesLibrary.hasSpecialDispatch(self)"})
Object doFunctionalDispatch(
@Specialization(guards = {
"typesLibrary.hasType(self)",
"!typesLibrary.hasSpecialDispatch(self)",
"cachedSymbol == symbol",
"cachedSelfTpe == typesLibrary.getType(self)",
"function != null"
}, limit = "CACHE_SIZE")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limit is good.

Object doFunctionalDispatchCachedSymbol(
VirtualFrame frame,
State state,
UnresolvedSymbol symbol,
Object self,
Object[] arguments,
@CachedLibrary(limit = "10") TypesLibrary typesLibrary,
@Cached MethodResolverNode methodResolverNode) {
@Cached MethodResolverNode methodResolverNode,
@Cached("symbol") UnresolvedSymbol cachedSymbol,
@Cached("typesLibrary.getType(self)") Type cachedSelfTpe,
@Cached("resolveFunction(cachedSymbol, cachedSelfTpe, methodResolverNode)") Function function) {
return invokeFunctionNode.execute(function, frame, state, arguments);
}

Type selfTpe = typesLibrary.getType(self);
Function function = methodResolverNode.expectNonNull(self, selfTpe, symbol);
Function resolveFunction(UnresolvedSymbol symbol, Type selfTpe, MethodResolverNode methodResolverNode) {
Function function = methodResolverNode.execute(selfTpe, symbol);
if (function == null) {
return null;
}

RootNode where = function.getCallTarget().getRootNode();
// If both Any and the type where `function` is declared, define `symbol`
// and the method is invoked statically, i.e. type of self is the eigentype,
// then we want to disambiguate method resolution by always resolved to the one in Any.
if (where instanceof MethodRootNode node && typeCanOverride(node, EnsoContext.get(this))) {
Function anyFun = symbol.getScope().lookupMethodDefinition(EnsoContext.get(this).getBuiltins().any(), symbol.getName());
EnsoContext ctx = EnsoContext.get(this);
if (where instanceof MethodRootNode node && typeCanOverride(node, ctx)) {
Type any = ctx.getBuiltins().any();
Function anyFun = symbol.getScope().lookupMethodDefinition(any, symbol.getName());
if (anyFun != null) {
function = anyFun;
}
}
return invokeFunctionNode.execute(function, frame, state, arguments);
return function;
}

private boolean typeCanOverride(MethodRootNode node, EnsoContext ctx) {
Expand All @@ -137,13 +154,32 @@ private boolean typeCanOverride(MethodRootNode node, EnsoContext ctx) {
Type warning = builtins.warning();
Type panic = builtins.panic();
return methodOwnerType.isEigenType()

&& builtins.nothing() != methodOwnerType
&& any.getEigentype() != methodOwnerType
&& panic.getEigentype() != methodOwnerType
&& warning.getEigentype() != methodOwnerType;
}

@Specialization(
replaces = "doFunctionalDispatchCachedSymbol",
guards = {"typesLibrary.hasType(self)", "!typesLibrary.hasSpecialDispatch(self)"})
Object doFunctionalDispatchUncachedSymbol(
hubertp marked this conversation as resolved.
Show resolved Hide resolved
VirtualFrame frame,
State state,
UnresolvedSymbol symbol,
Object self,
Object[] arguments,
@CachedLibrary(limit = "10") TypesLibrary typesLibrary,
@Cached MethodResolverNode methodResolverNode) {
Type selfTpe = typesLibrary.getType(self);
Function function = resolveFunction(symbol, selfTpe, methodResolverNode);
if (function == null) {
throw new PanicException(
EnsoContext.get(this).getBuiltins().error().makeNoSuchMethod(self, symbol), this);
}
return invokeFunctionNode.execute(function, frame, state, arguments);
}

@Specialization
Object doDataflowError(
VirtualFrame frame,
Expand Down