This is really cool, Bob! It adds a history to each AST node showing where it came from. Thus, you can track an individual node backwards through a long series of
Right now, the histories just hold a parent node and a string description of what changed. Given how it's laid out, though, it will be easy to add more information to these histories as the desire comes up. I see two issues with the patch as it stands that would be good to improve. First, I'm not sure that the soft reference for SourceInfo.getParents() is a net win. It looks similar to the parent chasing done in "fast union find", in which case the cache gives an algorithmic improvement. Without the cache, if you query all nodes for all parents it takes O(nodes*depth) time where depth is the average depth. With the cache, the depth doesn't matter, and it takes O(nodes) time for practical purposes. Besides, suppose the following micro-opt were in place: if a node has exactly one parent, then reuse whatever it returns for getParents() ? If most nodes have exactly one parent, this could be such a big win that the overhead from tracking the parents is not much more than the overhead from the SoftReference objects. That's not a big deal, though, just a thought. More substantively, SourceInfo.disableCollection is a mutable static field. It would be a lot nicer if this setting could be specified per compiler instance, via the settings object, instead of being held in a mutable field. That way we preserve the property that multiple GWT compiles can happen in multiple threads even if they have different settings. It's not a common use case, but it would be nice to keep it if possible. There are multiple ways to do this, and the precise way doesn't seem important. One way would be to have a DisabledSourceInfo subclass, and move the root classes like UNKNOWN over to the ever-growing JProgram. Small bug: SourceInfo.java:136 is missing a null check, causing Showcase not to compile Nits: GWTCompiler: needs a sort SourceInfoHistogram.java: blank line at 455 SourceInfoHistogram.java: HistogramData is empty and commentless. I'm sure it's a placeholder. A comment would be nice. MakeCallsStatic: spelling: degelgating JsArrayAccess, JsProgram, JsNameRef, JsInvocation, JsExpression, JsConditional, JsCatch, JsArrayAccess, GenerateJavaScriptLiterals: copyright year IEBlockSizeVisitor: "Maximum block size" seems hard to understand. Maybe "Decreases maximum block size" ? JsStaticEval: "Minimal execution order" I don't understand SourceInfo.makeChild has a "caller" var shadowing a field, yielding an Eclipse warning -Lex --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
