You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Joerg Heinicke <jo...@gmx.de> on 2008/03/30 08:50:42 UTC
Re: svn commit: r629374 - /cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java
On 20.02.2008 01:42, antonio@apache.org wrote:
> Author: antonio
> Date: Tue Feb 19 22:42:45 2008
> New Revision: 629374
>
> URL: http://svn.apache.org/viewvc?rev=629374&view=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=629374&r1=629373&r2=629374&view=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 {
>
>
Re: svn commit: r629374 -
/cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java
Posted by Alfred Nathaniel <an...@apache.org>.
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=629374&view=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-cvs&m=120348979305179&w=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
Posted by Joerg Heinicke <jo...@gmx.de>.
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=629374&view=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-cvs&m=120348979305179&w=4