[fdo-commits] r2617 - in trunk/Fdo: UnitTest Unmanaged Unmanaged/Inc/Common Unmanaged/Inc/Common/Xml Unmanaged/Src/Common Unmanaged/Src/Common/Xml

svn_fdo at osgeo.org svn_fdo at osgeo.org
Thu Mar 1 10:58:13 EST 2007


Author: brentrobinson
Date: 2007-03-01 10:58:12 -0500 (Thu, 01 Mar 2007)
New Revision: 2617

Added:
   trunk/Fdo/Unmanaged/Src/Common/CommonInternal.cpp
   trunk/Fdo/Unmanaged/Src/Common/CommonInternal.h
Modified:
   trunk/Fdo/UnitTest/CommonTest.cpp
   trunk/Fdo/Unmanaged/Common.vcproj
   trunk/Fdo/Unmanaged/Inc/Common/DictionaryElement.h
   trunk/Fdo/Unmanaged/Inc/Common/NamedCollection.h
   trunk/Fdo/Unmanaged/Inc/Common/Xml/Attribute.h
   trunk/Fdo/Unmanaged/Src/Common/Makefile.am
   trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.cpp
   trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.h
Log:
Ticket #22: Improved performance for reading from XML, by reducing number of memory allocations and string copies.

Modified: trunk/Fdo/UnitTest/CommonTest.cpp
===================================================================
--- trunk/Fdo/UnitTest/CommonTest.cpp	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/UnitTest/CommonTest.cpp	2007-03-01 15:58:12 UTC (rev 2617)
@@ -106,6 +106,46 @@
 
 void CommonTest::testCollections()
 {
+    // Test named collection, with hash map, for objects that can be renamed.
+
+    FdoFeatureSchemasP schemas = FdoFeatureSchemaCollection::Create(NULL);
+
+    FdoInt32 idx;
+
+    // Add enough items to force use of hash map
+    for ( idx = 0; idx < 1000; idx++ ) {
+        schemas->Add(
+            FdoFeatureSchemaP(
+                FdoFeatureSchema::Create(
+                    FdoStringP::Format( L"%d", idx ),
+                    L""
+                )
+            )
+        );
+    }
+
+    FdoFeatureSchemaP schema = schemas->FindItem(L"593");
+	FDO_CPPUNIT_ASSERT(schema);
+    // Rename an item. This makes the hash map stale. In hash map, "renamed" schema is still under
+    // key "593".
+    schema->SetName( L"renamed" );
+
+    // Add another "593". should work since other one was renamed
+    schema = FdoFeatureSchema::Create( L"593", L"" );
+    schemas->Add( schema );
+
+    // Add a second "renamed" schema. Must fail since duplicate.
+    schema = FdoFeatureSchema::Create( L"renamed", L"" );
+	bool failed = false;
+	try {
+        schemas->Add( schema );
+	}
+	catch ( FdoException* ex) {
+        ex->Release();
+		failed = true;
+	}
+
+	FDO_CPPUNIT_ASSERT(failed);
 }
 
 // Do some stuff with FdoIntArray, as non-Byte arrays are not currently 
@@ -465,6 +505,40 @@
 		) == 0 
 	);
 
+    // Test that duplicate name checking works when against the hash map.
+    // First, put lots of items in the dictionary to trigger the user of the map
+
+    int idx;
+
+    for ( idx = 0; idx < 1000; idx++ ) {
+		dictionary->Add( 
+            FdoDictionaryElementP( 
+                FdoDictionaryElement::Create(
+                    FdoStringP::Format( L"%d", idx ),
+                    L""
+                ) 
+            ) 
+        );
+    }
+
+    // Try adding a duplicate entry.
+    failed = false;
+	try {
+		dictionary->Add( 
+            FdoDictionaryElementP( 
+                FdoDictionaryElement::Create(
+                    FdoStringP::Format( L"535", idx ),
+                    L""
+                ) 
+            ) 
+        );
+	}
+	catch ( FdoException* ex) {
+        ex->Release();
+		failed = true;
+	}
+	FDO_CPPUNIT_ASSERT(failed);
+
 }
 
 void CommonTest::testVector()

