OSGeo.org › OSGeo Software and Data Projects › GeoTools, the java GIS toolkit › geotools-devel

app-schema build time/mem

12 messages

app-schema build time/mem

Reply Threaded

Hi all,

I took some time to dig a bit into app-schema build times and got some
interesting results.

I've grabbed the profiler and ran the test suite, then I got one of the
most time/mem consuming tests, FeatureChainingTest and profiled it alone.
Aside, I actually doubt it is one of the most offending ones, but just
where the profiler went crazy with 512m heap when running the test
suite, which makes me think there're some serious memory leaks at the
feature type registry or close to it.

Still, I was able to reduce the needs of FetureChainingTest from 255M of
Heap and 27 seconds build time to 102M Heap and 9.2 seconds build time.

That is achieved by the attached patch, which converts the test to JUnit
4 and uses @BeforeClass and @AfterClass methods to set up the needed
DataAccesses once for the whole suite.

I guess we can do the same for most of the test suites, except for the
few that actually assert the loading of a configuration. But other than
those, they should be all read-only tests, since app-schema support is
read only after all, so we could migrate them to one time loading.

That said, I'd be glad in keep investigating the source of the leaks AND
in helping redesign the schema parser so it does not parses ALL the
types and top level elements in the xsd schemas but only the ones needed
by the mappings. Those two improvements I'm pretty sure will provide a
substantial difference in startup time and memory requirements for the
module.

So, ok to commit the patch to FeatureChainingTest? Rini do you want to
go through the rest of them and refactor like I did for this one?

Cheers,

Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Index: src/test/java/org/geotools/data/complex/FeatureChainingTest.java
===================================================================
--- src/test/java/org/geotools/data/complex/FeatureChainingTest.java (revision 33535)
+++ src/test/java/org/geotools/data/complex/FeatureChainingTest.java (working copy)
@@ -17,6 +17,9 @@

package org.geotools.data.complex;

+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
@@ -26,8 +29,6 @@
import java.util.List;
import java.util.Map;

-import junit.framework.TestCase;
-
import org.geotools.data.DataAccess;
import org.geotools.data.DataAccessFinder;
import org.geotools.data.FeatureSource;
@@ -36,6 +37,9 @@
import org.geotools.feature.Types;
import org.geotools.filter.FilterFactoryImpl;
import org.geotools.filter.FunctionExpressionImpl;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
import org.opengis.feature.Feature;
import org.opengis.feature.Property;
import org.opengis.feature.type.FeatureType;
@@ -45,13 +49,15 @@
import org.opengis.filter.expression.Expression;
import org.xml.sax.Attributes;

+import com.vividsolutions.jts.util.Stopwatch;
+
/**
* This is the tests for feature chaining; nesting complex attributes (feature and non-feature)
* inside another complex attribute.
*
* @author Rini Angreani, Curtin University of Technology
*/
-public class FeatureChainingTest extends TestCase {
+public class FeatureChainingTest {
static final String GSMLNS = "http://www.cgi-iugs.org/xml/GeoSciML/2";

static final String GMLNS = "http://www.opengis.net/gml";
@@ -125,38 +131,52 @@
}
};

- final String schemaBase = "/test-data/";
+ private static final String schemaBase = "/test-data/";

- private FeatureSource<FeatureType, Feature> mfSource;
+ private static FeatureSource<FeatureType, Feature> mfSource;

/**
* Generated mapped features
*/
- private FeatureCollection<FeatureType, Feature> mfFeatures;
+ private static FeatureCollection<FeatureType, Feature> mfFeatures;

/**
* Generated geological unit features
*/
- private FeatureCollection<FeatureType, Feature> guFeatures;
+ private static FeatureCollection<FeatureType, Feature> guFeatures;

/**
* Generated compositional part fake "features"
*/
- private FeatureCollection<FeatureType, Feature> cpFeatures;
+ private static FeatureCollection<FeatureType, Feature> cpFeatures;

/**
* Generated controlled concept fake "features"
*/
- private FeatureCollection<FeatureType, Feature> ccFeatures;
+ private static FeatureCollection<FeatureType, Feature> ccFeatures;

