Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Avoid installing QgsCPLHTTPFetchOverrider for every ogr feature
fetched if we can avoid it

This is a relatively expensive operation to do (specifically
the calls to QgsCPLHTTPFetchOverrider::setAttribute) compared
with the actual costs of fetching features from GDAL

Avoid creating a new override for every feature, if we detect
that its safe to use an override created in the OGR feature
iterator constructor.
  • Loading branch information
nyalldawson committed May 4, 2023
1 parent 6bce647 commit a8fd082
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 10 deletions.
21 changes: 15 additions & 6 deletions src/core/providers/ogr/qgsogrfeatureiterator.cpp
Expand Up @@ -18,9 +18,8 @@
#include "qgsogrexpressioncompiler.h"
#include "qgssqliteexpressioncompiler.h"

#include "qgscplhttpfetchoverrider.h"
#include "qgsogrutils.h"
#include "qgsapplication.h"
#include "qgscplhttpfetchoverrider.h"
#include "qgsgeometry.h"
#include "qgsexception.h"
#include "qgswkbtypes.h"
Expand Down Expand Up @@ -60,8 +59,8 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
( mRequest.filterType() != QgsFeatureRequest::FilterType::FilterFid
&& mRequest.filterType() != QgsFeatureRequest::FilterType::FilterFids );

QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg );
QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrFeatureIterator" ) )
mCplHttpFetchOverrider = std::make_unique< QgsCPLHTTPFetchOverrider >( mAuthCfg );
QgsSetCPLHTTPFetchOverriderInitiatorClass( *mCplHttpFetchOverrider, QStringLiteral( "QgsOgrFeatureIterator" ) )

for ( const auto &id : mRequest.filterFids() )
{
Expand Down Expand Up @@ -401,14 +400,24 @@ bool QgsOgrFeatureIterator::checkFeature( gdal::ogr_feature_unique_ptr &fet, Qgs
void QgsOgrFeatureIterator::setInterruptionChecker( QgsFeedback *interruptionChecker )
{
mInterruptionChecker = interruptionChecker;
if ( mCplHttpFetchOverrider && QThread::currentThread() == mCplHttpFetchOverrider->thread() )
{
mCplHttpFetchOverrider->setFeedback( interruptionChecker );
}
}

bool QgsOgrFeatureIterator::fetchFeature( QgsFeature &feature )
{
QMutexLocker locker( mSharedDS ? &mSharedDS->mutex() : nullptr );

QgsCPLHTTPFetchOverrider oCPLHTTPFetcher( mAuthCfg, mInterruptionChecker );
QgsSetCPLHTTPFetchOverriderInitiatorClass( oCPLHTTPFetcher, QStringLiteral( "QgsOgrFeatureIterator" ) )
// if we are on the same thread as the iterator was created in, we don't need to initializer another
// QgsCPLHTTPFetchOverrider (which is expensive)
std::unique_ptr< QgsCPLHTTPFetchOverrider > localHttpFetchOverride;
if ( QThread::currentThread() != mCplHttpFetchOverrider->thread() )
{
localHttpFetchOverride = std::make_unique< QgsCPLHTTPFetchOverrider >( mAuthCfg, mInterruptionChecker );
QgsSetCPLHTTPFetchOverriderInitiatorClass( *localHttpFetchOverride, QStringLiteral( "QgsOgrFeatureIterator" ) )
}

feature.setValid( false );

Expand Down
5 changes: 5 additions & 0 deletions src/core/providers/ogr/qgsogrfeatureiterator.h
Expand Up @@ -27,12 +27,15 @@
#include <set>
#include "qgis_sip.h"


///@cond PRIVATE
#define SIP_NO_FILE

class QgsOgrFeatureIterator;
class QgsOgrProvider;
class QgsOgrDataset;
class QgsCPLHTTPFetchOverrider;

using QgsOgrDatasetSharedPtr = std::shared_ptr< QgsOgrDataset>;

class QgsOgrFeatureSource final: public QgsAbstractFeatureSource
Expand Down Expand Up @@ -121,6 +124,8 @@ class QgsOgrFeatureIterator final: public QgsAbstractFeatureIteratorFromSource<Q
QgsGeometry mDistanceWithinGeom;
std::unique_ptr< QgsGeometryEngine > mDistanceWithinEngine;

std::unique_ptr< QgsCPLHTTPFetchOverrider > mCplHttpFetchOverrider;

QVector< int > mRequestAttributes;

bool fetchFeatureWithId( QgsFeatureId id, QgsFeature &feature ) const;
Expand Down
17 changes: 14 additions & 3 deletions src/core/qgscplhttpfetchoverrider.cpp
Expand Up @@ -20,9 +20,10 @@
#include "cpl_http.h"
#include "gdal.h"

QgsCPLHTTPFetchOverrider::QgsCPLHTTPFetchOverrider( const QString &authCfg, QgsFeedback *feedback ):
mAuthCfg( authCfg ),
mFeedback( feedback )
QgsCPLHTTPFetchOverrider::QgsCPLHTTPFetchOverrider( const QString &authCfg, QgsFeedback *feedback )
: mAuthCfg( authCfg )
, mFeedback( feedback )
, mThread( QThread::currentThread() )
{
CPLHTTPPushFetchCallback( QgsCPLHTTPFetchOverrider::callback, this );
}
Expand Down Expand Up @@ -188,3 +189,13 @@ void QgsCPLHTTPFetchOverrider::setAttribute( QNetworkRequest::Attribute code, co
{
mAttributes[code] = value;
}

void QgsCPLHTTPFetchOverrider::setFeedback( QgsFeedback *feedback )
{
mFeedback = feedback;
}

QThread *QgsCPLHTTPFetchOverrider::thread() const
{
return mThread;
}
19 changes: 18 additions & 1 deletion src/core/qgscplhttpfetchoverrider.h
Expand Up @@ -20,6 +20,7 @@

#include <QNetworkRequest>
#include <QString>
#include <QPointer>
#include "qgsnetworkaccessmanager.h" // for QgsSetRequestInitiatorClass

#include "cpl_http.h"
Expand All @@ -28,7 +29,7 @@
class QgsFeedback;

#ifndef SIP_RUN
#define QgsSetCPLHTTPFetchOverriderInitiatorClass(overrider, _class) QgsSetRequestInitiatorClass(overrider, _class)
#define QgsSetCPLHTTPFetchOverriderInitiatorClass(overrider, _class) QgsSetRequestInitiatorClass((overrider), _class)
#endif

/**
Expand All @@ -53,6 +54,20 @@ class QgsCPLHTTPFetchOverrider
//! Define attribute that must be forwarded to the actual QNetworkRequest
void setAttribute( QNetworkRequest::Attribute code, const QVariant &value );

/**
* Sets the \a feedback cancellation object for the redirection.
*
* \since QGIS 3.32
*/
void setFeedback( QgsFeedback *feedback );

/**
* Returns the thread associated with the overrider.
*
* \since QGIS 3.32
*/
QThread *thread() const;

private:

#if GDAL_VERSION_NUM >= GDAL_COMPUTE_VERSION(3,2,0)
Expand All @@ -69,6 +84,8 @@ class QgsCPLHTTPFetchOverrider

QgsFeedback *mFeedback = nullptr;

QPointer< QThread > mThread;

std::map<QNetworkRequest::Attribute, QVariant> mAttributes;
};

Expand Down

0 comments on commit a8fd082

Please sign in to comment.