Bug 352630 - ASTInstructionCompiler should be pluggable to handle bytecode rewriting
Summary: ASTInstructionCompiler should be pluggable to handle bytecode rewriting
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-Debug-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-20 13:03 EDT by Rajeev Dayal CLA
Modified: 2011-08-04 14:44 EDT (History)
3 users (show)

See Also:


Attachments
Adds an extension point to contribute a factory that can generate ASTInstructionCompilers. It is queried in the ASTEvaluationEngine class. (15.23 KB, patch)
2011-07-20 13:06 EDT, Rajeev Dayal CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rajeev Dayal CLA 2011-07-20 13:03:45 EDT
Build Identifier: 20110615-0604

The ASTEvaluationEngine class is used to create compiled expressions that can be evaluated in a runtime context. First, an AST representing the expression is created, and then the AST is traversed, all the while generating a set of abstract instructions that correspond to specific JDI commands. The generation of instructions is performed by the ASTInstructionCompiler. 

There are cases where the default implementation of the ASTInstructionCompiler is insufficient. The problem occurs when bytecode rewriting takes place at runtime. In such a case, a given class' name may change, and so may the methods on it - as such, it's not possible to determine the method name and class name on which to invoke the method based on static information. In such cases, a more customized version of the ASTInstructionCompiler is needed.

One such example occurs in GWT, with the use of JavaScriptObjects (http://code.google.com/webtoolkit/doc/latest/DevGuideCodingBasicsOverlay.html). These are Java overlays over native JavaScript objects. To make them work in GWT's development mode (i.e. in a Java environment), bytecode rewriting is used. Using the JSOEval class (http://code.google.com/p/google-web-toolkit/source/browse/trunk/dev/core/src/com/google/gwt/core/ext/debug/JsoEval.java) in the target VM for a given class name and method name (as found in the source) will yield the result of a method invocation on a JavaScriptObject.

So, to support displaying the results of a method invocation on a JavaScriptObject in the Display or Expressions view of an Eclipse debug session, the standard method invocation needs to be translated to a call to JSOEval. To achieve this, we need to have a way to plug in a different ASTInstructionCompiler. 

This is a very important feature that is affecting a large number of GWT + Eclipse users, so a fix for this would be most welcome. The only workaround is to provide our own ASTInstructionCompiler in the same package space as the existing one, and then run Eclipse in development mode with this class available to every plugin. This is what we're recommending for our internal users of Eclipse right now.

I've attached a patch that introduces an extension point which is queried in the ASTEvaluationEngine class. It allows other plugins to contribute an implementation of a factory that produces ASTInstructionCompilers. This is just a first cut; I wanted to get opinions on it before delving too deeply into the perfect implementation. One issue is that the implementers of ASTInstructionCompilers are going to be using internal API.


Reproducible: Always

Steps to Reproduce:
N/A
Comment 1 Rajeev Dayal CLA 2011-07-20 13:06:47 EDT
Created attachment 200028 [details]
Adds an extension point to contribute a factory that can generate ASTInstructionCompilers. It is queried in the ASTEvaluationEngine class.
Comment 2 Michael Rennie CLA 2011-07-20 14:16:08 EDT
(In reply to comment #0)

> a first cut; I wanted to get opinions on it before delving too deeply into the
> perfect implementation. 
> 

I like the direction. It would allow for some interesting extensions to our evaluation system.

> One issue is that the implementers of
> ASTInstructionCompilers are going to be using internal API.

Not such a big issue, we can investigate introducing new APIs as needed.

One thought I had when looking at your patch was how we would resolve / handle compiler conflicts - i.e. how do we determine who wins if a contributed compiler and the default one would both apply to a given context?
Comment 3 Rajeev Dayal CLA 2011-07-20 14:23:14 EDT
> (In reply to comment #0)
> 
> > a first cut; I wanted to get opinions on it before delving too deeply into the
> > perfect implementation. 
> > 
> 
> I like the direction. It would allow for some interesting extensions to our
> evaluation system.
> 
> > One issue is that the implementers of
> > ASTInstructionCompilers are going to be using internal API.
> 
> Not such a big issue, we can investigate introducing new APIs as needed.
> 
> One thought I had when looking at your patch was how we would resolve / handle
> compiler conflicts - i.e. how do we determine who wins if a contributed
> compiler and the default one would both apply to a given context?

Thanks for the comments! You've got a good point there - maybe we can pass in the IJavaProject into the ASTInstructionCompilerFactory's creation method, and the implementers could determine whether or not they're applicable to the environment? I think we'd also have to introduce a priority attribute as well, to have some sort of tiebreaker. 

It would be nice if we could sort of  have two ASTInstructionCompilers sort of work in tandem, but I can't see how that would be possible.
Comment 4 Michael Rennie CLA 2011-07-20 14:53:10 EDT
(In reply to comment #3)
> You've got a good point there - maybe we can pass in
> the IJavaProject into the ASTInstructionCompilerFactory's creation method, and
> the implementers could determine whether or not they're applicable to the
> environment? I think we'd also have to introduce a priority attribute as well,
> to have some sort of tiebreaker. 
>

Worst-case scenario I think we would end up having user interaction to pick the compiler.
 
> It would be nice if we could sort of  have two ASTInstructionCompilers sort of
> work in tandem, but I can't see how that would be possible.

I agree, I don't see how (or why) we would want that, especially if there was no way to validate the shared / combined instruction sequence.
Comment 5 Rajeev Dayal CLA 2011-07-20 15:01:40 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > You've got a good point there - maybe we can pass in
> > the IJavaProject into the ASTInstructionCompilerFactory's creation method, and
> > the implementers could determine whether or not they're applicable to the
> > environment? I think we'd also have to introduce a priority attribute as well,
> > to have some sort of tiebreaker. 
> >
> 
> Worst-case scenario I think we would end up having user interaction to pick the
> compiler.

That sounds good. I could add this functionality to the existing patch - or do you think it would be better to introduce this in a separate patch? I was thinking of adding a project-level setting under Java->Debug that allows the user to choose a particular evaluation engine.

> 
> > It would be nice if we could sort of  have two ASTInstructionCompilers sort of
> > work in tandem, but I can't see how that would be possible.
> 
> I agree, I don't see how (or why) we would want that, especially if there was
> no way to validate the shared / combined instruction sequence.
Comment 6 Rajeev Dayal CLA 2011-07-26 08:35:47 EDT
Ping - any additional thoughts? I'm going to be out of town for a couple of weeks, but once I"m back, I'd love to get started on the second iteration of the patch.
Comment 7 Michael Rennie CLA 2011-07-27 10:20:12 EDT
(In reply to comment #5)

>I was thinking of adding a project-level setting under Java-
>Debug that allows the user to choose a particular evaluation engine.

That would be one way to do it. Perhaps to start with we could could work out a way to pass the context to the contributors and have them answer yes / no to if they apply. I think in most cases there will be no conflicts, but in the event there is we could investigate prompting the user somehow or providing a preference as you mentioned.