Thanks for the info - that was informative!
I'm still not sold on keeping the DOM tree though. I seem to recall Adam
Heath doing a memory-use analysis a few years ago and he discovered that
even keeping a reference to a String from the DOM kept the whole tree in
memory. That is why he did all those String.intern()s in the models. I
can't be sure though, my memory is a bit foggy on that.
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 1/6/2014 5:49 PM, David E. Jones wrote:
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
