Re: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Jeremias Maerki
Hi Nils

The idea would have been that you get the Handler by using
org.apache.fop.apps.Fop.getDefaultHandler() not by working directly on
the FOTreeBuilder:
https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk/src/java/org/apache/fop/apps/Fop.java

Any particular reason why you chose this approach?

Committers, I don't see a problem with what Nils proposes. Does anyone
else? If not, I can do the change next week. If Nils already has a patch
for us, even better. :-)

On 26.06.2005 06:43:22 Nils Meier wrote:
 Hi
 
 I've tried to use FOP for this scenario :
 
  + I have a docbook DOM tree
  + I send it through a transformer using docbook-xsl to generate fo
  + The output of that transformation 'is piped' into the result 
 new SAXResult(new FOTreeBuilder)
 
 so no files are generated - the FOTreeBuilder is fed directly
 from an in-memory DOM tree.
 
 Now the subtypes inheriting from FONode contain an identity
 check in:
 
 protected void validateChildNode(Locator loc, String nsURI, String
 localName) throws ValidationException {
 if (nsURI == FO_URI ...
 
 This fails because FO_URI != nsURI *but* FO_URI.equals(nsURI). This
 is so because the generated FO document was dynamically created in 
 my setup - apparently without String.intern() for the namespace URI.
 
 I'd propose to replace all those identity checks with .equals() to make
 this safe without relying on intern()-Strings - example:
 
 public class Root extends FObj {
 
  ...
   protected void validateChildNode(Locator loc, String nsURI, String
 localName) throws ValidationException {
 if (FO_URI.equals(nsURI)) {
   ...
 
 Does that sound reasonable? Affects around 30 types in
 org.apache.fop.fo.*
 
 Thanks
 Nils
 
 
 



Jeremias Maerki



Re: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Glen Mazza
Sounds reasonable to me--I think I should have done the validation that 
way to begin with (IIRC =='s are Not Good with Strings anyway for the 
very reason you gave.)  I am surprised this problem did not occur 
before.  I'll make the change next.


I am personally pleased that FOP 1.0 is now activatible by directly 
instantiating FOTreeBuilder, i.e. one can avoid the apps package 
completely.  This is because FOTreeBuilder now has all its needed 
business logic within it.  Still, you may wish to try this method[1] for 
your work (i.e., using the apps.Fop class instead of FOTreeBuilder 
directly).  FOP.getDefaultHandler()[2] should do the same thing as your 
new SAXResult(FOTreeBuilder), but FOTreeBuilder is within FOP's black 
box and may not be available/could be renamed, etc., in the future.


Also, I have to update our embed page -- apparently the links to our 
examples are no longer working.


[Team--viewSVN apparently does not have an annotate (can see line 
numbers) option -- G]


Thanks,
Glen

[1] *http://tinyurl.com/8ofpc*
[2] *http://tinyurl.com/c7c3z*


Nils Meier wrote:


Hi

I've tried to use FOP for this scenario :

+ I have a docbook DOM tree
+ I send it through a transformer using docbook-xsl to generate fo
+ The output of that transformation 'is piped' into the result 
   new SAXResult(new FOTreeBuilder)


so no files are generated - the FOTreeBuilder is fed directly
from an in-memory DOM tree.

Now the subtypes inheriting from FONode contain an identity
check in:

   protected void validateChildNode(Locator loc, String nsURI, String
localName) throws ValidationException {
   if (nsURI == FO_URI ...

This fails because FO_URI != nsURI *but* FO_URI.equals(nsURI). This
is so because the generated FO document was dynamically created in 
my setup - apparently without String.intern() for the namespace URI.


I'd propose to replace all those identity checks with .equals() to make
this safe without relying on intern()-Strings - example:

public class Root extends FObj {

...
 protected void validateChildNode(Locator loc, String nsURI, String
localName) throws ValidationException {
   if (FO_URI.equals(nsURI)) {
 ...

Does that sound reasonable? Affects around 30 types in
org.apache.fop.fo.*

Thanks
Nils





 





Re: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Glen Mazza

Andreas L. Delmelle wrote:


-Original Message-
From: Jeremias Maerki [mailto:[EMAIL PROTECTED]

Committers, I don't see a problem with what Nils proposes. Does anyone
else? If not, I can do the change next week. If Nils already has a patch
for us, even better. :-)

   



Well, the whole idea behind using interned strings and the == operator is
speed.
As you both are probably well aware, using .equals() on interned strings is
a lot slower than comparing them with ==.

 



Not necessarily--I suspect implementations of .equals() probably first 
check if they == each other, and if so quickly return true before trying 
a character-by-character compare.


You do have a point here though--this portion of the code is heavily 
activated.  What I don't understand is why this problem hasn't happened 
before.  If I run command-line, or even embedded, the String for the 
namespaceURI normally ends up being shared with the static FO_URI String 
created earlier.  It's probably a poor programming practice to rely on 
that fact, but I still wonder why Nils' namespaceURI  FOP's FO_URI here.



It may affect 'only' 30 types, but of these types, many can have an
unlimited number of instances or children --think of fo:block/fo:inline...--
so this means that such comparisons could be made hundreds (maybe even
thousands) of times.
 



Another option:  validateChildNode() is called from only one place, 
FOTreeBuilder.startElement().  At that point, we can also feed vcN() the 
parameter namespaceURI.intern() instead of just namespaceURI.  This 
could be slightly faster for some VCN()'s that compare against multiple 
URI's--but I would think .intern() is much slower than .equals() for the 
reason given above.


Glen



RE: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Andreas L. Delmelle
 -Original Message-
 From: Glen Mazza [mailto:[EMAIL PROTECTED]


Hi Glen,

snip /

 Another option:  validateChildNode() is called from only one place,
 FOTreeBuilder.startElement().  At that point, we can also feed vcN() the
 parameter namespaceURI.intern() instead of just namespaceURI.  This
 could be slightly faster for some VCN()'s that compare against multiple
 URI's--but I would think .intern() is much slower than .equals() for the
 reason given above.

The other idea to consider is the impact on memory. The string values of
interned strings are only stored once. There is indeed the overhead of a
call to .intern(), but the workings of that method will be nearly as
optimized as the .equals() method. Look up the string value in a hashtable:
if it doesn't exist, create a new one and return an internalized reference
to the value that's already stored in that hashtable. If it does, just
return the reference.
The source string value is immediately discarded in any case, only the
reference is kept.

The benefit of interning can be most appreciated in cases where the strings'
lengths are long enough --exceed the size of a reference-- AND the number of
them being created is large enough. Both are the case for a lot of namespace
URIs, and node or attribute names.
This is precisely the reason why the SAX parser feature for string
interning --http://xml.org/sax/features/string-interning -- defaults to
'true' in Xerces-J (and can't be set to 'false').

To put it quite bluntly: my concern is that we would essentially be adapting
our code to make it possible for people to use it to waste resources. Feed
it interned strings and it will work. Why would one really want to create a
separate string object for all occurences of a given namespace URI in a
random document, and at the very same time expect us to take into account
that they didn't intern those strings themselves...

I still think Nils would gain more by manipulating his setup so that these
types of strings are already interned, more than we would gain by changing
our code to allow for them to be 'just' strings.


Cheers,

Andreas



Re: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Jeremias Maerki

On 26.06.2005 14:41:13 Glen Mazza wrote:
snip/
 Well, the whole idea behind using interned strings and the == operator is
 speed.
 As you both are probably well aware, using .equals() on interned strings is
 a lot slower than comparing them with ==.
 
   
 
 
 Not necessarily--I suspect implementations of .equals() probably first 
 check if they == each other, and if so quickly return true before trying 
 a character-by-character compare.

Glen is right. java.lang.String.equals() checks == as the first
statement. So this change shouldn't have a big impact on performance. It'
just an additional method call (which might even be inlined by the JIT).


Jeremias Maerki



RE: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Andreas L. Delmelle
 -Original Message-
 From: Jeremias Maerki [mailto:[EMAIL PROTECTED]
 

Hi,

 Glen is right. java.lang.String.equals() checks == as the first
 statement. So this change shouldn't have a big impact on performance. It'
 just an additional method call (which might even be inlined by the JIT).

Well... (see my earlier response)

Somehow this s*** don't smell right. Whatch'all been eating?

Again, no veto, but I AM going to force you to give it some thought :-)

Cheers,

Andreas



Re: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Glen Mazza

Jeremias Maerki escribió:


On 26.06.2005 14:41:13 Glen Mazza wrote:
snip/
 


Well, the whole idea behind using interned strings and the == operator is
speed.
As you both are probably well aware, using .equals() on interned strings is
a lot slower than comparing them with ==.



 

Not necessarily--I suspect implementations of .equals() probably first 
check if they == each other, and if so quickly return true before trying 
a character-by-character compare.
   



Glen is right. java.lang.String.equals() checks == as the first
statement. So this change shouldn't have a big impact on performance. It'
just an additional method call (which might even be inlined by the JIT).


Jeremias Maerki


 



Thanks for checking.  BTW, Jeremias--the recent warning you added to the 
code on ignoring an span attribute on an fo:static-content descendant.  
Keep in mind, it may end up *not* being ignored for three reasons:  (1) 
layout may someday allow fo:static-content to be redirected to the 
fo:region-body (where span values become relevant), although FOP 
currently raises an error when that occurs; (2) There are some XSL 
functions which allow you to reference the property value on that FO; 
and (3) the span attribute could be inherited by 
fo:instream-foreign-object that an fo:block encloses.  Personally, I 
think this warning is not really needed (it is a given from the spec 
that multiple columns aren't supported in side regions), or would better 
be placed in layout (query the span attribute from SCLM and complain if 
not 1 or all.)


Glen



Re: problem with identity comparison in types inheriting from org.apache.fop.fo.FONode

2005-06-26 Thread Nils Meier
Hi 

thanks for the quick response from everyone.

 Sounds reasonable to me--I think I should have done the validation
 that way to begin with (IIRC =='s are Not Good with Strings anyway

yes, I think so as well. Since String.equals() does the == check
first anyways there shouldn't be a performance impact. Using
intern() by FO internally might or might not be beneficial
as mentioned. Upfront cost vs. saving memory and comparison effort.

 I am personally pleased that FOP 1.0 is now activatible by directly 
 instantiating FOTreeBuilder, i.e. one can avoid the apps package 
 completely.  This is because FOTreeBuilder now has all its needed 
 business logic within it.  

That was exactly my thinking - I'm planning to embed FOP as well
as xsl-docbook. Going through apps.FOP forces me to package
stuff that I don't really need since it seems to be designed
around running it from the command-line (forcing dependencies
to CommandLineOptions and the avalon stuff).

So embedding something like the FOTreeBuilder that operates
on the xsl level is more intuitive.

 but FOTreeBuilder is within FOP's
 black box and may not be available/could be renamed

agreed but for an embedded solution that's a risk I'm 
willing to take :) It would be nice if the layer below
apps.Fop was a public supported API actually.

Thanks for considering that change
Regards
Nils