This repository was archived by the owner on Apr 14, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 133
Cache function eval result per argument set #988
Merged
Merged
Changes from 39 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
8bca56b
Fix #668 (partial)
a731c2a
Merge branch 'master' of https://github.com/Microsoft/python-language…
069910b
Merge branch 'master' of https://github.com/Microsoft/python-language…
7ffc9db
Tests
6303c45
Revert "Tests"
945451c
Merge branch 'master' of https://github.com/Microsoft/python-language…
034e437
Merge branch 'master' of https://github.com/Microsoft/python-language…
f997d68
Merge branch 'master' of https://github.com/Microsoft/python-language…
0e4ff95
Merge branch 'master' of https://github.com/Microsoft/python-language…
878c8c5
Merge branch 'master' of https://github.com/Microsoft/python-language…
35e1033
Merge branch 'master' of https://github.com/Microsoft/python-language…
1073711
Merge branch 'master' of https://github.com/Microsoft/python-language…
86028da
Merge branch 'master' of https://github.com/Microsoft/python-language…
4d4bca7
Merge branch 'master' of https://github.com/Microsoft/python-language…
ac04f76
Merge branch 'master' of https://github.com/Microsoft/python-language…
bab03da
Merge branch 'master' of https://github.com/Microsoft/python-language…
85ec685
Merge branch 'master' of https://github.com/Microsoft/python-language…
be7466d
Merge branch 'master' of https://github.com/Microsoft/python-language…
01781c0
Exp
f544f8a
Limit concurrency
01bd719
Concurrency limit
2f8e564
Drop cache after analysis
9a933bd
Merge branch 'master' of https://github.com/Microsoft/python-language…
4ad56bb
Fix regression
c280c55
Merge branch 'master' of https://github.com/Microsoft/python-language…
b21b8b8
Merge branch 'master' of https://github.com/Microsoft/python-language…
babaee4
Merge branch 'master' of https://github.com/Microsoft/python-language…
092efd6
Merge branch 'master' of https://github.com/Microsoft/python-language…
b97594d
Merge branch 'master' of https://github.com/Microsoft/python-language…
2a50eb9
Fix test
9c7f58d
Merge branch 'master' of https://github.com/Microsoft/python-language…
00faa5f
Merge branch 'master' of https://github.com/Microsoft/python-language…
4c8aa92
Merge branch 'master' of https://github.com/Microsoft/python-language…
1a67bc8
Merge branch 'master' of https://github.com/Microsoft/python-language…
7a13215
Merge branch 'master' of https://github.com/Microsoft/python-language…
3d907d8
Merge branch 'master' of https://github.com/Microsoft/python-language…
e5a225f
Merge branch 'master' of https://github.com/Microsoft/python-language…
d5ba0db
Cache argument eval
cd67de7
Fix hash calculation
87b0d33
Merge branch 'master' of https://github.com/Microsoft/python-language…
ab6da4d
Fix exception when there is a space before triple quote
5747ba4
Fix exception
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -196,7 +196,7 @@ public IMember GetValueFromFunctionType(IPythonFunctionType fn, IPythonInstance | |
// make sense to evaluate stubs since they already should be annotated. | ||
if (fn.DeclaringModule is IDocument doc && fd?.Ast == doc.GetAnyAst()) { | ||
// Stubs are coming from another module. | ||
return TryEvaluateWithArguments(fn.DeclaringModule, fd, args); | ||
return TryEvaluateWithArguments(fn, args); | ||
} | ||
return UnknownType; | ||
} | ||
|
@@ -207,9 +207,26 @@ public IMember GetValueFromProperty(IPythonPropertyType p, IPythonInstance insta | |
return instance.Call(p.Name, ArgumentSet.Empty); | ||
} | ||
|
||
private IMember TryEvaluateWithArguments(IPythonModule module, FunctionDefinition fd, IArgumentSet args) { | ||
|
||
private readonly Dictionary<int, IMember> _argEvalCache = new Dictionary<int, IMember>(); | ||
|
||
private IMember TryEvaluateWithArguments(IPythonFunctionType fn, IArgumentSet args) { | ||
var name = fn.DeclaringType != null ? $"{fn.DeclaringModule.Name}.{fn.Name}" : fn.Name; | ||
var argHash = args | ||
.Arguments | ||
.Select(a => a.Name.GetHashCode() ^ 397 * (a.Value?.GetHashCode() ?? 0)) | ||
.Aggregate(0, (current, d) => 31 * current ^ d); | ||
var key = fn.DeclaringModule.Name.GetHashCode() ^ name.GetHashCode() ^ (397 * argHash); | ||
|
||
if (_argEvalCache.TryGetValue(key, out var result)) { | ||
return result; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't hash collisions here make some unrelated functions have the same evaluation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Functions have fully qualified names like module.class.method which should have unique hashes |
||
} | ||
|
||
var fd = fn.FunctionDefinition; | ||
var module = fn.DeclaringModule; | ||
|
||
// Attempt to evaluate with specific arguments but prevent recursion. | ||
IMember result = UnknownType; | ||
result = UnknownType; | ||
if (fd != null && !_callEvalStack.Contains(fd)) { | ||
using (OpenScope(module, fd.Parent, out _)) { | ||
_callEvalStack.Push(fd); | ||
|
@@ -222,6 +239,8 @@ private IMember TryEvaluateWithArguments(IPythonModule module, FunctionDefinitio | |
} | ||
} | ||
} | ||
|
||
_argEvalCache[key] = result; | ||
return result; | ||
} | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This might be affected by #979 where the DeclaringModule of a function I had was
null
, so all functions of that style with the same name would get cached together.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.
Yes, but it is a bug. I'll work on it right after this. All types should have declaring module, there are many many places where we would crash otherwise.