Jody Garnett ha scritto:
Jody Garnett ha scritto:
I have made the change proposal here:
- http://docs.codehaus.org/display/GEOTOOLS/Clean+up+Generics+from+DataStore
I am going to complete the SimpleFeatureCollection patch today for trunk.
The idea looks good, there is probably something about the language in
the proposal that confuses me.
Yeah I was sleepy; let me do an editing run.
For example, what is the section "Introduce SimpleFeatureSource to 2.6.x" about?
The change to introduce SimpleFeatureSource, SimpleFeatureStore was actually
pretty small.
The change to introduce SimpleFeatureCollection was not.
They are two seperate patches.
If you like we can consider the SimpleFeatureSource patch for 2.6.x; it has 50%
of the benifit in terms of cleaning up the example code.
I believe we're too late in the 2.6.x game to make such a change, better
to do that only on trunk.
And then we can have a look at what happens in GeoServer to gauge how
much it will break the existing code (I guess it should not,
but one never knows).
It's a big change, better to avoid doing it on trunk and besides you say above
and in the jira that the patch is for trunk (trunk only I guess).
And you cite FeatureAccess... which is actually not there.
About the renderer using simple features, it's really about performance,
I can see if I can use Feature but the current implementation may be bad
performance wise (accessing attributes through it is more costly afaik).
It is not really an issue until we get more generic feature collections; there
is a small number of places where SimpleFeature was assumed in the rendering
code.
With respect to performance I think that if SimpleFeatures are passed in the
performance will be the same?
Performance will definitely not be the same.
SimpleFeatureImpl.getAttribute(name) returns the actual value,
Feature.getProperty(name) returns a "Property" object, a
wrapper around the value which has to be built.
The SLD handling code uses property
accessors, which are slower than direct access but make the code
portable.
I made a little benchmark for different property access styles
(attached):
- using the simple feature getAttribute()
- using Feature getProperty().getValue()
- dynamically looking up property accessors
- caching property accessors
The timings on Java 6 are:
Building features...
Building features: 2.759
Testing SimpleFeature access...
Direct access: 0.081
Testing Feature access...
Feature access: 0.468
Testing property accessors...
Property accessors: 2.438
Testing cached accessors...
Cached property accessors: 0.105
As you can see using Feature is 8 times slower,
and using dynamically found accessors takes as much time
as actually building the features.
The only way to get decent performance is to use
accessors and reuse them.
I can have a look into doing that.
Btw, a curiosity: in this test the difference between
Java 5 and Java 6 is massive. Look at the results
with Java 1.5.0_22:
Building features...
Building features: 18.702
Testing SimpleFeature access...
Direct access: 0.085
Testing Feature access...
Feature access: 0.474
Testing property accessors...
Property accessors: 1.898
Testing cached accessors...
Cached property accessors: 0.141
With a Java 7 recent build the results are actually odd
in another way:
Building features...
Building features: 2.131
Testing SimpleFeature access...
Direct access: 0.138
Testing Feature access...
Feature access: 1.008
Testing property accessors...
Property accessors: 1.524
Testing cached accessors...
Cached property accessors: 0.091
As you can see... cached property accessors are fast than direct
access??? :-) And Feature access is actually twice as slow as Java 6,
ugh. Not sure what to make of the Java 7 numbers, but it's also
just a preview build.
To sum up, maybe clean up a little the proposal. I feel good about the change,
I should just get some time to actually try to apply the patch
(will try to do that soon)
Thanks Andrea; I would like the feedback of yourself and Micheal if possible.
Basic feedback is that I prefer to have everything happen on trunk only.
This will make for a few months of hard to port forward patches btw...
probably late July would have been a better time for this patch.
However I also understand that if we delay you'll have to start over.
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.
import java.util.ArrayList;
import java.util.List;
import org.geotools.feature.simple.SimpleFeatureBuilder;
import org.geotools.feature.simple.SimpleFeatureTypeBuilder;
import org.geotools.filter.expression.PropertyAccessor;
import org.geotools.filter.expression.PropertyAccessors;
import org.opengis.feature.Feature;
import org.opengis.feature.simple.SimpleFeature;
import org.opengis.feature.simple.SimpleFeatureType;
import com.vividsolutions.jts.geom.Polygon;
import com.vividsolutions.jts.io.WKTReader;
public class PropertyAccessTest {
public static void main(String[] args) throws Exception {
SimpleFeatureTypeBuilder ftb = new SimpleFeatureTypeBuilder();
ftb.add("geom", Polygon.class, "EPSG:4326");
ftb.add("name", String.class);
ftb.add("persons", Long.class);
ftb.setName("testFeature");
SimpleFeatureType ft = ftb.buildFeatureType();
// building features
System.out.println("Building features...");
long start = System.currentTimeMillis();
List features = new ArrayList();
SimpleFeatureBuilder fb = new SimpleFeatureBuilder(ft);
Polygon geom = (Polygon) new WKTReader().read("POLYGON((0 0, 0 10, 10 10, 10 0, 0 0))");
for (int i = 0; i < 1000000; i++) {
fb.add(geom);
fb.add("feature" + i);
fb.add(i);
features.add(fb.buildFeature(null));
}
long end = System.currentTimeMillis();
System.out.println("Building features: " + (end - start) / 1000.0);
System.out.println("Testing SimpleFeature access...");
start = System.currentTimeMillis();
testDirectAccess(features);
end = System.currentTimeMillis();
System.out.println("Direct access: " + (end - start) / 1000.0);
System.out.println("Testing Feature access...");
start = System.currentTimeMillis();
testFeatureAccess(features);
end = System.currentTimeMillis();
System.out.println("Feature access: " + (end - start) / 1000.0);
System.out.println("Testing property accessors...");
start = System.currentTimeMillis();
testAccessors(features);
end = System.currentTimeMillis();
System.out.println("Property accessors: " + (end - start) / 1000.0);
System.out.println("Testing cached accessors...");
start = System.currentTimeMillis();
testCachedAccessors(features);
end = System.currentTimeMillis();
System.out.println("Cached property accessors: " + (end - start) / 1000.0);
}
private static void testFeatureAccess(List<Feature> features) {
for (Feature feature : features) {
feature.getProperty("geom").getValue();
feature.getProperty("name").getValue();
feature.getProperty("persons").getValue();
}
}
private static void testDirectAccess(List<SimpleFeature> features) {
for (SimpleFeature feature : features) {
feature.getAttribute("geom");
feature.getAttribute("name");
feature.getAttribute("persons");
}
}
private static void testAccessors(List features) {
for (Object feature : features) {
PropertyAccessors.findPropertyAccessor(feature, "geom", Polygon.class, null).get(feature, "geom", Polygon.class);
PropertyAccessors.findPropertyAccessor(feature, "name", String.class, null).get(feature, "geom", String.class);
PropertyAccessors.findPropertyAccessor(feature, "persons", Integer.class, null).get(feature, "geom", Integer.class);
}
}
private static void testCachedAccessors(List features) {
PropertyAccessor geomAccessor = PropertyAccessors.findPropertyAccessor(features.get(0), "geom", Polygon.class, null);
PropertyAccessor nameAccessor = PropertyAccessors.findPropertyAccessor(features.get(0), "name", String.class, null);
PropertyAccessor personsAccessor = PropertyAccessors.findPropertyAccessor(features.get(0), "persons", Integer.class, null);
for (Object feature : features) {
geomAccessor.get(feature, "geom", Polygon.class);
nameAccessor.get(feature, "geom", String.class);
personsAccessor.get(feature, "geom", Integer.class);
}
}
}
------------------------------------------------------------------------------
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
