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.
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
Optimize library model lookups #265
Conversation
@@ -208,7 +208,7 @@ private NullnessStore lambdaInitialStore( | |||
NullnessStore environmentMapping = | |||
Objects.requireNonNull( | |||
environmentNullness.getEnvironmentMapping(underlyingAST.getLambdaTree()), | |||
"no environment stored for lambda " + underlyingAST.getLambdaTree()); | |||
"no environment stored for lambda"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
separate, tiny optimization: I saw the string conversion of the lambda tree showing up in profiles
bfe650d
to
05b2707
Compare
@lazaroclapp this is ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is merged already, but I just went through it. Looks pretty cool to me.
private static Symbol.MethodSymbol lookupHandlingOverrides( | ||
Symbol.MethodSymbol symbol, Types types, NameIndexedMap<Boolean> optLookup) { | ||
if (optLookup.nameNotPresent(symbol)) { | ||
// no model matching the method name, so we don't need to check for overridden methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice optimization 👍
NullAway was spending a lot of time checking if we had library models for methods. This PR optimizes the process in two ways:
Name
objects, as converting aName
to aString
is actually costly.