Modified: trunk/Fdo/Unmanaged/Common.vcproj
===================================================================
--- trunk/Fdo/Unmanaged/Common.vcproj	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/Unmanaged/Common.vcproj	2007-03-01 15:58:12 UTC (rev 2617)
@@ -207,6 +207,14 @@
 				>
 			</File>
 			<File
+				RelativePath=".\Src\Common\CommonInternal.cpp"
+				>
+			</File>
+			<File
+				RelativePath=".\Src\Common\CommonInternal.h"
+				>
+			</File>
+			<File
 				RelativePath=".\Src\Common\Context.cpp"
 				>
 			</File>

Modified: trunk/Fdo/Unmanaged/Inc/Common/DictionaryElement.h
===================================================================
--- trunk/Fdo/Unmanaged/Inc/Common/DictionaryElement.h	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/Unmanaged/Inc/Common/DictionaryElement.h	2007-03-01 15:58:12 UTC (rev 2617)
@@ -26,6 +26,7 @@
 /// name-value pair.
 class FdoDictionaryElement : public FdoDisposable
 {
+    friend class FdoCommonInternal;
 
 public:
 

Modified: trunk/Fdo/Unmanaged/Inc/Common/NamedCollection.h
===================================================================
--- trunk/Fdo/Unmanaged/Inc/Common/NamedCollection.h	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/Unmanaged/Inc/Common/NamedCollection.h	2007-03-01 15:58:12 UTC (rev 2617)
@@ -101,25 +101,37 @@
         OBJ* obj = NULL;
 
         if ( mpNameMap ) {
-    // Accessing the map is faster for large collections, so use it if built.
+            // Accessing the map is faster for large collections, so use it if built.
             obj = GetMap(name);
 
-    // If the object name can't be modified then we're done.
-    // Otherwise, there's a chance the object name was modified,
-    // meaning that it can be in the collection but not the map,
-    // or in the wrong place in the map.
-            if ( (obj != NULL) && !obj->CanSetName() )
+            // The map can become stale if the name of any item changed after it
+            // was added to the map. Check if the object's class allows name modifications.
+            // Do this check on found object by default
+            OBJ* canSetObj = obj;
+
+            if ( !canSetObj && (GetCount() > 0))
+                // Object was not found, do check on first object in collection.
+                canSetObj = GetItem(0);
+
+            // Check if object name can be modified. 
+            bool canSetName = canSetObj ? canSetObj->CanSetName() : true;
+
+            // If the object name can't be modified then we're done.
+            // Otherwise, there's a chance the object name was modified,
+            // meaning that it can be in the collection but not the map,
+            // or in the wrong place in the map.
+            if ( !canSetName )
                 return(obj);
 
-    // If the found object's name is the same as the given name
-    // then we're done. Otherwise, this object's name has changed
-    // and a linear search is needed to find the requested object.
+            // If the found object's name is the same as the given name
+            // then we're done. Otherwise, this object's name has changed
+            // and a linear search is needed to find the requested object.
             if ( (obj != NULL) && (Compare(obj->GetName(), name) != 0) )
                 FDO_SAFE_RELEASE( obj );
         }
 
         if ( obj == NULL ) {
-    // No map so do linear search.
+            // No map or map might be stale, so do linear search.
             for ( FdoInt32 i = 0; i < FdoCollection<OBJ, EXC>::GetCount(); i++ ) {
                 OBJ* obj = GetItem(i);
 
@@ -425,9 +437,9 @@
     // Add an element to the map. Elements are keyed by name, which may or may not be case sensitive.
     // Case insensitive names are stored in lower case.
         if ( mbCaseSensitive ) 
-            mpNameMap->insert( std::pair<FdoStringP,OBJ*> ( value->GetName(), value ) );
+            mpNameMap->insert( std::pair<FdoStringP,OBJ*> ( FdoStringP(value->GetName(),true), value ) );
         else
-            mpNameMap->insert( std::pair<FdoStringP,OBJ*> ( FdoStringP(value->GetName()).Lower(), value ) );            
+            mpNameMap->insert( std::pair<FdoStringP,OBJ*> ( FdoStringP(value->GetName(),true).Lower(), value ) );            
     }
 
     // Remove the element at the specified index, from the map

Modified: trunk/Fdo/Unmanaged/Inc/Common/Xml/Attribute.h
===================================================================
--- trunk/Fdo/Unmanaged/Inc/Common/Xml/Attribute.h	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/Unmanaged/Inc/Common/Xml/Attribute.h	2007-03-01 15:58:12 UTC (rev 2617)
@@ -26,6 +26,8 @@
 /// from an XML document.
 class FdoXmlAttribute : public FdoDictionaryElement
 {
+    friend class FdoCommonInternal;
+
 public:
     /// \brief
     /// Constructs an XML Attribute object

Added: trunk/Fdo/Unmanaged/Src/Common/CommonInternal.cpp
===================================================================
--- trunk/Fdo/Unmanaged/Src/Common/CommonInternal.cpp	                        (rev 0)
+++ trunk/Fdo/Unmanaged/Src/Common/CommonInternal.cpp	2007-03-01 15:58:12 UTC (rev 2617)
@@ -0,0 +1,52 @@
+// 
+//  
+//  Copyright (C) 2004-2006  Autodesk, Inc.
+//  
+//  This library is free software; you can redistribute it and/or
+//  modify it under the terms of version 2.1 of the GNU Lesser
+//  General Public License as published by the Free Software Foundation.
+//  
+//  This library is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+//  Lesser General Public License for more details.
+//  
+//  You should have received a copy of the GNU Lesser General Public
+//  License along with this library; if not, write to the Free Software
+//  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+//  
+#include "CommonInternal.h"
+
+FdoDictionaryElement* FdoCommonInternal::CreateDictionaryElement(FdoStringP& name, FdoStringP& value, int dummy)
+{
+    FdoDictionaryElement* elem = new FdoDictionaryElement();
+    elem->mName = name;
+    elem->mValue = value;
+
+    return elem;
+}
+
+FdoXmlAttribute* FdoCommonInternal::CreateXmlAttribute(
+    FdoStringP& name, 
+    FdoStringP& value, 
+    FdoStringP& localName,
+    FdoStringP& uri,
+    FdoStringP& prefix,
+    FdoStringP& valueUri,
+    FdoStringP& localValue,
+    FdoStringP& valuePrefix
+)
+{
+    FdoXmlAttribute* attr = new FdoXmlAttribute();
+    attr->mName = name;
+    attr->mValue = value;
+
+    attr->mLocalName = localName;
+    attr->mUri = uri;
+    attr->mPrefix = prefix;
+    attr->mValueUri = valueUri;
+    attr->mLocalValue = (localValue != L"") ? localValue : value;
+    attr->mValuePrefix = valuePrefix;
+
+    return attr;
+}

Added: trunk/Fdo/Unmanaged/Src/Common/CommonInternal.h
===================================================================
--- trunk/Fdo/Unmanaged/Src/Common/CommonInternal.h	                        (rev 0)
+++ trunk/Fdo/Unmanaged/Src/Common/CommonInternal.h	2007-03-01 15:58:12 UTC (rev 2617)
@@ -0,0 +1,79 @@
+#ifndef FDO_COMMON_INTERNAL_H
+#define FDO_COMMON_INTERNAL_H
+// 
+
+//
+// Copyright (C) 2004-2007  Autodesk, Inc.
+// 
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of version 2.1 of the GNU Lesser
+// General Public License as published by the Free Software Foundation.
+// 
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+// 
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+//
+
+#include <FdoCommon.h>
+#include <Common/Xml/Attribute.h>
+
+// FdoCommonInternal contains various functions internal to FdoCommon.dll.
+// These functions were created as workarounds to preserve binary compatibility.
+// Eventually they should be moved to the classes that they operate on.
+class FdoCommonInternal
+{
+
+public:
+
+    // Faster dictionary element creation function. Re-uses name and value strings,
+    // rather than allocating new memory for them.
+    static FdoDictionaryElement* CreateDictionaryElement(FdoStringP& name, FdoStringP& value, int dummy);
+
+    // Faster XML Attribute creation function. Re-uses name and value strings,
+    // rather than allocating new memory for them.
+    //
+    /// \param name 
+    /// Input unique attribute name. If the attribute name is namespace qualified
+    /// the name is {uri}:{localName}. Otherwise, it is {localName}
+    /// \param value 
+    /// Input attribute value.
+    /// \param localName 
+    /// Input attribute name without namespace qualification.
+    /// \param uri 
+    /// Input uri for the attribute namespace. L"" if the attribute
+    /// name is not namespace qualified.
+    /// \param prefix 
+    /// Input prefix for the attribute namespace. L"" if the attribute
+    /// name is not namespace qualified.
+    /// \param valueUri 
+    /// Input uri for the attribute value's namespace. L"" if the attribute
+    /// value is not namespace qualified.
+    /// \param localValue 
+    /// Input attribute value without namespace qualification.
+    /// \param valuePrefix 
+    /// Input prefix for the attribute value's namespace. L"" if the attribute
+    /// value is not namespace qualified.
+    /// 
+    /// \return
+    /// Returns FdoXmlAttribute
+    /// 
+    static FdoXmlAttribute* CreateXmlAttribute(
+        FdoStringP& name, 
+        FdoStringP& value, 
+        FdoStringP& localName,
+        FdoStringP& uri,
+        FdoStringP& prefix,
+        FdoStringP& valueUri,
+        FdoStringP& localValue,
+        FdoStringP& valuePrefix
+    );
+};
+
+#endif
+
+

Modified: trunk/Fdo/Unmanaged/Src/Common/Makefile.am
===================================================================
--- trunk/Fdo/Unmanaged/Src/Common/Makefile.am	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/Unmanaged/Src/Common/Makefile.am	2007-03-01 15:58:12 UTC (rev 2617)
@@ -28,6 +28,7 @@
 
 libFDOCommon_la_SOURCES = \
   ArrayHelper.cpp \
+  CommonInternal.cpp \
   Context.cpp \
   DictionaryElement.cpp \
   Exception.cpp  \

Modified: trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.cpp
===================================================================
--- trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.cpp	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.cpp	2007-03-01 15:58:12 UTC (rev 2617)
@@ -22,6 +22,7 @@
 #include <xercesc/sax2/XMLReaderFactory.hpp>
 #include <xercesc/sax/SAXParseException.hpp>
 #include <stdio.h>
+#include "../CommonInternal.h"
 
 FdoXmlReaderXrcs::FdoXmlReaderXrcs(FdoIoTextReader* reader) : 
     FdoXmlReader(reader),
@@ -215,21 +216,34 @@
     const XERCES_CPP_NAMESPACE::Attributes &attrs
 )
 {
+    // Reuse current attribute collection if exists and no one else references it.
+    // Saves some memory allocations.
+    if ( mFdoAttrs && (mFdoAttrs->GetRefCount() == 1) ) {
+        mFdoAttrs->Clear();
+    }
+    else {
+        mFdoAttrs = FdoXmlAttributeCollection::Create();
+    }
 
-    FdoXmlAttributesP fdoAttrs = FdoXmlAttributeCollection::Create();
-    FdoSize i;
+    unsigned int i;
+    unsigned int attrCount = attrs.getLength();
 
     // Convert each Xerces XML attribute to a FDO XML attribute.
-    for ( i = 0; i < attrs.getLength(); i++ ) {
+    for ( i = 0; i < attrCount; i++ ) {
+#ifdef _WIN32
+        // on Windows, XMLCh same as FdoString so assign directly to save memory allocations
+        FdoStringP uri((FdoString*) attrs.getURI(i));
+        FdoStringP localName((FdoString*) attrs.getLocalName(i));
+        FdoStringP qName((FdoString*)attrs.getQName(i));
+#else
         FdoStringP uri = FdoXmlUtilXrcs::Xrcs2Unicode(attrs.getURI(i));
         FdoStringP localName = FdoXmlUtilXrcs::Xrcs2Unicode(attrs.getLocalName(i));
         FdoStringP qName = FdoXmlUtilXrcs::Xrcs2Unicode(attrs.getQName(i));
+#endif
         FdoStringP prefix;
 
-        // Parse the namespace prefix from the attribute qualified name. 
-        FdoStringsP tokens = FdoStringCollection::Create( qName, L":" );
-        if ( tokens->GetCount() > 1 ) 
-            prefix = tokens->GetString(0);
+        if ( qName.Contains(L":") )
+            prefix = qName.Left(L":");
 
         // Generate unique name for attribute. For qualified attribute name, concatenate
         // the namespace uri and local name. If name unqualified, use local name as is. 
@@ -237,34 +251,44 @@
                                     uri + L":" + localName :
                                     localName;
 
+#ifdef _WIN32
+        // on Windows, XMLCh same as FdoString so assign directly to save memory allocations
+        FdoStringP value( (FdoString*)(attrs.getValue(i)) );
+#else
         FdoStringP value = FdoXmlUtilXrcs::Xrcs2Unicode(attrs.getValue(i));
+#endif
+
         FdoStringP valueUri;
         FdoStringP valuePrefix;
         FdoStringP localValue = value;
+        FdoStringP leftValue;
+        FdoStringP rightValue;
 
-        tokens = FdoStringCollection::Create( value, L":" );
-        if ( tokens->GetCount() == 2 ) {
-            // Attribute value qualified by namespace prefix.
+        if ( value.Contains(L":") ) {
+            leftValue = value.Left(L":");
+            rightValue = value.Right(L":");
 
-            // Set the uri for the prefix
-            valueUri = PrefixToUri( tokens->GetString(0) );
-            // Set the prefix and unqualified value.
-            if ( valueUri.GetLength() > 0 ) {
-                valuePrefix = tokens->GetString(0);
-                localValue = tokens->GetString(1);
+            if ( !rightValue.Contains(L":") ) {
+                // Attribute value qualified by namespace prefix.
+
+                // Set the prefix and unqualified value.
+                if ( leftValue.GetLength() > 0 ) {
+                    valueUri = PrefixToUri(leftValue);
+                    valuePrefix = leftValue;
+                    localValue = rightValue;
+                }
             }
         }
-        else {
+
+        if ( valuePrefix == L"" ) {
             // Attribute not qualified, set the uri for the 
             // current default namespace.
             valueUri = PrefixToUri( L"" );
-            if ((valueUri.GetLength() > 0) && (tokens->GetCount() > 0))
-                localValue = tokens->GetString(0);
         }
 
-        fdoAttrs->Add( FdoXmlAttributeP( FdoXmlAttribute::Create( 
+        mFdoAttrs->Add( FdoXmlAttributeP( FdoCommonInternal::CreateXmlAttribute( 
             uniqueName, 
-            FdoStringP( FdoXmlUtilXrcs::Xrcs2Unicode(attrs.getValue(i)) ),
+            value,
             localName,
             uri,
             prefix,
@@ -272,16 +296,23 @@
             localValue,
             valuePrefix
         )));
+
     }
-
+ 
     // Call our start element handler. Convert text from Xrcs to FDO Unicode format.
     // Xrcs wide characters are always 2 bytes. FDO wide characters vary according
     // to operating system ( 2 bytes on Windows, 4 on Linux ).
     HandleStartElement( 
+#ifdef _WIN32
+        (FdoString*) uri, 
+        (FdoString*) name, 
+        (FdoString*) qname, 
+#else
         FdoXmlUtilXrcs::Xrcs2Unicode(uri), 
-        FdoXmlUtilXrcs::Xrcs2Unicode(name), 
-        FdoXmlUtilXrcs::Xrcs2Unicode(qname), 
-        fdoAttrs 
+        FdoXmlUtilXrcs::Xrcs2Unicode(name),
+        FdoXmlUtilXrcs::Xrcs2Unicode(qname),
+#endif
+        mFdoAttrs 
     );
 }
 
@@ -293,10 +324,18 @@
 {
     // Call our end element handler.
     HandleEndElement( 
-        FdoStringP(FdoXmlUtilXrcs::Xrcs2Unicode(uri)), 
-        FdoStringP(FdoXmlUtilXrcs::Xrcs2Unicode(name)), 
-        FdoStringP(FdoXmlUtilXrcs::Xrcs2Unicode(qname)) 
+#ifdef _WIN32
+        // on Windows, XMLCh same as FdoString so assign directly to save memory allocations
+        (FdoString*) uri, 
+        (FdoString*) name, 
+        (FdoString*) qname 
+#else
+        FdoXmlUtilXrcs::Xrcs2Unicode(uri), 
+        FdoXmlUtilXrcs::Xrcs2Unicode(name), 
+        FdoXmlUtilXrcs::Xrcs2Unicode(qname) 
+#endif
     );
+
 }
 
 void  FdoXmlReaderXrcs::startPrefixMapping (
@@ -304,9 +343,16 @@
     const XMLCh *const uri
 )
 {
+
     HandleStartPrefixMapping( 
+#ifdef _WIN32
+        // on Windows, XMLCh same as FdoString so assign directly to save memory allocations
+        (FdoString*) prefix, 
+        (FdoString*) uri
+#else
         FdoXmlUtilXrcs::Xrcs2Unicode(prefix), 
         FdoXmlUtilXrcs::Xrcs2Unicode(uri)
+#endif
     );
 }
 
@@ -314,15 +360,22 @@
     const XMLCh *const prefix 
 )
 {
+
     HandleEndPrefixMapping( 
+#ifdef _WIN32
+        // on Windows, XMLCh same as FdoString so assign directly to save memory allocations
+        (FdoString*) prefix
+#else
         FdoXmlUtilXrcs::Xrcs2Unicode(prefix)
+#endif
     );
 }
 
 void  FdoXmlReaderXrcs::characters (const XMLCh *const chars, const unsigned int length)
 {
+
     // Call our element content handler.
-    HandleCharacters( FdoStringP(FdoXmlUtilXrcs::Xrcs2Unicode(chars,length)) );
+    HandleCharacters( FdoXmlUtilXrcs::Xrcs2Unicode(chars,length) );
 }
 
 XERCES_CPP_NAMESPACE::BinInputStream* FdoXmlReaderXrcs::makeStream ()  const

Modified: trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.h
===================================================================
--- trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.h	2007-03-01 06:19:39 UTC (rev 2616)
+++ trunk/Fdo/Unmanaged/Src/Common/Xml/ReaderXrcs.h	2007-03-01 15:58:12 UTC (rev 2617)
@@ -139,6 +139,9 @@
 
     // Semaphore for prevent re-entrant parsing.
     FdoBoolean mParsing;
+
+private:
+    FdoXmlAttributesP mFdoAttrs;
 };
 /// \endcond
 



More information about the fdo-commits mailing list