I agree it should be instance based, I originally wanted it to be able to 'branch' with different runs and preserve or clear stats. The reason for clearing the stats for each optimization is that I was looking at the output of each run of the main compile loop (hence, why it was called Tracing and not Stats), so I could see the progressive effect of each loop. So for example, one each of the inliner, I would see how many methods there were, how many got inlined, and for what reason.
The problem with not clearing the stats is some of the counters will be multiplied increments. So for example, the COUNT_METHOD one would not show the true number of methods in the AST, but a multiple. And some optimizations which are rejected in one or more passes, but later allowed to proceed might register multiple 'failure' counts. However, I think a case could be made that the tracing is rather low level and perhaps not as useful/important as summary statistics at the end of the compile, which might be useful for developers "2312 of your methods were inlined, 137 because they used recursion", etc. Is there any benefits this information can provide for SOYC reports? On 2009/09/14 20:08:09, Lex wrote: > This looks good except that it's all static state and static methods. I believe > Tracing should have instance state and instance methods. It's not out of the > question to run two compiler instances in separate threads. > That means that an instance of Tracing needs to be stashed somewhere, in the > worst case in JProgram. It would be better if it could be a local variable in > some method in JavaToJavaScriptCompiler, though, and then passed to > MethodInliner as part of the call to MethodInliner.exec. > The counting should be extremely cheap, so I don't see a big need to disable the > actual counting. For outputting the data, what would you think of making it a > compiler artifact in the extras output? To see how to do that, look at > JavaToJavaScriptCompiler.makeSoycArtifact. > Finally, I don't understand the pattern of clearing out the stats along the way. > I would have thought that the counts would be accumulated for an entire > compiler run and then output at the end. So, I don't understand the true goal, > but perhaps MethodInliner could make a separate Tracing instance rather than > clearing out the main one? Or, maybe MethodInliner should simply have some > local variables to do its counts? > http://gwt-code-reviews.appspot.com/64813/diff/1/2 > File dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java (right): > http://gwt-code-reviews.appspot.com/64813/diff/1/2#newcode529 > Line 529: Tracing.clearStats(); > I'm surprised that the inliner would clear the compiler-wide stats. Doesn't > that mean that all stats until now are forgotten? > http://gwt-code-reviews.appspot.com/64813/diff/1/3 > File dev/core/src/com/google/gwt/dev/jjs/impl/Tracing.java (right): > http://gwt-code-reviews.appspot.com/64813/diff/1/3#newcode56 > Line 56: Boolean.parseBoolean(System.getProperty("gwt.tracestats")); > Instead of a property, how about logging at a low log level? Properties are at > a very large scope. > http://gwt-code-reviews.appspot.com/64813/diff/1/3#newcode76 > Line 76: System.err.println(results.toString()); > This would seem better to go to a logger, whether or not a property is used to > enable or disable tracing. > Best of all would be if it made a compiler artifact in the -aux directory, but I > suppose that's too much to get into right now. http://gwt-code-reviews.appspot.com/64813 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
