Re: svn commit: r629374 - /cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java

2008-05-08 Thread Alfred Nathaniel
On Wed, 2008-05-07 at 22:52 -0400, Joerg Heinicke wrote:
 On 30.03.2008 02:50, Joerg Heinicke wrote:
 
  Author: antonio
  Date: Tue Feb 19 22:42:45 2008
  New Revision: 629374
 
  URL: http://svn.apache.org/viewvc?rev=629374view=rev
  Log:
  Faster implementation.
  
  Saw this one only now ... I'm a bit concerned about the approach.
  
  First, do you really think this implementation is significantly faster? 
  Your implementation only caches the parser instance, you replace the 
  instantiation with ThreadLocal handling. Parsing itself should still be 
  the slowest part. How many Strings do you convert to SAX per thread? 
  Second, who cleans up the thread before it goes back to the thread pool?
  
  At the end is it really worth the ugly implementation?
  
  IMO it's a much better approach to make it a real Cocoon component 
  (Serializeable) instead and look up the SAXParser.
 
 Any opinions on this one?
 
 Joerg
 
 [1] http://marc.info/?l=xml-cocoon-cvsm=120348979305179w=4

What happens if an exception leaves the parser in a messy state when it
is reused?  Is there any rule which would guarantee that this should
never happen for the average SAXParser implementation?

Cheers, Alfred.



Re: svn commit: r629374 - /cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java

2008-05-07 Thread Joerg Heinicke

On 30.03.2008 02:50, Joerg Heinicke wrote:


Author: antonio
Date: Tue Feb 19 22:42:45 2008
New Revision: 629374

URL: http://svn.apache.org/viewvc?rev=629374view=rev
Log:
Faster implementation.


Saw this one only now ... I'm a bit concerned about the approach.

First, do you really think this implementation is significantly faster? 
Your implementation only caches the parser instance, you replace the 
instantiation with ThreadLocal handling. Parsing itself should still be 
the slowest part. How many Strings do you convert to SAX per thread? 
Second, who cleans up the thread before it goes back to the thread pool?


At the end is it really worth the ugly implementation?

IMO it's a much better approach to make it a real Cocoon component 
(Serializeable) instead and look up the SAXParser.


Any opinions on this one?

Joerg

[1] http://marc.info/?l=xml-cocoon-cvsm=120348979305179w=4


Re: svn commit: r629374 - /cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java

2008-03-30 Thread Joerg Heinicke

On 20.02.2008 01:42, [EMAIL PROTECTED] wrote:


Author: antonio
Date: Tue Feb 19 22:42:45 2008
New Revision: 629374

URL: http://svn.apache.org/viewvc?rev=629374view=rev
Log:
Faster implementation.


Saw this one only now ... I'm a bit concerned about the approach.

First, do you really think this implementation is significantly faster? 
Your implementation only caches the parser instance, you replace the 
instantiation with ThreadLocal handling. Parsing itself should still be 
the slowest part. How many Strings do you convert to SAX per thread? 
Second, who cleans up the thread before it goes back to the thread pool?


At the end is it really worth the ugly implementation?

IMO it's a much better approach to make it a real Cocoon component 
(Serializeable) instead and look up the SAXParser.


Joerg


Modified:

cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java

Modified: 
cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java
URL: 
http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java?rev=629374r1=629373r2=629374view=diff
==
--- 
cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java
 (original)
+++ 
cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java
 Tue Feb 19 22:42:45 2008
@@ -5,9 +5,9 @@
  * The ASF licenses this file to You under the Apache License, Version 2.0
  * (the License); you may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
- * 
+ *

  *  http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *

  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an AS IS BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -29,25 +29,40 @@
 
 /**

  * XMLizable a String
- * 
+ *

  * @since 2.1.7
  */
 public class StringXMLizable implements XMLizable {
+private static class Context {
+SAXParser parser;
+Context() throws SAXException {
+SAXParserFactory parserFactory = SAXParserFactory.newInstance();
+parserFactory.setNamespaceAware(true);
+parser = null;
+try {
+parser = parserFactory.newSAXParser();
+} catch (ParserConfigurationException e) {
+throw new SAXException(Error creating SAX parser., e);
+}
+}
+}
+
+private static final ThreadLocal context = new ThreadLocal();
 private String data;
 
-public StringXMLizable(String data) {

+public StringXMLizable(final String data) {
 this.data = data;
 }
 
-public void toSAX(ContentHandler contentHandler) throws SAXException {

-SAXParserFactory parserFactory = SAXParserFactory.newInstance();
-parserFactory.setNamespaceAware(true);
-SAXParser parser = null;
-try {
-parser = parserFactory.newSAXParser();
-} catch (ParserConfigurationException e) {
-throw new SAXException(Error creating SAX parser., e);
+private Context getContext() throws SAXException {
+if (context.get() == null) {
+context.set(new Context());
 }
+return (Context) context.get();
+}
+
+public void toSAX(ContentHandler contentHandler) throws SAXException {
+final SAXParser parser = getContext().parser;
 parser.getXMLReader().setContentHandler(contentHandler);
 InputSource is = new InputSource(new StringReader(data));
 try {