+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception{
+ Stopwatch sw = new Stopwatch();
+ sw.start();
+ loadDataAccesses();
+ sw.stop();
+ System.out.println("Set up time: " + sw.getTimeString());
+ }
+
+ @AfterClass
+ public static void tearDownAfterClass() throws Exception{
+ DataAccessRegistry.unregisterAll();
+ }
+
/**
* Test that chaining works
*
* @throws Exception
*/
+ @Test
public void testFeatureChaining() throws Exception {
- this.loadDataAccesses();
-
Iterator<Feature> mfIterator = mfFeatures.iterator();

Iterator<Feature> guIterator = guFeatures.iterator();
@@ -261,8 +281,6 @@
mfFeatures.close(mfIterator);
guFeatures.close(guIterator);
cpFeatures.close(cpIterator);
-
- DataAccessRegistry.unregisterAll();
}

/**
@@ -272,10 +290,9 @@
*
* @throws Exception
*/
+ @Test
public void testManyOnChainedSide() throws Exception {

- this.loadDataAccesses();
-
final String LITHOLOGY = "lithology";
final int EXPECTED_RESULT_COUNT = 2;
// get controlled concept features on their own
@@ -319,7 +336,6 @@
assertEquals(lithologies.isEmpty(), true);
}
}
- DataAccessRegistry.unregisterAll();
}

/**
@@ -329,8 +345,8 @@
*
* @throws Exception
*/
+ @Test
public void testMultipleMultiValuedProperties() throws Exception {
- this.loadDataAccesses();
Iterator guIterator = guFeatures.iterator();

Feature guFeature;
@@ -386,7 +402,6 @@
assertEquals(realValues.size(), values.length);
assertEquals(realValues.containsAll(Arrays.asList(values)), true);
}
- DataAccessRegistry.unregisterAll();
guFeatures.close(guIterator);
}

@@ -395,6 +410,7 @@
*
* @throws Exception
*/
+ @Test
public void testMultiValuedSimpleProperties() throws Exception {
// Controlled Concept can have many gml:name
Map dsParams = new HashMap();
@@ -436,8 +452,8 @@
*
* @throws Exception
*/
+ @Test
public void testFilters() throws Exception {
- this.loadDataAccesses();
// make sure filter query can be made on MappedFeature based on GU properties
//
// <ogc:Filter>
@@ -487,7 +503,6 @@
filteredResults = guSource.getFeatures(filter);
assertEquals(getCount(filteredResults), 3);

- DataAccessRegistry.unregisterAll();
}

/**
@@ -497,6 +512,7 @@
*
* @throws Exception
*/
+ @Test
public void testComplexTypeWithSimpleContent() throws Exception {
Map dsParams = new HashMap();
URL url = getClass().getResource(schemaBase + "ComplexTypeWithSimpleContent.xml");
@@ -594,6 +610,7 @@
*
* @throws Exception
*/
+ @Test
public void testMultiValuedPropertiesByRef() throws Exception {
final String MF_PREFIX = "urn:cgi:feature:MappedFeature:";
final String OCCURENCE = "occurence";
@@ -605,8 +622,6 @@
}
};

- this.loadDataAccesses();
-
ArrayList<String> processedFeatureIds = new ArrayList<String>();

Iterator guIterator = guFeatures.iterator();
@@ -640,7 +655,6 @@

// clean ups
guFeatures.close(guIterator);
- DataAccessRegistry.unregisterAll();
}

