-
Notifications
You must be signed in to change notification settings - Fork 29
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
alternative for #46, hybrid approach: #47
Conversation
- toplevel method definition - closures
- selective interpretation for non-abstract statements - then profile on toplevel `MethodInstance`
… by `profile_toplevel!`
Codecov Report
@@ Coverage Diff @@
## master #47 +/- ##
==========================================
- Coverage 85.69% 80.15% -5.54%
==========================================
Files 9 18 +9
Lines 706 1401 +695
==========================================
+ Hits 605 1123 +518
- Misses 101 278 +177
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
okay, this is in pretty good shape, now |
|
||
# adapted from https://github.com/JuliaDebug/JuliaInterpreter.jl/blob/2f5f80034bc287a60fe77c4e3b5a49a087e38f8b/src/interpret.jl#L188-L199 | ||
# works as `JuliaInterpreter.evaluate_call_compiled!`, but special cases for TypeProfiler.jl added | ||
function evaluate_call_recurse!(interp::ActualInterpreter, frame::Frame, call_expr::Expr; enter_generated::Bool=false) |
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.
Today I noticed that defining this method triggers more than 500 invalidations if you've loaded Revise. (In contrast, extending step_expr!
only triggered 8 invalidations, though if one gets fixed it might alter the count for the other.) We can live with that, but it might also be worth considering whether there are alternatives:
- define some inner call in JuliaInterpreter's
evaluate_call_recurse!
method that obviates the need for this extension (i.e., can we add a branch that handlesinclude
s?) - move the definition of
ConcreteInterpreter
and this second method to JuliaInterpreter.jl (I suspect that is not a good solution) - always use runtime dispatch to call
evaluate_call_recurse!
MethodInstance
eventually closes #26 , #46