Skip to content

Commit

Permalink
[python] Always release the GIL before calling PyQGIS c++ methods
Browse files Browse the repository at this point in the history
Switches on the sip "-g" switch, which forces sip to release the
Python Global Interpreter Lock before calling a qgis c++ method,
and reacquire it after.

While this flag is not a default sip flag, it is used when building
the PyQt API, so can't forsee any issues from enabling it.

The benefit however is extreme for PyQGIS based scripts which
rely on threads, potentially resulting in massive performance
boosts.

Without this switch, calling an expensive c++ method (say,
building a QgsSpatialIndex using a QgsFeatureIterator) would lock
the Python GIL for the duration of the c++ call... which could
potentially take minutes or more. With the switch, the lock
is released before all calls, so other Python threads are free
to merrily grab the lock and do other processing while the
original thread chugs away in c++ land.

Benchtests of worst-case scenarios (single thread calling
thousands of very inexpensive PyQGIS methods (simple getters))
regressed from mean of 154 seconds to 158 with this flag. But
that's worst case (and as Intel have recently demonstrated...
we can't take yesterday's computing speed as the benchmark
for todays ;). Given that best case scenarious (multi-threaded
operations calling slow c++ methods) will benefit so greatly
from this change, I think it's an acceptable trade off.

*This is a step toward potentially re-enabling background
execution of python based Processing algorithms, and also
should greatly improve QGIS responsiveness when using
python based renderers/symbols.
  • Loading branch information
nyalldawson committed Jan 26, 2018
1 parent f4f89bb commit b3256ad
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions python/CMakeLists.txt
Expand Up @@ -182,7 +182,7 @@ ENDIF(${SIP_VERSION_STR} VERSION_GREATER 4.19.6)
# core module
FILE(GLOB_RECURSE sip_files_core core/*.sip core/*.sip.in)
SET(SIP_EXTRA_FILES_DEPEND ${sip_files_core})
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -o -a ${CMAKE_BINARY_DIR}/python/qgis.core.api)
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -g -o -a ${CMAKE_BINARY_DIR}/python/qgis.core.api)
GENERATE_SIP_PYTHON_MODULE_CODE(qgis._core core/core.sip "${sip_files_core}" cpp_files)
BUILD_SIP_PYTHON_MODULE(qgis._core core/core.sip ${cpp_files} "" qgis_core)
SET(SIP_CORE_CPP_FILES ${cpp_files})
Expand All @@ -200,7 +200,7 @@ IF (WITH_GUI)

FILE(GLOB_RECURSE sip_files_gui gui/*.sip gui/*.sip.in)
SET(SIP_EXTRA_FILES_DEPEND ${sip_files_core} ${sip_files_gui})
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -o -a ${CMAKE_BINARY_DIR}/python/qgis.gui.api)
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -g -o -a ${CMAKE_BINARY_DIR}/python/qgis.gui.api)

IF(QSCI_SIP_DIR)
SET(SIP_EXTRA_OPTIONS ${SIP_EXTRA_OPTIONS} -I ${QSCI_SIP_DIR})
Expand All @@ -224,7 +224,7 @@ IF (WITH_SERVER AND WITH_SERVER_PLUGINS)

FILE(GLOB_RECURSE sip_files_server server/*.sip server/*.sip.in)
SET(SIP_EXTRA_FILES_DEPEND ${sip_files_core} ${sip_files_server})
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -o -a ${CMAKE_BINARY_DIR}/python/qgis.server.api)
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -g -o -a ${CMAKE_BINARY_DIR}/python/qgis.server.api)
GENERATE_SIP_PYTHON_MODULE_CODE(qgis._server server/server.sip "${sip_files_server}" cpp_files)
BUILD_SIP_PYTHON_MODULE(qgis._server server/server.sip ${cpp_files} "" qgis_core qgis_server)
ENDIF (WITH_SERVER AND WITH_SERVER_PLUGINS)
Expand All @@ -248,7 +248,7 @@ INCLUDE_DIRECTORIES(BEFORE
# analysis module
FILE(GLOB_RECURSE sip_files_analysis analysis/*.sip analysis/*.sip.in)
SET(SIP_EXTRA_FILES_DEPEND ${sip_files_core} ${sip_files_analysis})
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -o -a ${CMAKE_BINARY_DIR}/python/qgis.analysis.api)
SET(SIP_EXTRA_OPTIONS ${PYQT_SIP_FLAGS} -g -o -a ${CMAKE_BINARY_DIR}/python/qgis.analysis.api)
GENERATE_SIP_PYTHON_MODULE_CODE(qgis._analysis analysis/analysis.sip "${sip_files_analysis}" cpp_files)
BUILD_SIP_PYTHON_MODULE(qgis._analysis analysis/analysis.sip ${cpp_files} "" qgis_core qgis_analysis)

Expand Down

0 comments on commit b3256ad

Please sign in to comment.