cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joerg Heinicke <joerg.heini...@gmx.de>
Subject Re: svn commit: r629374 - /cocoon/trunk/core/cocoon-core/src/main/java/org/apache/cocoon/xml/StringXMLizable.java
Date Sun, 30 Mar 2008 06:50:42 GMT
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 {
> 
> 

Mime
View raw message