/**
@@ -649,12 +663,12 @@
* @return
* @throws Exception
*/
- private void loadDataAccesses() throws Exception {
+ private static void loadDataAccesses() throws Exception {
/**
* Load mapped feature data access
*/
Map dsParams = new HashMap();
- URL url = getClass().getResource(schemaBase + "MappedFeaturePropertyfile.xml");
+ URL url = FeatureChainingTest.class.getResource(schemaBase + "MappedFeaturePropertyfile.xml");
assertNotNull(url);

dsParams.put("dbtype", "app-schema");
@@ -671,7 +685,7 @@
/**
* Load geologic unit data access
*/
- url = getClass().getResource(schemaBase + "GeologicUnit.xml");
+ url = FeatureChainingTest.class.getResource(schemaBase + "GeologicUnit.xml");
assertNotNull(url);

dsParams.put("url", url.toExternalForm());

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Geotools-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Rini Angreani

Re: app-schema build time/mem

Reply Threaded

Hi Gabriel,

Thank you for your help.
I will refactor the rest of other tests like that as well.

Cheers
Rini

-----Original Message-----
From: Gabriel Roldan [mailto:[hidden email]]
Sent: Friday, 10 July 2009 4:35 AM
To: Geotools-Devel list; Caradoc-Davies, Ben (E&M, Kensington); Angreani, Rini (E&M, Kensington); Justin Deoliveira; Andrea Aime
Subject: app-schema build time/mem

Hi all,

I took some time to dig a bit into app-schema build times and got some
interesting results.

I've grabbed the profiler and ran the test suite, then I got one of the
most time/mem consuming tests, FeatureChainingTest and profiled it alone.
Aside, I actually doubt it is one of the most offending ones, but just
where the profiler went crazy with 512m heap when running the test
suite, which makes me think there're some serious memory leaks at the
feature type registry or close to it.

Still, I was able to reduce the needs of FetureChainingTest from 255M of
Heap and 27 seconds build time to 102M Heap and 9.2 seconds build time.

That is achieved by the attached patch, which converts the test to JUnit
4 and uses @BeforeClass and @AfterClass methods to set up the needed
DataAccesses once for the whole suite.

I guess we can do the same for most of the test suites, except for the
few that actually assert the loading of a configuration. But other than
those, they should be all read-only tests, since app-schema support is
read only after all, so we could migrate them to one time loading.

That said, I'd be glad in keep investigating the source of the leaks AND
in helping redesign the schema parser so it does not parses ALL the
types and top level elements in the xsd schemas but only the ones needed
by the mappings. Those two improvements I'm pretty sure will provide a
substantial difference in startup time and memory requirements for the
module.

So, ok to commit the patch to FeatureChainingTest? Rini do you want to
go through the rest of them and refactor like I did for this one?

Cheers,

Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Geotools-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Ben Caradoc-Davies

Re: app-schema build time/mem

Reply Threaded

In reply to this post by Gabriel Roldan

Thanks Gabriel, that is a definite improvement. GEOT-2608 committed as
r33538.

Gabriel Roldan wrote:

> Hi all,
>
> I took some time to dig a bit into app-schema build times and got some
> interesting results.
>
> I've grabbed the profiler and ran the test suite, then I got one of the
> most time/mem consuming tests, FeatureChainingTest and profiled it alone.
> Aside, I actually doubt it is one of the most offending ones, but just
> where the profiler went crazy with 512m heap when running the test
> suite, which makes me think there're some serious memory leaks at the
> feature type registry or close to it.
>
> Still, I was able to reduce the needs of FetureChainingTest from 255M of
> Heap and 27 seconds build time to 102M Heap and 9.2 seconds build time.
>
> That is achieved by the attached patch, which converts the test to JUnit
> 4 and uses @BeforeClass and @AfterClass methods to set up the needed
> DataAccesses once for the whole suite.
>
> I guess we can do the same for most of the test suites, except for the
> few that actually assert the loading of a configuration. But other than
> those, they should be all read-only tests, since app-schema support is
> read only after all, so we could migrate them to one time loading.
>
>
> That said, I'd be glad in keep investigating the source of the leaks AND
> in helping redesign the schema parser so it does not parses ALL the
> types and top level elements in the xsd schemas but only the ones needed
> by the mappings. Those two improvements I'm pretty sure will provide a
> substantial difference in startup time and memory requirements for the
> module.
>
> So, ok to commit the patch to FeatureChainingTest? Rini do you want to
> go through the rest of them and refactor like I did for this one?
>
> Cheers,
>
> Gabriel
>

--
Ben Caradoc-Davies <[hidden email]>
Software Engineer, CSIRO Exploration and Mining
Australian Resources Research Centre
26 Dick Perry Ave, Kensington WA 6151, Australia

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Geotools-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Ben Caradoc-Davies

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

Ben Caradoc-Davies wrote:
> Thanks Gabriel, that is a definite improvement. GEOT-2608 committed as
> r33538.

Still not enough to get FeatureChainingTest to run with 256M heap, as on
Hudson.

--
Ben Caradoc-Davies <[hidden email]>
Software Engineer, CSIRO Exploration and Mining
Australian Resources Research Centre
26 Dick Perry Ave, Kensington WA 6151, Australia

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Geotools-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Gabriel Roldan

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

Ben Caradoc-Davies wrote:
> Ben Caradoc-Davies wrote:
>> Thanks Gabriel, that is a definite improvement. GEOT-2608 committed as
>> r33538.
>
> Still not enough to get FeatureChainingTest to run with 256M heap, as on
> Hudson.
>
nope, but because of a memory leak. If you run FeatureChainingTest only
you can do that with 128m:
mvn test -Dtest=org.geotools.data.complex.FeatureChainingTest
-Dtest.maxHeapSize=128m -o

So the two improvements needed keep being:
- lazy parsing of emf elements/types into gt descriptors/types
- fix mem leak (DataAccessRegistry is my suspect)

I am looking into the former now. No promises though, it's getting late,
but I may hopefully come up with at least a sensible plan.

As for the seconds, I would also like to provide a patch. But may be
over the weekend as this is spare time.

Cheers,
Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Geotools-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Rini Angreani

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

I think I found the culprit.
I refactored a bunch of tests to run the set up once, it got faster, but not enough.
Then I changed all the test mapping files to point to:
<schemaUri>http://schemas.opengis.net/GeoSciML/geosciml.xsd </schemaUri>
Instead of the local schemas in test-resources.

Now I can run all gt-app-schema tests with 256 M :D.
I will make sure I didn't break any geoserver app-schema tests before I commit.

-----Original Message-----
From: Gabriel Roldan [mailto:[hidden email]]
Sent: Friday, 10 July 2009 11:54 AM
To: Caradoc-Davies, Ben (E&M, Kensington)
Cc: Angreani, Rini (E&M, Kensington); Geotools-Devel list
Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem

Ben Caradoc-Davies wrote:
> Ben Caradoc-Davies wrote:
>> Thanks Gabriel, that is a definite improvement. GEOT-2608 committed as
>> r33538.
>
> Still not enough to get FeatureChainingTest to run with 256M heap, as on
> Hudson.
>
nope, but because of a memory leak. If you run FeatureChainingTest only
you can do that with 128m:
mvn test -Dtest=org.geotools.data.complex.FeatureChainingTest
-Dtest.maxHeapSize=128m -o

So the two improvements needed keep being:
- lazy parsing of emf elements/types into gt descriptors/types
- fix mem leak (DataAccessRegistry is my suspect)

I am looking into the former now. No promises though, it's getting late,
but I may hopefully come up with at least a sensible plan.

As for the seconds, I would also like to provide a patch. But may be
over the weekend as this is spare time.

Cheers,
Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Geotools-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Gabriel Roldan

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

That's cool, so it seems that way it uses the Oasis catalog?

btw, I'm about to get ready a patch to get the resource consumption even
lower, but am afraid of getting too much conflicts with your changes. Do
you think you can commit the test refactorings relatively soon?

Cheers,
Gabriel

[hidden email] wrote:

> I think I found the culprit.
> I refactored a bunch of tests to run the set up once, it got faster, but not enough.
> Then I changed all the test mapping files to point to:
> <schemaUri>http://schemas.opengis.net/GeoSciML/geosciml.xsd </schemaUri>
> Instead of the local schemas in test-resources.
>
> Now I can run all gt-app-schema tests with 256 M :D.
> I will make sure I didn't break any geoserver app-schema tests before I commit.
>
> -----Original Message-----
> From: Gabriel Roldan [mailto:[hidden email]]
> Sent: Friday, 10 July 2009 11:54 AM
> To: Caradoc-Davies, Ben (E&M, Kensington)
> Cc: Angreani, Rini (E&M, Kensington); Geotools-Devel list
> Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem
>
> Ben Caradoc-Davies wrote:
>> Ben Caradoc-Davies wrote:
>>> Thanks Gabriel, that is a definite improvement. GEOT-2608 committed as
>>> r33538.
>> Still not enough to get FeatureChainingTest to run with 256M heap, as on
>> Hudson.
>>
> nope, but because of a memory leak. If you run FeatureChainingTest only
> you can do that with 128m:
> mvn test -Dtest=org.geotools.data.complex.FeatureChainingTest
> -Dtest.maxHeapSize=128m -o
>
> So the two improvements needed keep being:
> - lazy parsing of emf elements/types into gt descriptors/types
> - fix mem leak (DataAccessRegistry is my suspect)
>
> I am looking into the former now. No promises though, it's getting late,
> but I may hopefully come up with at least a sensible plan.
>
> As for the seconds, I would also like to provide a patch. But may be
> over the weekend as this is spare time.
>
> Cheers,
> Gabriel

--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/Challenge
_______________________________________________
Geotools-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Rini Angreani

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

It seems that way.
Go ahead and commit your changes first. We have the whole day here still.

Cheers
Rini

-----Original Message-----
From: Gabriel Roldan [mailto:[hidden email]]
Sent: Friday, 10 July 2009 2:16 PM
To: Angreani, Rini (E&M, Kensington)
Cc: Caradoc-Davies, Ben (E&M, Kensington); [hidden email]
Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem

That's cool, so it seems that way it uses the Oasis catalog?

btw, I'm about to get ready a patch to get the resource consumption even
lower, but am afraid of getting too much conflicts with your changes. Do
you think you can commit the test refactorings relatively soon?

Cheers,
Gabriel

[hidden email] wrote:

Gabriel Roldan

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

Hi Rini,

I can do that right away but there's a gotcha. I got a single test
failing that I'm finding myself unable to fix:
AppSchemaDataAccessIntegrationTest.

Problem seems to be that it references mineralOccurrence.xsd but it
doesn't import Gsml.xsd and hence GeologicUnitType can't be found. I'm
not sure why it worked before though cause the logic didn't essentially
changed. May be if I commit you'll be able to assess it? (as it extends
DataAccessIntegrationTest but I'm not sure exactly what for, other than
to reuse the fake DataAccess on it?)

What I did is the following:
I decoupled the xsd to EMF and EMF to GeoTools parsing so that they're
the responsibilities of different classes.

So I like the separation of concerns more that way. EmfAppSchemaReader
now only cares about parsing the schemas into SchemaIndex. The new class
FeatureTypeRegistry takes them to parse the EMF objects on demand. This
means not all the types and elements are parsed to geotools objects but
only the needed ones, decreasing the memory usage lot. From 255m to 52m
for FeatureChainingTest, for example.

I can build now with 256m in 59 seconds. I think with that and your
<schemaUri> magic we should be more than good. But certainly I didn't
have time to test that on the wild (ie, with geoserver).

Are you still going to let me commit?

Cheers,

Gabriel

[hidden email] wrote:

> It seems that way.
> Go ahead and commit your changes first. We have the whole day here still.
>
> Cheers
> Rini
>
> -----Original Message-----
> From: Gabriel Roldan [mailto:[hidden email]]
> Sent: Friday, 10 July 2009 2:16 PM
> To: Angreani, Rini (E&M, Kensington)
> Cc: Caradoc-Davies, Ben (E&M, Kensington); [hidden email]
> Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem
>
> That's cool, so it seems that way it uses the Oasis catalog?
>
> btw, I'm about to get ready a patch to get the resource consumption even
> lower, but am afraid of getting too much conflicts with your changes. Do
> you think you can commit the test refactorings relatively soon?
>
> Cheers,
> Gabriel
>
> [hidden email] wrote:
>> I think I found the culprit.
>> I refactored a bunch of tests to run the set up once, it got faster, but not enough.
>> Then I changed all the test mapping files to point to:
>> <schemaUri>http://schemas.opengis.net/GeoSciML/geosciml.xsd </schemaUri>
>> Instead of the local schemas in test-resources.
>>
>> Now I can run all gt-app-schema tests with 256 M :D.
>> I will make sure I didn't break any geoserver app-schema tests before I commit.
>>
>> -----Original Message-----
>> From: Gabriel Roldan [mailto:[hidden email]]
>> Sent: Friday, 10 July 2009 11:54 AM
>> To: Caradoc-Davies, Ben (E&M, Kensington)
>> Cc: Angreani, Rini (E&M, Kensington); Geotools-Devel list
>> Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem
>>
>> Ben Caradoc-Davies wrote:
>>> Ben Caradoc-Davies wrote:
>>>> Thanks Gabriel, that is a definite improvement. GEOT-2608 committed as
>>>> r33538.
>>> Still not enough to get FeatureChainingTest to run with 256M heap, as on
>>> Hudson.
>>>
>> nope, but because of a memory leak. If you run FeatureChainingTest only
>> you can do that with 128m:
>> mvn test -Dtest=org.geotools.data.complex.FeatureChainingTest
>> -Dtest.maxHeapSize=128m -o
>>
>> So the two improvements needed keep being:
>> - lazy parsing of emf elements/types into gt descriptors/types
>> - fix mem leak (DataAccessRegistry is my suspect)
>>
>> I am looking into the former now. No promises though, it's getting late,
>> but I may hopefully come up with at least a sensible plan.
>>
>> As for the seconds, I would also like to provide a patch. But may be
>> over the weekend as this is spare time.
>>
>> Cheers,
>> Gabriel
>
>

Rini Angreani

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

OK sure.

Cheers
Rini

-----Original Message-----
From: Gabriel Roldan [mailto:[hidden email]]
Sent: Friday, 10 July 2009 2:41 PM
To: Angreani, Rini (E&M, Kensington)
Cc: Caradoc-Davies, Ben (E&M, Kensington); [hidden email]
Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem

Hi Rini,

I can do that right away but there's a gotcha. I got a single test
failing that I'm finding myself unable to fix:
AppSchemaDataAccessIntegrationTest.

Problem seems to be that it references mineralOccurrence.xsd but it
doesn't import Gsml.xsd and hence GeologicUnitType can't be found. I'm
not sure why it worked before though cause the logic didn't essentially
changed. May be if I commit you'll be able to assess it? (as it extends
DataAccessIntegrationTest but I'm not sure exactly what for, other than
to reuse the fake DataAccess on it?)

