Hi Andrea,
The patch looks good, I see no problems with it.
I still think somehow isolating the renderer from the extension point is
a better long term solution as its an extension point and have really no
control over someone writing a bad implementation. My 2c.
-Justin
Andrea Aime wrote:
> Hi,
> not that things are cooled down I got back to that test rendering
> 100000 points and trying to get back to 2.3.x performance level.
> First, the old benchmarks, at the point were I did stop improving
> things:
>
> * 24 seconds on jdk 1.4
> * 5.2 seconds on jdk 1.5
> * 3.8 seconds on jdk 1.6
>
> With the patches attached to this mail, we get down to:
>
> * 17.5 seconds on jdk 1.4
> * 4.2 seconds on jdk 1.5
> * 2.9 seconds on jdk 1.6
>
> The patch contains various changes, and I would like rendering
> and attribute people to have a look at them, but basically
> they can be summed up to:
> * avoiding completely the converters layer when possible,
> that is, evaluate expressions without a target, use instanceof
> to check it it's the class we were looking for, and fall
> back on the converters only if the test is negative. This
> speeds up things a lot because instanceof is orders or magnitude
> faster than myClazz.isInstance(xxx), and Class.isInstance was one
> of the worst offenders in the profile
> * cache the last used property accessor in AttributeExpressionImpl,
> and call canHandle(obj, attPath, target) on it to make sure it's
> still valid. This eliminate the second worst offender in the
> profile, which was
> SimpleFeaturePropertyAccessorFactory.createPropertyAccessor.
>
> Well, look at the patch, and let me know if I can commit it straight
> away, or you require some modifications.
>
> Cheers
> Andrea
>
>
> ------------------------------------------------------------------------
>
> Index: render/src/main/java/org/geotools/renderer/style/SLDStyleFactory.java
> ===================================================================
> --- render/src/main/java/org/geotools/renderer/style/SLDStyleFactory.java
> (revisione 25054)
> +++ render/src/main/java/org/geotools/renderer/style/SLDStyleFactory.java
> (copia locale)
> @@ -85,6 +85,7 @@
> import org.geotools.styling.TextMark;
> import org.geotools.styling.TextSymbolizer;
> import org.geotools.styling.TextSymbolizer2;
> +import org.geotools.util.SoftValueHashMap;
> import org.w3c.dom.Document;
>
> import com.vividsolutions.jts.geom.Geometry;
> @@ -217,13 +218,13 @@
> }
>
> /** Parsed SVG glyphs */
> - WeakHashMap svgGlyphs = new WeakHashMap();
> + Map svgGlyphs = new SoftValueHashMap();
>
> /** Symbolizers that depend on attributes */
> - WeakHashMap dynamicSymbolizers = new WeakHashMap();
> + Map dynamicSymbolizers = new SoftValueHashMap();
>
> /** Symbolizers that do not depend on attributes */
> - WeakHashMap staticSymbolizers = new WeakHashMap();
> + Map staticSymbolizers = new SoftValueHashMap();
>
> private static Set getSupportedGraphicFormats() {
> if (supportedGraphicFormats == null) {
> @@ -306,7 +307,7 @@
> if ((nameSet == null) || (nameSet.size() == 0)) {
> staticSymbolizers.put(key, style);
> } else {
> - dynamicSymbolizers.put(key, null);
> + dynamicSymbolizers.put(key, Boolean.TRUE);
> }
> }
> }
> @@ -1441,6 +1442,9 @@
> if(exp == null){
> return fallback;
> }
> + Object o = exp.evaluate(f);
> + if(o instanceof Number)
> + return ((Number) o).floatValue();
> Float fo = (Float) exp.evaluate( f, Float.class );
> if( fo != null ){
> return fo.floatValue();
> @@ -1453,6 +1457,9 @@
> if(exp == null){
> return fallback;
> }
> + Object o = exp.evaluate(f);
> + if(o instanceof Number)
> + return ((Number) o).doubleValue();
> Double d = (Double) exp.evaluate( f, Double.class );
> if( d != null ){
> return d.doubleValue();
> Index: render/src/main/java/org/geotools/renderer/lite/StreamingRenderer.java
> ===================================================================
> --- render/src/main/java/org/geotools/renderer/lite/StreamingRenderer.java
> (revisione 25054)
> +++ render/src/main/java/org/geotools/renderer/lite/StreamingRenderer.java
> (copia locale)
> @@ -179,6 +179,8 @@
> private final static PropertyName gridPropertyName=
> filterFactory.property("grid");
>
> private final static PropertyName paramsPropertyName=
> filterFactory.property("params");
> +
> + private final static PropertyName defaultGeometryPropertyName=
> filterFactory.property("");
>
>
> private final static CoordinateOperationFactory operationFactory = new
> BufferedCoordinateOperationFactory(
> @@ -2237,7 +2239,7 @@
> }
>
> if( geomName == null ){
> - geomName = ""; // indicate default geometry!
> + return defaultGeometryPropertyName;
> }
> return filterFactory.property(geomName);
> }
> Index: main/src/main/java/org/geotools/filter/AttributeExpressionImpl.java
> ===================================================================
> --- main/src/main/java/org/geotools/filter/AttributeExpressionImpl.java
> (revisione 25054)
> +++ main/src/main/java/org/geotools/filter/AttributeExpressionImpl.java
> (copia locale)
> @@ -158,27 +158,35 @@
> * @param obj Object from which we need to extract a property value.
> */
> public Object evaluate(Object obj) {
> - PropertyAccessor accessor =
> - PropertyAccessors.findPropertyAccessor( obj, attPath, null,
> null );
> -
> - if ( accessor == null ) {
> - return null; //JD:not throwing exception to remain backwards
> compatabile, just returnign null
> - }
> - return accessor.get( obj, attPath, null );
> + return evaluate(obj, null);
> }
>
> +
> public Object evaluate(Object obj, Class target) {
> - PropertyAccessor accessor =
> - PropertyAccessors.findPropertyAccessor( obj, attPath, target,
> null );
> + PropertyAccessor accessor = getPropertyAccessor(obj, target);
>
> if ( accessor == null ) {
> return null; //JD:not throwing exception to remain backwards
> compatabile, just returnign null
> }
> Object propertyValue = accessor.get( obj, attPath, target );
> + if(target == null)
> + return propertyValue;
> +
> Value value = new Value( propertyValue );
> return value.value( target ); // pull into the requested shape
>
> }
> +
> + private PropertyAccessor lastAccessor;
> + private PropertyAccessor getPropertyAccessor(Object obj, Class target) {
> + if(lastAccessor == null)
> + lastAccessor = PropertyAccessors.findPropertyAccessor( obj,
> attPath, target, null );
> + else if(!lastAccessor.canHandle(obj, attPath, target))
> + lastAccessor = PropertyAccessors.findPropertyAccessor( obj,
> attPath, target, null );
> +
> + return lastAccessor;
> + }
> +
> /**
> * Return this expression as a string.
> *
>
>
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share your
> opinions on IT & business topics through brief surveys-and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Geotools-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
--
Justin Deoliveira
The Open Planning Project
[EMAIL PROTECTED]
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
