-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Feature/instrumentation api #1
Conversation
These are the changes that adapt JRuby's attachment system to use this new API https://github.com/jruby/jruby/compare/truffle-new-instrumentation. |
@chrisseaton I think your patch is outdated you need to use @Instrumentable instead of @GenerateWrapperNode in the latest revision. |
How do I give a node a tag now? I've got a field that tells me if a node is the first one on a line, and I'm calling that a statement. I used to return Do I need to instead have a |
@chrisseaton Yes: tags are now bound only (at "AST Engineering time") by annotating Node classes. The new Instrumentation APIs reduce functionality from the prototype in several ways, but this is the one with which I have been disagreeing most. I maintain that another (more dynamic) mechanism will have to be created to address situations like this one (and many others). |
I am not sure this is a proper definition of a statement. Do you have the notion of statement at all in Ruby?
Right. How much slower would it really be? Can you make measurements? Are there additional cases were you would need to split classes? If it turns out its too much then we need to go back to a dynamic mechanism for tagging. This is how SimpleLanguage does it: 5616185 . Tags are declared using annotations and are now bound to Java classes in order to solve mutability for tags so we can safely assume they are not changing per node class. @mlvdv We discussed this several times and as long as there is no use case that really needs "dynamic" tagging we should not have it in the API. |
Everything is just an expression in Ruby, so if I have a statement node then it's always just going to be a simple wrapper around another node. I'll do that for now and will see if it's the only complication. |
Ruby's attachments work with the latest version of Truffle now, but not with Graal yet. |
I’ve been arguing for two alternate views about tag binding time. Here's a recap: Too early (new): only at AST engineering time, only by language implementor (class annotations)
Reasonable (prototype): at AST creation time by language implementor and other tools
Most flexible (prototype): Any time from AST creation onward, by language and tool implementors
|
} | ||
|
||
@Before | ||
public void setupDebugger() throws IOException { |
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.
should this be named setupProfiler()
? Potential copy/paste artifact?
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.
Fixed in 6aaab06
Client AccessTCR: Need Jarda to speed up work on service lookup to simplify getting Instrumenter. The tags
Chris: added StatementNode around ExpressionNode? Is duplicating classes a problem?
Tags are stable or not? SourceSection are immutable. Changing a node (with different SourceSection) would trigger AST rescan. Michael: SourceSection well defined, tags are not - a policy decision? SourceSection created by parser - need to inject. Jarda: Are tags in annotations OK for majority and just a problem in corner cases? Michael: annotations are just wrong. Unproven tags vs. experiments: Jarda: restrict the API; Michael: don't restrict the users of the API. Chris: tags as strings. Tutorial for Instrument ProvidersMichael: Too complex to build an instrument. Singleton vs. InstrumentDescriptor. Michael: will press-on the debugger rewrite |
I got one more issue with the TruffleProfiler, not sure whether that is a general problem, but I think so. Could it be that this is missing Graal support? I tried and tried and always failed to get anything from the TruffleProfiler when running with SOMns. I guess, part of the problem is that such support would need to be in the Graal part of Truffle, i.e., the |
} | ||
|
||
static Instrumentable getInstrumentable(Node node) { | ||
Instrumentable instrumentable = node.getClass().getAnnotation(Instrumentable.class); |
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 seems to get only the annotations of the current class. Is that intentional?
I tried to annotate an Invokable
node class with InstrumentationTag.ROOT
and I was surprised that this is not recognized in the Method
and Primitive
subclasses.
This is especially surprising since this behavior seems to be different from the TruffleDSL annotations.
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.
Ok, this might be solved with removing InstrumentationTag.
…cerning: Move actions from Parser/ATG to NodeFactory Restructured DebugExprVarNode s.t. a type of a variable is already known while building the AST
Move actions from Parser/ATG to NodeFactory Restructured DebugExprVarNode s.t. a type of a variable is already known while building the AST
* Before: VMThread 000000000afdc2f0 STATUS_IN_SAFEPOINT VMThread 00007f3e6c000b60 STATUS_IN_SAFEPOINT * After: "main" #1 tid=0x000000000ab2b2f0 state=TIMED_WAITING "nfi-gc" #4 daemon tid=0x00007f9dec000b60 state=WAITING * HotSpot: "main" #1 prio=5 os_prio=0 tid=0x00007fac2400a800 nid=0x4c2f waiting on condition [0x00007fac2b591000] "nfi-gc" #20 daemon prio=5 os_prio=0 tid=0x00007f548c472800 nid=0x74a9 in Object.wait() [0x00007f5469518000] * Omit the VMThread status since it's always STATUS_IN_SAFEPOINT.
Here it is: the new instrumentation framework. It is the result of several sessions/discussions/walkthroughs with [~mlvdv] and [~jtulach].
Reading Recommendation:
If you are not interested in the details skip over to the example uses in com.oracle.truffle.api.instrumentation.examples they should give the best idea.
For everybody else:
There is also a trace mode (InstrumentationHandler#TRACE) that should make it easier to understand what is going on.
New and Noteworthy:
o) Filter expression language for source sections with tags that do all the syntax probing/synchronization automatically and language agnostically (see SourceSectionFilter, Instrumenter).
o) Everything is now lazy. Wrappers are created only for places which are actually instrumented and which were executed at least once. Wrappers are updated lazy on next/first execution and removed if not needed anymore.
o) Fixed set of instrumentation tags for guest languages to adopt.
o) Event bindings for language implementations that have special privileges to implement language features with the framework (see InstrumentationLanguage).
o) Creation/disposal of instruments in PolyglotEngine and out,err,in for instrumentations.
o) Only instrumented RootNode instances are tracked. No tracking of anything else.
o) Easy dispose of event bindings. Automatic binding dispose for instrumentations on removal.
o) Language agnostic annotation processor for generating wrappers (see @Instrumentable, SL implementation)
Specific review questions:
A) Which use cases do you have that are not yet covered in the API?
B) Which use cases do you see that are now covered which were not covered before?
C) How can the implementation/API be improved/simplified?
D) How could the naming be improved? (consistency, intuition, the hardest part :-) )
E) Do you see issues in evolving of this API?
F) How can the documentation be improved?
G) Do you have ideas for tools (try to build them)?
Open/Known Issues:
a) The Registration annotation processor does not work with incremental compiling.
Migration strategy/Next steps:
After taking your feedback into account I plan to push this through (if nobody objects). The old instrumentation framework will then be deprecated and removed later on. Old and new can live side-by-side for a while during adoption of the languages.
Thanks in advance to the reviewers.