On Mon, Apr 6, 2009 at 8:48 PM, Freeland Abbott <[email protected]> wrote: >> The main thing is that many problems are still logged via TreeLogger >> and not stored in the ProblemReport. Shouldn't we jump over >> consistently to the new system rather than have a mix? Are there any > > Probably; I was worried that I might lose something valuable (for example, > if we put a log message in ProblemReport and then drop it as above). That > shouldn't be an issue if we're selective about which TreeLogger logs > migrate; I just wanted to get the focal thing working first. Happy to do a > second revision.
Cool. By all means mail out any cases that look questionable. Decisions about log messages are sure to get lots of, um, input. >> Also, the new "visited" field is redundant with the typeToTyeInfoComputed >> map. >> It looks like you ran into some recursion issue somewhere. We should >> fix that problem by improving the existing short-circuiting via TICs, not >> by adding a new and subtly different field. > > Yeah, I suspected this would come up. I'll take a closer look at using TIC > instead. > > There's no special recursion I had to provoke; if you put logging in instead > of my short circuit, I think DynaTable reconsiders java.lang.String > something like 23 times before reaching TIC shortcircuits. My patch saved > almost 10% time for DynaTable, I think due to this earlier shortcircuit. > (Sorry, should have bragged on that in the first posting.) Wow, great! Hopefully we can make this same thing happen with the TIC shortcut. Probably a TIC-based short circuit could be inserted at the same place a visited shortcut is. >> 1. In checkSubtypes(), add a problem for each candidate that is >> rejected. For example, "tried subtype Foo but it was not >> instantiable". > > This one I think we don't lose, because of the context in the > ProblemReport. We get messages like "com.foocorp.SubFoo had no default > instructor (reached via com.foocorp.Foo)". The complete chain of subtypes, > yes, you'd need debug logging to see. Oh, I overlooked the "reached via" part. That means the information is still there, which was my main concern. I do still wonder about the exact user experience reading one of these logs. Could you post one the next time you have one handy? It seems like they will see, "Foo is not instantiable and has no instantiable subclasses". To find out the subclasses, they have to do a search to find all the "reached via Foo". It seems slightly better, but only slightly, if instead there is a list right there saying "Tried subclass Bar but it's not instantiable; tried subclass Baz but it's not instantiable". They would then have to search for Bar or Baz, etc., whichever ones they expected to be serializable but weren't. >> To simplify the code and help with future developments, the errors >> could be stored in the TIC class instead of in ProblemReport. Simply >> add a field "List<String> problems". The two static methods that take >> a ProblemReport as an argument could instead take a List as an >> argument. The general rule is that TIC holds the information STOB >> deduces about a particular type. Surely it will work well to log >> the errors there directly. > > The reason I was hesitant here was memory pressure... I wasn't sure how > long-lived the STOB and TIC info was (yes, I should have worked it out). > The purpose to ProblemReport is to become garbage relatively quickly. A good thing to worry about. As it is, STOB actually doesn't live that long, and has about the life span of ProblemReport. The RPC generator does hang around, but it creates STOB instances on demand and then discards them once it gets the info out. Lex --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
