On Jan 6, 2014, at 12:58 PM, adrian.c...@sandglass-software.com wrote: > Quoting "David E. Jones" <d...@me.com>: > >> >> On Jan 6, 2014, at 12:26 PM, adrian.c...@sandglass-software.com wrote: >> >>> There are two problems with using DOM trees in OFBiz: >>> >>> 1. They consume a lot of memory. Keep in mind the entire XML file is kept >>> in memory, not just the bits you are interested in. >> >> The parsed version is kept in memory, that is true, but for each >> screen/form/etc you just keep the node object for the top-level element. >> Under that element the only stuff it would keep around by default that you >> don't need/want (but could be eliminated with a little code) is comment >> elements. >> >>> 2. They are not thread-safe. >> >> They should be used read-only, so unless someone does something funny thread >> safety isn't an issue. On that note, the same is true for the shared objects >> in the current OFBiz widget implementations. > > > And that is the problem we have now. Many contributors/committers will not > understand what should and shouldn't be done with DOM nodes, and consequently > they will do things they shouldn't. History has proven that, given the > opportunity, OFBiz developers will "do something funny" with the code. > > Over the last few years, the framework code-base has been moving toward > immutable shared objects, and I think that is the safest strategy to use. We > have fixed numerous bugs going that route. > > I understand your perspective and I appreciate it - make code thread-unsafe > for the sake of convenience. But that strategy exists currently in OFBiz > legacy code and it keeps hurting us. > > What I find interesting about your proposal is how similar it is to what I > proposed years ago - make the widgets lightweight representations of their > XML files. The screen widget models would contain Lists of model Elements, > and the model Elements would contain Maps of Element Attributes. At the time > you rejected that idea - you said performance would suffer by making the > widget models that generic. Now you're suggesting we skip the models > altogether and just use the DOM tree. So, what has changed your opinion?
My concerns about performance were clearly overblown. After trying the direct node tree and profiling performance I now know it doesn't result in much of a performance difference. In other words, the direction you proposed is a good one and would simplify code, and letting FTL walk the node tree instead of Java code is just as fast and that little step eliminates a bunch more framework code and at the same time makes it far more flexible and extensible. IMO the minor impact on runtime performance is well worth the additional flexibility. It makes the tools usable in so many more situations where you would otherwise need to drop to plain FTL instead of using the form widget or certain parts of the screen widget, and addresses many of the concerns that push people to using other web UI frameworks. BTW, from profiling work in Moqui Framework I found it was really other stuff that killed performance, little things you wouldn't expect sometimes like how long it takes to call System.currentTimeMillis(). FTL and Groovy are impressively fast for what they do and JVMs do so much better with Map and List operations (iteration, add/put, get, etc) than they did when OFBiz was young, especially once the JIT natively compiles that frequently used code, that the overhead barely shows up in profiling results. BTW2, in early profiling a few years ago with Groovy there was noticeable overhead from on the fly object and method binding and such, but much of this changed with Groovy 2 when they incorporated features from Groovy++ to more quickly bind based on types declared or casted in the code, so it runs quite a bit faster with a little more effort in declaring types (as opposed to using Object or def all over the place). There is still somewhat of a performance difference in doing simplified method names (like foo.bar versus foo.getBar()), but even that doesn't show up in profilers like it used to with Groovy (so all the effort I put into Moqui making such changes don't matter so much any more :) ). One thing about thread safety with this approach is that there is SO little code, the vast bulk of it is in the FTL macros. The framework code just has to parse the XML file, grab the desired top-level node, wrap it so FTL can use it (in Moqui I implemented a simple wrapper for the Groovy Node object to implement the FTL node interface), then pass it to FTL when rendering the template. That's really it. The FTL macros will also need framework code to call back into to do things like including forms, FTL files, other screens, etc... but that is also pretty small/simple code. I was hesitant about this approach at first, in spite of the great flexibility, but in Moqui it has worked well... the code for that is about 3 years old now with hundreds of screens running on various production instances (that I'm aware of anyway, it's like OFBiz in that I often don't find out about projects and even deployed instances either at all or until well after they are in production). -David
