Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #7146 from rouault/fix_18342
[OGR provider] Improve performance of subLayers(), particularly on FileGDB with the proprietary driver (fixes #18342)
  • Loading branch information
rouault committed Jun 3, 2018
2 parents 9115d7f + 55aa7a8 commit 7f97a2f
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 24 deletions.
51 changes: 33 additions & 18 deletions src/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -198,11 +198,11 @@ void QgsOgrProvider::repack()
QString errCause;
if ( mLayerName.isNull() )
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerIndex, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerIndex, errCause, true );
}
else
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerName, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerName, errCause, true );
}
if ( !mOgrOrigLayer )
Expand Down Expand Up @@ -837,18 +837,30 @@ QStringList QgsOgrProvider::subLayers() const
}
else
{
// In case there is no free opened dataset in the cache, keep the first
// layer alive while we iterate over the other layers, so that we can
// reuse the same dataset. Can help in a particular with a FileGDB with
// the FileGDB driver
QgsOgrLayerUniquePtr firstLayer;
for ( unsigned int i = 0; i < layerCount() ; i++ )
{
QString errCause;
QgsOgrLayerUniquePtr layer = QgsOgrProviderUtils::getLayer( mOgrOrigLayer->datasetName(),
mOgrOrigLayer->updateMode(),
mOgrOrigLayer->options(),
i,
errCause );
errCause,
// do not check timestamp beyond the first
// layer
firstLayer == nullptr );
if ( !layer )
continue;

addSubLayerDetailsToSubLayerList( i, layer.get() );
if ( firstLayer == nullptr )
{
firstLayer = std::move( layer );
}
}
}
return mSubLayerList;
Expand Down Expand Up @@ -4025,11 +4037,11 @@ void QgsOgrProvider::open( OpenMode mode )
// has precedence over the layerid if both are given.
if ( !mLayerName.isNull() )
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerName, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerName, errCause, true );
}
else
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerIndex, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerIndex, errCause, true );
}
}

Expand All @@ -4050,11 +4062,11 @@ void QgsOgrProvider::open( OpenMode mode )
// try to open read-only
if ( !mLayerName.isNull() )
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause, true );
}
else
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause, true );
}
}

Expand Down Expand Up @@ -4125,11 +4137,11 @@ void QgsOgrProvider::open( OpenMode mode )
// try to open read-only
if ( !mLayerName.isNull() )
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause, true );
}
else
{
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause );
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause, true );
}

mWriteAccess = false;
Expand Down Expand Up @@ -4351,18 +4363,19 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
errCause = QObject::tr( "Cannot find layer %1." ).arg( layerIndex );
return nullptr;
}
return getLayer( dsName, iter.key().updateMode, iter.key().options, layerName, errCause );
return getLayer( dsName, iter.key().updateMode, iter.key().options, layerName, errCause, true );
}
}
}
return getLayer( dsName, false, QStringList(), layerIndex, errCause );
return getLayer( dsName, false, QStringList(), layerIndex, errCause, true );
}

QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
bool updateMode,
const QStringList &options,
int layerIndex,
QString &errCause )
QString &errCause,
bool checkModificationDateAgainstCache )
{
QMutexLocker locker( &sGlobalMutex );

Expand Down Expand Up @@ -4403,7 +4416,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
errCause = QObject::tr( "Cannot find layer %1." ).arg( layerIndex );
return nullptr;
}
return getLayer( dsName, updateMode, options, layerName, errCause );
return getLayer( dsName, updateMode, options, layerName, errCause,
checkModificationDateAgainstCache );
}
}

Expand Down Expand Up @@ -4480,7 +4494,7 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
}
}
}
return getLayer( dsName, false, QStringList(), layerName, errCause );
return getLayer( dsName, false, QStringList(), layerName, errCause, true );
}

static QDateTime getLastModified( const QString &dsName )
Expand Down Expand Up @@ -4822,7 +4836,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
bool updateMode,
const QStringList &options,
const QString &layerName,
QString &errCause )
QString &errCause,
bool checkModificationDateAgainstCache )
{
QMutexLocker locker( &sGlobalMutex );

Expand All @@ -4840,7 +4855,7 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
auto iter = sMapSharedDS.find( ident );
if ( iter != sMapSharedDS.end() )
{
if ( !canUseOpenedDatasets( dsName ) )
if ( checkModificationDateAgainstCache && !canUseOpenedDatasets( dsName ) )
{
QgsDebugMsg( QString( "Cannot reuse existing opened dataset(s) on %1 since it has been modified" ).arg( dsName ) );
invalidateCachedDatasets( dsName );
Expand Down Expand Up @@ -5316,11 +5331,11 @@ QgsOgrLayerUniquePtr LoadDataSourceAndLayer( const QString &uri,

if ( !layerName.isEmpty() )
{
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerName, errCause );
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerName, errCause, true );
}
else
{
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerIndex, errCause );
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerIndex, errCause, true );
}
}

Expand Down
14 changes: 8 additions & 6 deletions src/providers/ogr/qgsogrprovider.h
Expand Up @@ -394,30 +394,32 @@ class QgsOgrProviderUtils
//! Wrapper for GDALClose()
static void GDALCloseWrapper( GDALDatasetH mhDS );

//! Open a layer given by name, potentially reusing an existing GDALDatasetH if it doesn't already use that layer. release() should be called when done with the object
//! Open a layer given by name, potentially reusing an existing GDALDatasetH if it doesn't already use that layer.
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
const QString &layerName,
QString &errCause );


//! Open a layer given by name, potentially reusing an existing GDALDatasetH if it has been opened with the same (updateMode, options) tuple and doesn't already use that layer. release() should be called when done with the object
//! Open a layer given by name, potentially reusing an existing GDALDatasetH if it has been opened with the same (updateMode, options) tuple and doesn't already use that layer.
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
bool updateMode,
const QStringList &options,
const QString &layerName,
QString &errCause );
QString &errCause,
bool checkModificationDateAgainstCache );

//! Open a layer given by index, potentially reusing an existing GDALDatasetH if it doesn't already use that layer. release() should be called when done with the object
//! Open a layer given by index, potentially reusing an existing GDALDatasetH if it doesn't already use that layer.
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
int layerIndex,
QString &errCause );

//! Open a layer given by index, potentially reusing an existing GDALDatasetH if it has been opened with the same (updateMode, options) tuple and doesn't already use that layer. release() should be called when done with the object
//! Open a layer given by index, potentially reusing an existing GDALDatasetH if it has been opened with the same (updateMode, options) tuple and doesn't already use that layer.
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
bool updateMode,
const QStringList &options,
int layerIndex,
QString &errCause );
QString &errCause,
bool checkModificationDateAgainstCache );

//! Returns a QgsOgrLayer* with a SQL result layer
static QgsOgrLayerUniquePtr getSqlLayer( QgsOgrLayer *baseLayer, OGRLayerH hSqlLayer, const QString &sql );
Expand Down

0 comments on commit 7f97a2f

Please sign in to comment.