What I did is the following:
I decoupled the xsd to EMF and EMF to GeoTools parsing so that they're
the responsibilities of different classes.

So I like the separation of concerns more that way. EmfAppSchemaReader
now only cares about parsing the schemas into SchemaIndex. The new class
FeatureTypeRegistry takes them to parse the EMF objects on demand. This
means not all the types and elements are parsed to geotools objects but
only the needed ones, decreasing the memory usage lot. From 255m to 52m
for FeatureChainingTest, for example.

I can build now with 256m in 59 seconds. I think with that and your
<schemaUri> magic we should be more than good. But certainly I didn't
have time to test that on the wild (ie, with geoserver).

Are you still going to let me commit?

Cheers,

Gabriel

[hidden email] wrote:

Gabriel Roldan

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

You got it. Remember I commented out AppSchemaDataAccessIntegrationTest
which I don't know why it doesn't get the gsml types (though gsml not
being imported might be the reason).

Cheers,

Gabriel
[hidden email] wrote:

> OK sure.
>
> Cheers
> Rini
>
> -----Original Message-----
> From: Gabriel Roldan [mailto:[hidden email]]
> Sent: Friday, 10 July 2009 2:41 PM
> To: Angreani, Rini (E&M, Kensington)
> Cc: Caradoc-Davies, Ben (E&M, Kensington); [hidden email]
> Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem
>
> Hi Rini,
>
> I can do that right away but there's a gotcha. I got a single test
> failing that I'm finding myself unable to fix:
> AppSchemaDataAccessIntegrationTest.
>
> Problem seems to be that it references mineralOccurrence.xsd but it
> doesn't import Gsml.xsd and hence GeologicUnitType can't be found. I'm
> not sure why it worked before though cause the logic didn't essentially
> changed. May be if I commit you'll be able to assess it? (as it extends
> DataAccessIntegrationTest but I'm not sure exactly what for, other than
> to reuse the fake DataAccess on it?)
>
> What I did is the following:
> I decoupled the xsd to EMF and EMF to GeoTools parsing so that they're
> the responsibilities of different classes.
>
> So I like the separation of concerns more that way. EmfAppSchemaReader
> now only cares about parsing the schemas into SchemaIndex. The new class
> FeatureTypeRegistry takes them to parse the EMF objects on demand. This
> means not all the types and elements are parsed to geotools objects but
> only the needed ones, decreasing the memory usage lot. From 255m to 52m
> for FeatureChainingTest, for example.
>
> I can build now with 256m in 59 seconds. I think with that and your
> <schemaUri> magic we should be more than good. But certainly I didn't
> have time to test that on the wild (ie, with geoserver).
>
> Are you still going to let me commit?
>
> Cheers,
>
> Gabriel
>
> [hidden email] wrote:
>> It seems that way.
>> Go ahead and commit your changes first. We have the whole day here still.
>>
>> Cheers
>> Rini
>>
>> -----Original Message-----
>> From: Gabriel Roldan [mailto:[hidden email]]
>> Sent: Friday, 10 July 2009 2:16 PM
>> To: Angreani, Rini (E&M, Kensington)
>> Cc: Caradoc-Davies, Ben (E&M, Kensington); [hidden email]
>> Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem
>>
>> That's cool, so it seems that way it uses the Oasis catalog?
>>
>> btw, I'm about to get ready a patch to get the resource consumption even
>> lower, but am afraid of getting too much conflicts with your changes. Do
>> you think you can commit the test refactorings relatively soon?
>>
>> Cheers,
>> Gabriel
>>
>> [hidden email] wrote:
>>> I think I found the culprit.
>>> I refactored a bunch of tests to run the set up once, it got faster, but not enough.
>>> Then I changed all the test mapping files to point to:
>>> <schemaUri>http://schemas.opengis.net/GeoSciML/geosciml.xsd </schemaUri>
>>> Instead of the local schemas in test-resources.
>>>
>>> Now I can run all gt-app-schema tests with 256 M :D.
>>> I will make sure I didn't break any geoserver app-schema tests before I commit.
>>>
>>> -----Original Message-----
>>> From: Gabriel Roldan [mailto:[hidden email]]
>>> Sent: Friday, 10 July 2009 11:54 AM
>>> To: Caradoc-Davies, Ben (E&M, Kensington)
>>> Cc: Angreani, Rini (E&M, Kensington); Geotools-Devel list
>>> Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem
>>>
>>> Ben Caradoc-Davies wrote:
>>>> Ben Caradoc-Davies wrote:
>>>>> Thanks Gabriel, that is a definite improvement. GEOT-2608 committed as
>>>>> r33538.
>>>> Still not enough to get FeatureChainingTest to run with 256M heap, as on
>>>> Hudson.
>>>>
>>> nope, but because of a memory leak. If you run FeatureChainingTest only
>>> you can do that with 128m:
>>> mvn test -Dtest=org.geotools.data.complex.FeatureChainingTest
>>> -Dtest.maxHeapSize=128m -o
>>>
>>> So the two improvements needed keep being:
>>> - lazy parsing of emf elements/types into gt descriptors/types
>>> - fix mem leak (DataAccessRegistry is my suspect)
>>>
>>> I am looking into the former now. No promises though, it's getting late,
>>> but I may hopefully come up with at least a sensible plan.
>>>
>>> As for the seconds, I would also like to provide a patch. But may be
>>> over the weekend as this is spare time.
>>>
>>> Cheers,
>>> Gabriel
>>
>
>

Rini Angreani

Re: [ExternalEmail] Re: app-schema build time/mem

Reply Threaded

I think I know why. The app-schema reader in that test doesn't have an OASIS catalog.
I happened to be find that yesterday, while making changes for a different issue.
It worked because it didn't need gsml types (it was creating a non-gsml data access).
Anyway, I'll look into it. Thank's for everything Gabriel.

Rini

-----Original Message-----
From: Gabriel Roldan [mailto:[hidden email]]
Sent: Friday, 10 July 2009 3:01 PM
To: Angreani, Rini (E&M, Kensington)
Cc: Caradoc-Davies, Ben (E&M, Kensington); [hidden email]
Subject: Re: [ExternalEmail] Re: [Geotools-devel] app-schema build time/mem

You got it. Remember I commented out AppSchemaDataAccessIntegrationTest
which I don't know why it doesn't get the gsml types (though gsml not
being imported might be the reason).

Cheers,

Gabriel
[hidden email] wrote: