Skip to content

Commit 7f97a2f

Browse files
rouaultrouault
authoredJun 3, 2018
Merge pull request #7146 from rouault/fix_18342
[OGR provider] Improve performance of subLayers(), particularly on FileGDB with the proprietary driver (fixes #18342)
·
ltr-3_40final-3_2_0
2 parents 9115d7f + 55aa7a8 commit 7f97a2f

File tree

2 files changed

+41
-24
lines changed

2 files changed

+41
-24
lines changed
 

‎src/providers/ogr/qgsogrprovider.cpp

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,11 @@ void QgsOgrProvider::repack()
198198
QString errCause;
199199
if ( mLayerName.isNull() )
200200
{
201-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerIndex, errCause );
201+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerIndex, errCause, true );
202202
}
203203
else
204204
{
205-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerName, errCause );
205+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, QStringList(), mLayerName, errCause, true );
206206
}
207207
208208
if ( !mOgrOrigLayer )
@@ -837,18 +837,30 @@ QStringList QgsOgrProvider::subLayers() const
837837
}
838838
else
839839
{
840+
// In case there is no free opened dataset in the cache, keep the first
841+
// layer alive while we iterate over the other layers, so that we can
842+
// reuse the same dataset. Can help in a particular with a FileGDB with
843+
// the FileGDB driver
844+
QgsOgrLayerUniquePtr firstLayer;
840845
for ( unsigned int i = 0; i < layerCount() ; i++ )
841846
{
842847
QString errCause;
843848
QgsOgrLayerUniquePtr layer = QgsOgrProviderUtils::getLayer( mOgrOrigLayer->datasetName(),
844849
mOgrOrigLayer->updateMode(),
845850
mOgrOrigLayer->options(),
846851
i,
847-
errCause );
852+
errCause,
853+
// do not check timestamp beyond the first
854+
// layer
855+
firstLayer == nullptr );
848856
if ( !layer )
849857
continue;
850858

851859
addSubLayerDetailsToSubLayerList( i, layer.get() );
860+
if ( firstLayer == nullptr )
861+
{
862+
firstLayer = std::move( layer );
863+
}
852864
}
853865
}
854866
return mSubLayerList;
@@ -4025,11 +4037,11 @@ void QgsOgrProvider::open( OpenMode mode )
40254037
// has precedence over the layerid if both are given.
40264038
if ( !mLayerName.isNull() )
40274039
{
4028-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerName, errCause );
4040+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerName, errCause, true );
40294041
}
40304042
else
40314043
{
4032-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerIndex, errCause );
4044+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, true, options, mLayerIndex, errCause, true );
40334045
}
40344046
}
40354047

@@ -4050,11 +4062,11 @@ void QgsOgrProvider::open( OpenMode mode )
40504062
// try to open read-only
40514063
if ( !mLayerName.isNull() )
40524064
{
4053-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause );
4065+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause, true );
40544066
}
40554067
else
40564068
{
4057-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause );
4069+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause, true );
40584070
}
40594071
}
40604072

@@ -4125,11 +4137,11 @@ void QgsOgrProvider::open( OpenMode mode )
41254137
// try to open read-only
41264138
if ( !mLayerName.isNull() )
41274139
{
4128-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause );
4140+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerName, errCause, true );
41294141
}
41304142
else
41314143
{
4132-
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause );
4144+
mOgrOrigLayer = QgsOgrProviderUtils::getLayer( mFilePath, false, QStringList(), mLayerIndex, errCause, true );
41334145
}
41344146

41354147
mWriteAccess = false;
@@ -4351,18 +4363,19 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
43514363
errCause = QObject::tr( "Cannot find layer %1." ).arg( layerIndex );
43524364
return nullptr;
43534365
}
4354-
return getLayer( dsName, iter.key().updateMode, iter.key().options, layerName, errCause );
4366+
return getLayer( dsName, iter.key().updateMode, iter.key().options, layerName, errCause, true );
43554367
}
43564368
}
43574369
}
4358-
return getLayer( dsName, false, QStringList(), layerIndex, errCause );
4370+
return getLayer( dsName, false, QStringList(), layerIndex, errCause, true );
43594371
}
43604372

43614373
QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
43624374
bool updateMode,
43634375
const QStringList &options,
43644376
int layerIndex,
4365-
QString &errCause )
4377+
QString &errCause,
4378+
bool checkModificationDateAgainstCache )
43664379
{
43674380
QMutexLocker locker( &sGlobalMutex );
43684381

@@ -4403,7 +4416,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
44034416
errCause = QObject::tr( "Cannot find layer %1." ).arg( layerIndex );
44044417
return nullptr;
44054418
}
4406-
return getLayer( dsName, updateMode, options, layerName, errCause );
4419+
return getLayer( dsName, updateMode, options, layerName, errCause,
4420+
checkModificationDateAgainstCache );
44074421
}
44084422
}
44094423

@@ -4480,7 +4494,7 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
44804494
}
44814495
}
44824496
}
4483-
return getLayer( dsName, false, QStringList(), layerName, errCause );
4497+
return getLayer( dsName, false, QStringList(), layerName, errCause, true );
44844498
}
44854499

44864500
static QDateTime getLastModified( const QString &dsName )
@@ -4822,7 +4836,8 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
48224836
bool updateMode,
48234837
const QStringList &options,
48244838
const QString &layerName,
4825-
QString &errCause )
4839+
QString &errCause,
4840+
bool checkModificationDateAgainstCache )
48264841
{
48274842
QMutexLocker locker( &sGlobalMutex );
48284843

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

53175332
if ( !layerName.isEmpty() )
53185333
{
5319-
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerName, errCause );
5334+
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerName, errCause, true );
53205335
}
53215336
else
53225337
{
5323-
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerIndex, errCause );
5338+
return QgsOgrProviderUtils::getLayer( filePath, true, QStringList(), layerIndex, errCause, true );
53245339
}
53255340
}
53265341

‎src/providers/ogr/qgsogrprovider.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -394,30 +394,32 @@ class QgsOgrProviderUtils
394394
//! Wrapper for GDALClose()
395395
static void GDALCloseWrapper( GDALDatasetH mhDS );
396396

397-
//! 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
397+
//! Open a layer given by name, potentially reusing an existing GDALDatasetH if it doesn't already use that layer.
398398
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
399399
const QString &layerName,
400400
QString &errCause );
401401

402402

403-
//! 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
403+
//! 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.
404404
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
405405
bool updateMode,
406406
const QStringList &options,
407407
const QString &layerName,
408-
QString &errCause );
408+
QString &errCause,
409+
bool checkModificationDateAgainstCache );
409410

410-
//! 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
411+
//! Open a layer given by index, potentially reusing an existing GDALDatasetH if it doesn't already use that layer.
411412
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
412413
int layerIndex,
413414
QString &errCause );
414415

415-
//! 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
416+
//! 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.
416417
static QgsOgrLayerUniquePtr getLayer( const QString &dsName,
417418
bool updateMode,
418419
const QStringList &options,
419420
int layerIndex,
420-
QString &errCause );
421+
QString &errCause,
422+
bool checkModificationDateAgainstCache );
421423

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

0 commit comments

Comments
 (0)
Please sign in to comment.