Skip to content

Commit

Permalink
Merge pull request #40542 from elpaso/nanoptimize-domnode-length
Browse files Browse the repository at this point in the history 
 Micro optimization of DOM loops
  • Loading branch information
@elpaso
elpaso committed Dec 11, 2020
2 parents b412189 + 301d838 commit efc4afc
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 75 deletions.
46 changes: 25 additions & 21 deletions src/core/annotations/qgsannotationmanager.cpp
View file
Expand Up @@ -93,32 +93,36 @@ bool QgsAnnotationManager::readXml( const QDomElement &element, const QgsReadWri

QDomElement annotationsElem = element.firstChildElement( QStringLiteral( "Annotations" ) );

QDomNodeList annotationNodes = annotationsElem.elementsByTagName( QStringLiteral( "Annotation" ) );
for ( int i = 0; i < annotationNodes.size(); ++i )
QDomElement annotationElement = annotationsElem.firstChildElement( QStringLiteral( "Annotation" ) );
while ( ! annotationElement .isNull() )
{
createAnnotationFromXml( annotationNodes.at( i ).toElement(), context );
createAnnotationFromXml( annotationElement, context );
annotationElement = annotationElement.nextSiblingElement( QStringLiteral( "Annotation" ) );
}

// restore old (pre 3.0) project annotations
QDomNodeList oldItemList = element.elementsByTagName( QStringLiteral( "TextAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
if ( annotationElement.isNull() )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
}
oldItemList = element.elementsByTagName( QStringLiteral( "FormAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
}
oldItemList = element.elementsByTagName( QStringLiteral( "HtmlAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
}
oldItemList = element.elementsByTagName( QStringLiteral( "SVGAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
QDomNodeList oldItemList = element.elementsByTagName( QStringLiteral( "TextAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
}
oldItemList = element.elementsByTagName( QStringLiteral( "FormAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
}
oldItemList = element.elementsByTagName( QStringLiteral( "HtmlAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
}
oldItemList = element.elementsByTagName( QStringLiteral( "SVGAnnotationItem" ) );
for ( int i = 0; i < oldItemList.size(); ++i )
{
createAnnotationFromXml( oldItemList.at( i ).toElement(), context );
}
}

return result;
Expand Down
77 changes: 51 additions & 26 deletions src/core/qgslayerdefinition.cpp
View file
Expand Up @@ -78,31 +78,34 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsProject *proj
}
QDomNode layersNode = doc.elementsByTagName( QStringLiteral( "maplayers" ) ).at( 0 );
// replace old children with new ones
QDomNodeList childNodes = layersNode.childNodes();
for ( int i = 0; i < childNodes.size(); i++ )
QDomNode childNode = layersNode.firstChild();
for ( int i = 0; ! childNode.isNull(); i++ )
{
layersNode.replaceChild( clonedSorted.at( i ), childNodes.at( i ) );
layersNode.replaceChild( clonedSorted.at( i ), childNode );
childNode = childNode.nextSibling();
}
}
// if a dependency is missing, we still try to load layers, since dependencies may already be loaded

// IDs of layers should be changed otherwise we may have more then one layer with the same id
// We have to replace the IDs before we load them because it's too late once they are loaded
QDomNodeList treeLayerNodes = doc.elementsByTagName( QStringLiteral( "layer-tree-layer" ) );
for ( int i = 0; i < treeLayerNodes.size(); ++i )
const QDomNodeList treeLayerNodes = doc.elementsByTagName( QStringLiteral( "layer-tree-layer" ) );
QDomNode treeLayerNode = treeLayerNodes.at( 0 );
for ( int i = 0; ! treeLayerNode.isNull(); ++i )
{
QDomNode treeLayerNode = treeLayerNodes.at( i );
treeLayerNode = treeLayerNodes.at( i );
QDomElement treeLayerElem = treeLayerNode.toElement();
QString oldid = treeLayerElem.attribute( QStringLiteral( "id" ) );
QString layername = treeLayerElem.attribute( QStringLiteral( "name" ) );
QString newid = QgsMapLayer::generateId( layername );
treeLayerElem.setAttribute( QStringLiteral( "id" ), newid );

// Replace IDs for map layers
QDomNodeList ids = doc.elementsByTagName( QStringLiteral( "id" ) );
for ( int i = 0; i < ids.size(); ++i )
const QDomNodeList ids = doc.elementsByTagName( QStringLiteral( "id" ) );
QDomNode idnode = ids.at( 0 );
for ( int j = 0; ! idnode.isNull() ; ++j )
{
QDomNode idnode = ids.at( i );
idnode = ids.at( j );
QDomElement idElem = idnode.toElement();
if ( idElem.text() == oldid )
{
Expand All @@ -111,7 +114,7 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsProject *proj
}

// change layer IDs for vector joins
QDomNodeList vectorJoinNodes = doc.elementsByTagName( QStringLiteral( "join" ) ); // TODO: Find a better way of searching for vectorjoins, there might be other <join> elements within the project.
const QDomNodeList vectorJoinNodes = doc.elementsByTagName( QStringLiteral( "join" ) ); // TODO: Find a better way of searching for vectorjoins, there might be other <join> elements within the project.
for ( int j = 0; j < vectorJoinNodes.size(); ++j )
{
QDomNode joinNode = vectorJoinNodes.at( j );
Expand All @@ -123,7 +126,7 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsProject *proj
}

// change IDs of dependencies
QDomNodeList dataDeps = doc.elementsByTagName( QStringLiteral( "dataDependencies" ) );
const QDomNodeList dataDeps = doc.elementsByTagName( QStringLiteral( "dataDependencies" ) );
for ( int i = 0; i < dataDeps.size(); i++ )
{
QDomNodeList layers = dataDeps.at( i ).childNodes();
Expand Down Expand Up @@ -274,13 +277,16 @@ QDomDocument QgsLayerDefinition::exportLayerDefinitionLayers( const QList<QgsMap
QList<QgsMapLayer *> QgsLayerDefinition::loadLayerDefinitionLayers( QDomDocument &document, QgsReadWriteContext &context )
{
QList<QgsMapLayer *> layers;
QDomNodeList layernodes = document.elementsByTagName( QStringLiteral( "maplayer" ) );
for ( int i = 0; i < layernodes.size(); ++i )
QDomElement layerElem = document.documentElement().firstChildElement( QStringLiteral( "projectlayers" ) ).firstChildElement( QStringLiteral( "maplayer" ) );
// For QLR:
if ( layerElem.isNull() )
{
QDomNode layernode = layernodes.at( i );
QDomElement layerElem = layernode.toElement();
layerElem = document.documentElement().firstChildElement( QStringLiteral( "maplayers" ) ).firstChildElement( QStringLiteral( "maplayer" ) );
}

QString type = layerElem.attribute( QStringLiteral( "type" ) );
while ( ! layerElem.isNull() )
{
const QString type = layerElem.attribute( QStringLiteral( "type" ) );
QgsDebugMsg( type );
QgsMapLayer *layer = nullptr;

Expand Down Expand Up @@ -309,6 +315,7 @@ QList<QgsMapLayer *> QgsLayerDefinition::loadLayerDefinitionLayers( QDomDocument
// at a later stage and still retain all the layer properties intact
layer->readLayerXml( layerElem, context );
layers << layer;
layerElem = layerElem.nextSiblingElement( QStringLiteral( "maplayer" ) );
}
return layers;
}
Expand Down Expand Up @@ -344,19 +351,30 @@ void QgsLayerDefinition::DependencySorter::init( const QDomDocument &doc )
QList< QPair<QString, QDomNode> > layersToSort;
QStringList layerIds;

QDomNodeList nl = doc.elementsByTagName( QStringLiteral( "maplayer" ) );
layerIds.reserve( nl.count() );
QDomElement layerElem = doc.documentElement().firstChildElement( QStringLiteral( "projectlayers" ) ).firstChildElement( QStringLiteral( "maplayer" ) );
// For QLR:
if ( layerElem.isNull() )
{
layerElem = doc.documentElement().firstChildElement( QStringLiteral( "maplayers" ) ).firstChildElement( QStringLiteral( "maplayer" ) );
}
// For tests (I don't know if there is a real use case for such a document except for test_qgslayerdefinition.py)
if ( layerElem.isNull() )
{
layerElem = doc.documentElement().firstChildElement( QStringLiteral( "maplayer" ) );
}

const QDomElement &firstElement { layerElem };

QVector<QString> deps; //avoid expensive allocation for list for every iteration
for ( int i = 0; i < nl.count(); i++ )
while ( !layerElem.isNull() )
{
deps.resize( 0 ); // preserve capacity - don't use clear
QDomNode node = nl.item( i );

QString id = node.namedItem( QStringLiteral( "id" ) ).toElement().text();
QString id = layerElem.namedItem( QStringLiteral( "id" ) ).toElement().text();
layerIds << id;

// dependencies for this layer
QDomElement layerDependenciesElem = node.firstChildElement( QStringLiteral( "layerDependencies" ) );
QDomElement layerDependenciesElem = layerElem.firstChildElement( QStringLiteral( "layerDependencies" ) );
if ( !layerDependenciesElem.isNull() )
{
QDomNodeList dependencyList = layerDependenciesElem.elementsByTagName( QStringLiteral( "layer" ) );
Expand All @@ -371,11 +389,14 @@ void QgsLayerDefinition::DependencySorter::init( const QDomDocument &doc )
if ( deps.empty() )
{
sortedLayers << id;
mSortedLayerNodes << node;
mSortedLayerNodes << layerElem;
mSortedLayerIds << id;
}
else
layersToSort << qMakePair( id, node );
{
layersToSort << qMakePair( id, layerElem );
}
layerElem = layerElem.nextSiblingElement( );
}

// check that all dependencies are present
Expand All @@ -389,8 +410,12 @@ void QgsLayerDefinition::DependencySorter::init( const QDomDocument &doc )
{
// some dependencies are not satisfied
mHasMissingDependency = true;
for ( int i = 0; i < nl.size(); i++ )
mSortedLayerNodes << nl.at( i );
layerElem = firstElement;
while ( ! layerElem.isNull() )
{
mSortedLayerNodes << layerElem;
layerElem = layerElem.nextSiblingElement( );
}
mSortedLayerIds = layerIds;
return;
}
Expand Down
57 changes: 29 additions & 28 deletions src/core/qgsproject.cpp
View file
Expand Up @@ -913,7 +913,6 @@ void dump_( const QgsProjectPropertyKey &topQgsPropertyKey )
topQgsPropertyKey.dump();
}


/**
Restore any optional properties found in "doc" to "properties".
Expand Down Expand Up @@ -955,7 +954,7 @@ void _getProperties( const QDomDocument &doc, QgsProjectPropertyKey &project_pro

QDomNodeList scopes = propertiesElem.childNodes();

if ( scopes.count() < 1 )
if ( propertiesElem.firstChild().isNull() )
{
QgsDebugMsg( QStringLiteral( "empty ``properties'' XML tag ... bailing" ) );
return;
Expand Down Expand Up @@ -994,18 +993,16 @@ QgsPropertyCollection getDataDefinedServerProperties( const QDomDocument &doc, c
*/
static void _getTitle( const QDomDocument &doc, QString &title )
{
QDomNodeList nl = doc.elementsByTagName( QStringLiteral( "title" ) );
QDomElement titleNode = doc.documentElement().firstChildElement( QStringLiteral( "title" ) );

title.clear(); // by default the title will be empty

if ( !nl.count() )
if ( titleNode.isNull() )
{
QgsDebugMsgLevel( QStringLiteral( "unable to find title element" ), 2 );
return;
}

QDomNode titleNode = nl.item( 0 ); // there should only be one, so zeroth element OK

if ( !titleNode.hasChildNodes() ) // if not, then there's no actual text
{
QgsDebugMsgLevel( QStringLiteral( "unable to find title element" ), 2 );
Expand Down Expand Up @@ -1092,11 +1089,11 @@ bool QgsProject::_getMapLayers( const QDomDocument &doc, QList<QDomNode> &broken
// Layer order is set by the restoring the legend settings from project file.
// This is done on the 'readProject( ... )' signal

QDomNodeList nl = doc.elementsByTagName( QStringLiteral( "maplayer" ) );
QDomElement layerElement = doc.documentElement().firstChildElement( QStringLiteral( "projectlayers" ) ).firstChildElement( QStringLiteral( "maplayer" ) );

// process the map layer nodes

if ( 0 == nl.count() ) // if we have no layers to process, bail
if ( layerElement.isNull() ) // if we have no layers to process, bail
{
return true; // Decided to return "true" since it's
// possible for there to be a project with no
Expand All @@ -1107,6 +1104,13 @@ bool QgsProject::_getMapLayers( const QDomDocument &doc, QList<QDomNode> &broken
}

bool returnStatus = true;
int numLayers = 0;

while ( ! layerElement.isNull() )
{
numLayers++;
layerElement = layerElement.nextSiblingElement( QStringLiteral( "maplayer" ) );
}

// order layers based on their dependencies
QgsScopedRuntimeProfile profile( tr( "Sorting layers" ), QStringLiteral( "projectload" ) );
Expand All @@ -1118,7 +1122,7 @@ bool QgsProject::_getMapLayers( const QDomDocument &doc, QList<QDomNode> &broken
if ( depSorter.hasMissingDependency() )
returnStatus = false;

emit layerLoaded( 0, nl.count() );
emit layerLoaded( 0, numLayers );

const QVector<QDomNode> sortedLayerNodes = depSorter.sortedLayerNodes();
const int totalLayerCount = sortedLayerNodes.count();
Expand Down Expand Up @@ -1525,11 +1529,11 @@ bool QgsProject::readProjectFile( const QString &filename, QgsProject::ReadFlags
QgsMessageLog::logMessage( tr( "Project Variables Invalid" ), tr( "The project contains invalid variable settings." ) );
}

QDomNodeList nl = doc->elementsByTagName( QStringLiteral( "projectMetadata" ) );
if ( !nl.isEmpty() )
QDomElement element = doc->documentElement().firstChildElement( QStringLiteral( "projectMetadata" ) );

if ( !element.isNull() )
{
QDomElement metadataElement = nl.at( 0 ).toElement();
mMetadata.readMetadataXml( metadataElement );
mMetadata.readMetadataXml( element );
}
else
{
Expand All @@ -1543,28 +1547,25 @@ bool QgsProject::readProjectFile( const QString &filename, QgsProject::ReadFlags
}
emit metadataChanged();

nl = doc->elementsByTagName( QStringLiteral( "autotransaction" ) );
if ( nl.count() )
element = doc->documentElement().firstChildElement( QStringLiteral( "autotransaction" ) );
if ( ! element.isNull() )
{
QDomElement transactionElement = nl.at( 0 ).toElement();
if ( transactionElement.attribute( QStringLiteral( "active" ), QStringLiteral( "0" ) ).toInt() == 1 )
if ( element.attribute( QStringLiteral( "active" ), QStringLiteral( "0" ) ).toInt() == 1 )
mAutoTransaction = true;
}

nl = doc->elementsByTagName( QStringLiteral( "evaluateDefaultValues" ) );
if ( nl.count() )
element = doc->documentElement().firstChildElement( QStringLiteral( "evaluateDefaultValues" ) );
if ( !element.isNull() )
{
QDomElement evaluateDefaultValuesElement = nl.at( 0 ).toElement();
if ( evaluateDefaultValuesElement.attribute( QStringLiteral( "active" ), QStringLiteral( "0" ) ).toInt() == 1 )
if ( element.attribute( QStringLiteral( "active" ), QStringLiteral( "0" ) ).toInt() == 1 )
mEvaluateDefaultValues = true;
}

// Read trust layer metadata config in the project
nl = doc->elementsByTagName( QStringLiteral( "trust" ) );
if ( nl.count() )
element = doc->documentElement().firstChildElement( QStringLiteral( "trust" ) );
if ( !element.isNull() )
{
QDomElement trustElement = nl.at( 0 ).toElement();
if ( trustElement.attribute( QStringLiteral( "active" ), QStringLiteral( "0" ) ).toInt() == 1 )
if ( element.attribute( QStringLiteral( "active" ), QStringLiteral( "0" ) ).toInt() == 1 )
mTrustLayerMetadata = true;
}

Expand Down Expand Up @@ -2850,11 +2851,10 @@ bool QgsProject::createEmbeddedLayer( const QString &layerId, const QString &pro
return false;
}

QDomNodeList mapLayerNodes = projectLayersElem.elementsByTagName( QStringLiteral( "maplayer" ) );
for ( int i = 0; i < mapLayerNodes.size(); ++i )
QDomElement mapLayerElem = projectLayersElem.firstChildElement( QStringLiteral( "maplayer" ) );
while ( ! mapLayerElem.isNull() )
{
// get layer id
QDomElement mapLayerElem = mapLayerNodes.at( i ).toElement();
QString id = mapLayerElem.firstChildElement( QStringLiteral( "id" ) ).text();
if ( id == layerId )
{
Expand All @@ -2876,6 +2876,7 @@ bool QgsProject::createEmbeddedLayer( const QString &layerId, const QString &pro
return false;
}
}
mapLayerElem = mapLayerElem.nextSiblingElement( QStringLiteral( "maplayer" ) );
}

return false;
Expand Down
Binary file modified tests/testdata/qgis_server/test_project_wms_filter.gpkg
View file
Binary file not shown.

0 comments on commit efc4afc

Please sign in to comment.