From 1c1e46029a1db5d6e9bf002faa11695498f920b5 Mon Sep 17 00:00:00 2001 From: Alexander Bruy Date: Mon, 24 Apr 2023 09:52:13 +0300 Subject: [PATCH] address review --- .../processing/qgsprocessingcontext.sip.in | 24 +++++--- .../plugins/processing/algs/qgis/TilesXYZ.py | 2 +- .../processing/pdal/qgspdalalgorithmbase.cpp | 2 +- src/core/processing/qgsprocessingcontext.cpp | 8 +-- src/core/processing/qgsprocessingcontext.h | 24 +++++--- .../qgsprocessingalgorithmdialogbase.cpp | 8 +-- .../qgsprocessingalgorithmdialogbase.h | 4 +- .../analysis/testqgsprocessingpdalalgs.cpp | 56 +++++++++---------- 8 files changed, 72 insertions(+), 56 deletions(-) diff --git a/python/core/auto_generated/processing/qgsprocessingcontext.sip.in b/python/core/auto_generated/processing/qgsprocessingcontext.sip.in index 60a1dc48840..7daac0cc516 100644 --- a/python/core/auto_generated/processing/qgsprocessingcontext.sip.in +++ b/python/core/auto_generated/processing/qgsprocessingcontext.sip.in @@ -611,20 +611,23 @@ for all temporary files created during algorithm execution. .. versionadded:: 3.32 %End - int numberOfThreads() const; + int maximumThreads() const; %Docstring Returns the (optional) number of threads to use when running algorithms. -If set, this overrides the standard global Processing number of threads setting. -Note that if algorithm implementation does not support multhreaded execution, this -setting will be ignored. +.. warning:: -.. seealso:: :py:func:`setNumberOfThreads` + Not all algorithms which support multithreaded execution will + respect this setting, depending on the multi-threading framework in use. + Multithreaded algorithms must check this value and adapt their thread + handling accordingly -- the setting will not be automatically applied. + +.. seealso:: :py:func:`setMaximumThreads` .. versionadded:: 3.32 %End - void setNumberOfThreads( int threads ); + void setMaximumThreads( int threads ); %Docstring Sets the (optional) number of ``threads`` to use when running algorithms. @@ -632,7 +635,14 @@ If set, this overrides the standard global Processing number of threads setting. Note that if algorithm implementation does not support multhreaded execution, this setting will be ignored. -.. seealso:: :py:func:`numberOfThreads` +.. warning:: + + Not all algorithms which support multithreaded execution will + respect this setting, depending on the multi-threading framework in use. + Multithreaded algorithms must check this value and adapt their thread + handling accordingly -- the setting will not be automatically applied. + +.. seealso:: :py:func:`maximumThreads` .. versionadded:: 3.32 %End diff --git a/python/plugins/processing/algs/qgis/TilesXYZ.py b/python/plugins/processing/algs/qgis/TilesXYZ.py index c5e3bae0229..49e01c852e8 100644 --- a/python/plugins/processing/algs/qgis/TilesXYZ.py +++ b/python/plugins/processing/algs/qgis/TilesXYZ.py @@ -252,7 +252,7 @@ class TilesXYZAlgorithmBase(QgisAlgorithm): self.tile_format = self.formats[self.parameterAsEnum(parameters, self.TILE_FORMAT, context)] self.quality = self.parameterAsInt(parameters, self.QUALITY, context) self.metatilesize = self.parameterAsInt(parameters, self.METATILESIZE, context) - self.maxThreads = context.numThreads() + self.maxThreads = context.maximumThreads() try: self.tile_width = self.parameterAsInt(parameters, self.TILE_WIDTH, context) self.tile_height = self.parameterAsInt(parameters, self.TILE_HEIGHT, context) diff --git a/src/analysis/processing/pdal/qgspdalalgorithmbase.cpp b/src/analysis/processing/pdal/qgspdalalgorithmbase.cpp index b81c79d887b..32e5d9b053b 100644 --- a/src/analysis/processing/pdal/qgspdalalgorithmbase.cpp +++ b/src/analysis/processing/pdal/qgspdalalgorithmbase.cpp @@ -90,7 +90,7 @@ void QgsPdalAlgorithmBase::applyCommonParameters( QStringList &arguments, QgsCoo void QgsPdalAlgorithmBase::applyThreadsParameter( QStringList &arguments, QgsProcessingContext &context ) { - const int numThreads = context.numberOfThreads(); + const int numThreads = context.maximumThreads(); if ( numThreads ) { diff --git a/src/core/processing/qgsprocessingcontext.cpp b/src/core/processing/qgsprocessingcontext.cpp index 9a6a04f68b4..4ce57c6e337 100644 --- a/src/core/processing/qgsprocessingcontext.cpp +++ b/src/core/processing/qgsprocessingcontext.cpp @@ -157,14 +157,14 @@ void QgsProcessingContext::setTemporaryFolder( const QString &folder ) mTemporaryFolderOverride = folder; } -int QgsProcessingContext::numberOfThreads() const +int QgsProcessingContext::maximumThreads() const { - return mThreadsToUse; + return mMaximumThreads; } -void QgsProcessingContext::setNumberOfThreads( int threads ) +void QgsProcessingContext::setMaximumThreads( int threads ) { - mThreadsToUse = threads; + mMaximumThreads = threads; } QVariantMap QgsProcessingContext::exportToMap() const diff --git a/src/core/processing/qgsprocessingcontext.h b/src/core/processing/qgsprocessingcontext.h index 3c8698805c1..7dabf89c448 100644 --- a/src/core/processing/qgsprocessingcontext.h +++ b/src/core/processing/qgsprocessingcontext.h @@ -101,7 +101,7 @@ class CORE_EXPORT QgsProcessingContext mAreaUnit = other.mAreaUnit; mLogLevel = other.mLogLevel; mTemporaryFolderOverride = other.mTemporaryFolderOverride; - mThreadsToUse = other.mThreadsToUse; + mMaximumThreads = other.mMaximumThreads; } /** @@ -684,14 +684,15 @@ class CORE_EXPORT QgsProcessingContext /** * Returns the (optional) number of threads to use when running algorithms. * - * If set, this overrides the standard global Processing number of threads setting. - * Note that if algorithm implementation does not support multhreaded execution, this - * setting will be ignored. + * \warning Not all algorithms which support multithreaded execution will + * respect this setting, depending on the multi-threading framework in use. + * Multithreaded algorithms must check this value and adapt their thread + * handling accordingly -- the setting will not be automatically applied. * - * \see setNumberOfThreads() + * \see setMaximumThreads() * \since QGIS 3.32 */ - int numberOfThreads() const; + int maximumThreads() const; /** * Sets the (optional) number of \a threads to use when running algorithms. @@ -700,10 +701,15 @@ class CORE_EXPORT QgsProcessingContext * Note that if algorithm implementation does not support multhreaded execution, this * setting will be ignored. * - * \see numberOfThreads() + * \warning Not all algorithms which support multithreaded execution will + * respect this setting, depending on the multi-threading framework in use. + * Multithreaded algorithms must check this value and adapt their thread + * handling accordingly -- the setting will not be automatically applied. + * + * \see maximumThreads() * \since QGIS 3.32 */ - void setNumberOfThreads( int threads ); + void setMaximumThreads( int threads ); /** * Exports the context's settings to a variant map. @@ -762,7 +768,7 @@ class CORE_EXPORT QgsProcessingContext LogLevel mLogLevel = DefaultLevel; QString mTemporaryFolderOverride; - int mThreadsToUse = QThread::idealThreadCount(); + int mMaximumThreads = QThread::idealThreadCount(); #ifdef SIP_RUN QgsProcessingContext( const QgsProcessingContext &other ); diff --git a/src/gui/processing/qgsprocessingalgorithmdialogbase.cpp b/src/gui/processing/qgsprocessingalgorithmdialogbase.cpp index 5aa870ca821..c84db47808e 100644 --- a/src/gui/processing/qgsprocessingalgorithmdialogbase.cpp +++ b/src/gui/processing/qgsprocessingalgorithmdialogbase.cpp @@ -160,7 +160,7 @@ QgsProcessingAlgorithmDialogBase::QgsProcessingAlgorithmDialogBase( QWidget *par mDistanceUnits = mContextOptionsWidget->distanceUnit(); mAreaUnits = mContextOptionsWidget->areaUnit(); mTemporaryFolderOverride = mContextOptionsWidget->temporaryFolder(); - mNumberOfThreads = mContextOptionsWidget->numberOfThreads(); + mMaximumThreads = mContextOptionsWidget->maximumThreads(); } ); } } @@ -870,7 +870,7 @@ void QgsProcessingAlgorithmDialogBase::applyContextOverrides( QgsProcessingConte context->setDistanceUnit( mDistanceUnits ); context->setAreaUnit( mAreaUnits ); context->setTemporaryFolder( mTemporaryFolderOverride ); - context->setNumberOfThreads( mNumberOfThreads ); + context->setMaximumThreads( mMaximumThreads ); } } @@ -1027,7 +1027,7 @@ void QgsProcessingContextOptionsWidget::setFromContext( const QgsProcessingConte whileBlocking( mDistanceUnitsCombo )->setCurrentIndex( mDistanceUnitsCombo->findData( QVariant::fromValue( context->distanceUnit() ) ) ); whileBlocking( mAreaUnitsCombo )->setCurrentIndex( mAreaUnitsCombo->findData( QVariant::fromValue( context->areaUnit() ) ) ); whileBlocking( mTemporaryFolderWidget )->setFilePath( context->temporaryFolder() ); - whileBlocking( mThreadsSpinBox )->setValue( context->numberOfThreads() ); + whileBlocking( mThreadsSpinBox )->setValue( context->maximumThreads() ); } QgsFeatureRequest::InvalidGeometryCheck QgsProcessingContextOptionsWidget::invalidGeometryCheck() const @@ -1050,7 +1050,7 @@ QString QgsProcessingContextOptionsWidget::temporaryFolder() return mTemporaryFolderWidget->filePath(); } -int QgsProcessingContextOptionsWidget::numberOfThreads() const +int QgsProcessingContextOptionsWidget::maximumThreads() const { return mThreadsSpinBox->value(); } diff --git a/src/gui/processing/qgsprocessingalgorithmdialogbase.h b/src/gui/processing/qgsprocessingalgorithmdialogbase.h index 7a6f4c8795a..ce2534d2c01 100644 --- a/src/gui/processing/qgsprocessingalgorithmdialogbase.h +++ b/src/gui/processing/qgsprocessingalgorithmdialogbase.h @@ -477,7 +477,7 @@ class GUI_EXPORT QgsProcessingAlgorithmDialogBase : public QDialog, public QgsPr Qgis::DistanceUnit mDistanceUnits = Qgis::DistanceUnit::Unknown; Qgis::AreaUnit mAreaUnits = Qgis::AreaUnit::Unknown; QString mTemporaryFolderOverride; - int mNumberOfThreads = QThread::idealThreadCount(); + int mMaximumThreads = QThread::idealThreadCount(); QString formatHelp( QgsProcessingAlgorithm *algorithm ); void scrollToBottomOfLog(); @@ -570,7 +570,7 @@ class GUI_EXPORT QgsProcessingContextOptionsWidget : public QgsPanelWidget, priv /** * Returns the number of threads to use selected in the widget. */ - int numberOfThreads() const; + int maximumThreads() const; }; #endif diff --git a/tests/src/analysis/testqgsprocessingpdalalgs.cpp b/tests/src/analysis/testqgsprocessingpdalalgs.cpp index a98dc2e8db6..00182aae09a 100644 --- a/tests/src/analysis/testqgsprocessingpdalalgs.cpp +++ b/tests/src/analysis/testqgsprocessingpdalalgs.cpp @@ -107,7 +107,7 @@ void TestQgsProcessingPdalAlgs::convertFormat() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -124,7 +124,7 @@ void TestQgsProcessingPdalAlgs::convertFormat() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "translate" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -139,7 +139,7 @@ void TestQgsProcessingPdalAlgs::reproject() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -158,7 +158,7 @@ void TestQgsProcessingPdalAlgs::reproject() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "translate" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -174,7 +174,7 @@ void TestQgsProcessingPdalAlgs::fixProjection() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -193,7 +193,7 @@ void TestQgsProcessingPdalAlgs::fixProjection() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "translate" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -209,7 +209,7 @@ void TestQgsProcessingPdalAlgs::thin() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -272,7 +272,7 @@ void TestQgsProcessingPdalAlgs::thin() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "thin" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -291,7 +291,7 @@ void TestQgsProcessingPdalAlgs::boundary() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -345,7 +345,7 @@ void TestQgsProcessingPdalAlgs::boundary() // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "boundary" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -364,7 +364,7 @@ void TestQgsProcessingPdalAlgs::density() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -453,7 +453,7 @@ void TestQgsProcessingPdalAlgs::density() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "density" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -474,7 +474,7 @@ void TestQgsProcessingPdalAlgs::exportRasterTin() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -563,7 +563,7 @@ void TestQgsProcessingPdalAlgs::exportRasterTin() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "to_raster_tin" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -584,7 +584,7 @@ void TestQgsProcessingPdalAlgs::tile() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -637,7 +637,7 @@ void TestQgsProcessingPdalAlgs::tile() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "tile" ) << QStringLiteral( "--length=150" ) @@ -655,7 +655,7 @@ void TestQgsProcessingPdalAlgs::exportRaster() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -761,7 +761,7 @@ void TestQgsProcessingPdalAlgs::exportRaster() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "to_raster" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -783,7 +783,7 @@ void TestQgsProcessingPdalAlgs::exportVector() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -831,7 +831,7 @@ void TestQgsProcessingPdalAlgs::exportVector() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "to_vector" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -849,7 +849,7 @@ void TestQgsProcessingPdalAlgs::merge() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -899,7 +899,7 @@ void TestQgsProcessingPdalAlgs::merge() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "merge" ) << QStringLiteral( "--output=%1" ).arg( outputFile ) @@ -917,7 +917,7 @@ void TestQgsProcessingPdalAlgs::buildVpc() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -979,7 +979,7 @@ void TestQgsProcessingPdalAlgs::buildVpc() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "build_vpc" ) << QStringLiteral( "--output=%1" ).arg( outputFile ) @@ -998,7 +998,7 @@ void TestQgsProcessingPdalAlgs::clip() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -1039,7 +1039,7 @@ void TestQgsProcessingPdalAlgs::clip() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "clip" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath ) @@ -1057,7 +1057,7 @@ void TestQgsProcessingPdalAlgs::filter() std::unique_ptr< QgsProcessingContext > context = std::make_unique< QgsProcessingContext >(); context->setProject( QgsProject::instance() ); - context->setNumberOfThreads( 0 ); + context->setMaximumThreads( 0 ); QgsProcessingFeedback feedback; @@ -1091,7 +1091,7 @@ void TestQgsProcessingPdalAlgs::filter() ); // set max threads to 2, a --threads argument should be added - context->setNumberOfThreads( 2 ); + context->setMaximumThreads( 2 ); args = alg->createArgumentLists( parameters, *context, &feedback ); QCOMPARE( args, QStringList() << QStringLiteral( "translate" ) << QStringLiteral( "--input=%1" ).arg( mPointCloudLayerPath )