Skip to content

Commit 170d4a4

Browse files
florianlinkflorianlink
committed
reimplemented argument frame caching (previous approach was not thread-safe when GIL is used)
1 parent 28c1c4b commit 170d4a4

6 files changed

+175
-197
lines changed

‎src/PythonQt.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -391,11 +391,9 @@ PythonQtPrivate::~PythonQtPrivate() {
391391
delete i.next().value();
392392
}
393393
}
394-
PythonQtConv::global_valueStorage.clear();
395-
PythonQtConv::global_ptrStorage.clear();
396-
PythonQtConv::global_variantStorage.clear();
397394

398395
PythonQtMethodInfo::cleanupCachedMethodInfos();
396+
PythonQtArgumentFrame::cleanupFreeList();
399397
}
400398

401399
void PythonQt::setRedirectStdInCallback(PythonQtInputChangedCB* callback, void * callbackData)

‎src/PythonQtConversion.cpp

+50-54
Large diffs are not rendered by default.

‎src/PythonQtConversion.h

+3-9
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,10 @@ class PYTHONQT_EXPORT PythonQtConv {
109109
static PyObject* ConvertQtValueToPython(const PythonQtMethodInfo::ParameterInfo& info, const void* data);
110110

111111
//! convert python object to Qt (according to the given parameter) and if the conversion should be strict (classInfo is currently not used anymore)
112-
static void* ConvertPythonToQt(const PythonQtMethodInfo::ParameterInfo& info, PyObject* obj, bool strict, PythonQtClassInfo* classInfo, void* alreadyAllocatedCPPObject = NULL);
112+
static void* ConvertPythonToQt(const PythonQtMethodInfo::ParameterInfo& info, PyObject* obj, bool strict, PythonQtClassInfo* classInfo, void* alreadyAllocatedCPPObject, PythonQtArgumentFrame* frame = NULL);
113113

114114
//! creates a data storage for the passed parameter type and returns a void pointer to be set as arg[0] of qt_metacall
115-
static void* CreateQtReturnValue(const PythonQtMethodInfo::ParameterInfo& info);
115+
static void* CreateQtReturnValue(const PythonQtMethodInfo::ParameterInfo& info, PythonQtArgumentFrame* frame);
116116

117117
//! converts QString to Python string (unicode!)
118118
static PyObject* QStringToPyObject(const QString& str);
@@ -193,19 +193,13 @@ class PYTHONQT_EXPORT PythonQtConv {
193193
//! Returns if the given object is a string (or unicode string)
194194
static bool isStringType(PyTypeObject* type);
195195

196-
public:
197-
198-
static PythonQtValueStorage<qint64, 128> global_valueStorage;
199-
static PythonQtValueStorage<void*, 128> global_ptrStorage;
200-
static PythonQtValueStorageWithCleanup<QVariant, 128> global_variantStorage;
201-
202196
protected:
203197
static QHash<int, PythonQtConvertMetaTypeToPythonCB*> _metaTypeToPythonConverters;
204198
static QHash<int, PythonQtConvertPythonToMetaTypeCB*> _pythonToMetaTypeConverters;
205199
static PythonQtConvertPythonSequenceToQVariantListCB* _pythonSequenceToQVariantListCB;
206200

207201
//! handle automatic conversion of some special types (QColor, QBrush, ...)
208-
static void* handlePythonToQtAutoConversion(int typeId, PyObject* obj, void* alreadyAllocatedCPPObject);
202+
static void* handlePythonToQtAutoConversion(int typeId, PyObject* obj, void* alreadyAllocatedCPPObject, PythonQtArgumentFrame* frame);
209203

210204
//! converts the list of pointers of given type to Python
211205
static PyObject* ConvertQListOfPointerTypeToPythonList(QList<void*>* list, const PythonQtMethodInfo::ParameterInfo& info);

‎src/PythonQtMisc.cpp

+74
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,78 @@
4040
//----------------------------------------------------------------------------------
4141

4242
#include "PythonQtMisc.h"
43+
#include <iostream>
4344

45+
#define PYTHONQT_MAX_ARGUMENT_FRAME_SIZE (PYTHONQT_MAX_ARGS * 2)
46+
47+
PythonQtArgumentFrame* PythonQtArgumentFrame::_freeListHead = NULL;
48+
49+
PythonQtArgumentFrame::PythonQtArgumentFrame()
50+
{
51+
_freeListNext = NULL;
52+
53+
// it is important to reserve the memory immediately,
54+
// otherwise pointers would change while pushing back new arguments.
55+
_variantArgs.reserve(PYTHONQT_MAX_ARGUMENT_FRAME_SIZE);
56+
_podArgs.reserve(PYTHONQT_MAX_ARGUMENT_FRAME_SIZE);
57+
}
58+
59+
PythonQtArgumentFrame::~PythonQtArgumentFrame()
60+
{
61+
}
62+
63+
PythonQtArgumentFrame* PythonQtArgumentFrame::newFrame()
64+
{
65+
PythonQtArgumentFrame* frame = NULL;
66+
if (_freeListHead) {
67+
frame = _freeListHead;
68+
_freeListHead = _freeListHead->_freeListNext;
69+
frame->_freeListNext = NULL;
70+
} else {
71+
frame = new PythonQtArgumentFrame();
72+
}
73+
return frame;
74+
}
75+
76+
void PythonQtArgumentFrame::deleteFrame(PythonQtArgumentFrame* frame)
77+
{
78+
frame->reset();
79+
frame->_freeListNext = _freeListHead;
80+
_freeListHead = frame;
81+
}
82+
83+
void PythonQtArgumentFrame::cleanupFreeList()
84+
{
85+
PythonQtArgumentFrame* head = _freeListHead;
86+
while (head) {
87+
PythonQtArgumentFrame* tmp = head;
88+
head = head->_freeListNext;
89+
delete tmp;
90+
}
91+
_freeListHead = NULL;
92+
}
93+
94+
void PythonQtArgumentFrame::reset()
95+
{
96+
// Note: clear still keeps the capacity of the vectors, which is what we want!
97+
_variantArgs.clear();
98+
_podArgs.clear();
99+
}
100+
101+
QVariant* PythonQtArgumentFrame::nextVariantPtr()
102+
{
103+
if (_variantArgs.size() >= PYTHONQT_MAX_ARGUMENT_FRAME_SIZE) {
104+
std::cerr << "PYTHONQT_MAX_ARGUMENT_FRAME_SIZE QVariants exceeded, use less complex slots or increase size!" << std::endl;
105+
}
106+
_variantArgs.push_back(QVariant());
107+
return &_variantArgs[_variantArgs.size() - 1];
108+
}
109+
110+
quint64* PythonQtArgumentFrame::nextPODPtr()
111+
{
112+
if (_podArgs.size() >= PYTHONQT_MAX_ARGUMENT_FRAME_SIZE) {
113+
std::cerr << "PYTHONQT_MAX_ARGUMENT_FRAME_SIZE PODs exceeded, use less complex slots or increase size!" << std::endl;
114+
}
115+
_podArgs.push_back(0);
116+
return &_podArgs[_podArgs.size() - 1];
117+
}

‎src/PythonQtMisc.h

+43-109
Original file line numberDiff line numberDiff line change
@@ -43,133 +43,67 @@
4343
//----------------------------------------------------------------------------------
4444

4545
#include "PythonQtPythonInclude.h"
46-
#include <QList>
46+
#include <vector>
47+
#include <QVariant>
4748

48-
#define PythonQtValueStorage_ADD_VALUE(store, type, value, ptr) \
49-
{ type* item = (type*)store.nextValuePtr(); \
49+
#define PYTHONQT_MAX_ARGS 32
50+
51+
#define PythonQtArgumentFrame_ADD_VALUE(store, type, value, ptr) \
52+
{ type* item = (type*)store->nextPODPtr(); \
5053
*item = value; \
5154
ptr = (void*)item; \
5255
}
5356

54-
#define PythonQtValueStorage_ADD_VALUE_IF_NEEDED(alreadyAllocatedPtr,store, type, value, ptr) \
57+
#define PythonQtArgumentFrame_ADD_VALUE_IF_NEEDED(alreadyAllocatedPtr,store, type, value, ptr) \
5558
{ \
56-
type* item = (type*)(alreadyAllocatedPtr?alreadyAllocatedPtr:store.nextValuePtr()); \
59+
type* item = (type*)(alreadyAllocatedPtr?alreadyAllocatedPtr:store->nextPODPtr()); \
5760
*item = value; \
5861
ptr = (void*)item; \
5962
}
6063

61-
//! stores a position in the PythonQtValueStorage
62-
class PythonQtValueStoragePosition {
63-
64-
public:
65-
PythonQtValueStoragePosition() { chunkIdx = 0; chunkOffset = 0; }
64+
#define PythonQtArgumentFrame_ADD_VARIANT_VALUE(store, value, ptr) \
65+
{ QVariant* item = store->nextVariantPtr(); \
66+
*item = value; \
67+
ptr = (void*)item; \
68+
}
6669

67-
int chunkIdx;
68-
int chunkOffset;
70+
#define PythonQtArgumentFrame_ADD_VARIANT_VALUE_IF_NEEDED(alreadyAllocatedPtr,store, value, ptr) \
71+
{ \
72+
QVariant* item = (QVariant*)(alreadyAllocatedPtr?alreadyAllocatedPtr:store->nextVariantPtr()); \
73+
*item = value; \
74+
ptr = (void*)item; \
75+
}
6976

70-
};
77+
//! Stores C++ arguments for a qt_metacall (which are created when converting data from Python to C++)
78+
class PythonQtArgumentFrame {
7179

72-
//! a helper class that stores basic C++ value types in chunks
73-
template <typename T, int chunkEntries> class PythonQtValueStorage
74-
{
7580
public:
76-
PythonQtValueStorage() {
77-
_chunkIdx = 0;
78-
_chunkOffset = 0;
79-
_currentChunk = new T[chunkEntries];
80-
_chunks.append(_currentChunk);
81-
};
82-
83-
//! clear all memory
84-
void clear() {
85-
T* chunk;
86-
Q_FOREACH(chunk, _chunks) {
87-
delete[]chunk;
88-
}
89-
_chunks.clear();
90-
}
91-
92-
//! get the current position to be restored with setPos
93-
void getPos(PythonQtValueStoragePosition & pos) {
94-
pos.chunkIdx = _chunkIdx;
95-
pos.chunkOffset = _chunkOffset;
96-
}
97-
98-
//! set the current position (without freeing memory, thus caching old entries for reuse)
99-
void setPos(const PythonQtValueStoragePosition& pos) {
100-
_chunkOffset = pos.chunkOffset;
101-
if (_chunkIdx != pos.chunkIdx) {
102-
_chunkIdx = pos.chunkIdx;
103-
_currentChunk = _chunks.at(_chunkIdx);
104-
}
105-
}
106-
107-
//! add one default constructed value and return the pointer to it
108-
T* nextValuePtr() {
109-
if (_chunkOffset>=chunkEntries) {
110-
_chunkIdx++;
111-
if (_chunkIdx >= _chunks.size()) {
112-
T* newChunk = new T[chunkEntries];
113-
_chunks.append(newChunk);
114-
_currentChunk = newChunk;
115-
} else {
116-
_currentChunk = _chunks.at(_chunkIdx);
117-
}
118-
_chunkOffset = 0;
119-
}
120-
T* newEntry = _currentChunk + _chunkOffset;
121-
_chunkOffset++;
122-
return newEntry;
123-
};
124-
125-
protected:
126-
QList<T*> _chunks;
127-
128-
int _chunkIdx;
129-
int _chunkOffset;
130-
T* _currentChunk;
81+
//! Create a new (empty) frame (which is typically reused from a freelist)
82+
static PythonQtArgumentFrame* newFrame();
83+
//! Frees the frame (resetting it and putting it back to the freelist)
84+
static void deleteFrame(PythonQtArgumentFrame* frame);
13185

132-
};
86+
//! Frees all PythonQtArgumentFrame frames that are stored.
87+
static void cleanupFreeList();
13388

134-
//! a helper class that stores basic C++ value types in chunks and clears the unused values on setPos() usage.
135-
template <typename T, int chunkEntries> class PythonQtValueStorageWithCleanup : public PythonQtValueStorage<T, chunkEntries>
136-
{
137-
public:
138-
void setPos(const PythonQtValueStoragePosition& pos) {
139-
if (_chunkIdx > pos.chunkIdx) {
140-
T* firstChunk = _chunks.at(pos.chunkIdx);
141-
// clear region in first chunk
142-
for (int i = pos.chunkOffset; i < chunkEntries; i++) {
143-
firstChunk[i] = T();
144-
}
145-
for (int chunk = pos.chunkIdx + 1; chunk < _chunkIdx; chunk++) {
146-
// clear the full chunks between the first and last chunk
147-
T* fullChunk = _chunks.at(chunk);
148-
for (int i = 0; i < chunkEntries; i++) {
149-
fullChunk[i] = T();
150-
}
151-
}
152-
// clear region in last chunk
153-
T* lastChunk = _chunks.at(_chunkIdx);
154-
for (int i = 0; i < _chunkOffset; i++) {
155-
lastChunk[i] = T();
156-
}
157-
} else if (_chunkIdx == pos.chunkIdx) {
158-
// clear the region in the last chunk only
159-
T* lastChunk = _chunks.at(_chunkIdx);
160-
for (int i = pos.chunkOffset; i<_chunkOffset; i++) {
161-
lastChunk[i] = T();
162-
}
163-
}
164-
165-
PythonQtValueStorage<T, chunkEntries>::setPos(pos);
166-
}
89+
//! Resets the pod and variant argument lists to empty lists.
90+
void reset();
91+
92+
//! Get next pointer to a variant
93+
QVariant* nextVariantPtr();
94+
//! Get next pointer to a POD.
95+
quint64* nextPODPtr();
16796

16897
private:
169-
using PythonQtValueStorage<T, chunkEntries>::_chunks;
170-
using PythonQtValueStorage<T, chunkEntries>::_chunkIdx;
171-
using PythonQtValueStorage<T, chunkEntries>::_chunkOffset;
172-
using PythonQtValueStorage<T, chunkEntries>::_currentChunk;
98+
PythonQtArgumentFrame();
99+
~PythonQtArgumentFrame();
100+
101+
std::vector<quint64> _podArgs;
102+
std::vector<QVariant> _variantArgs;
103+
104+
PythonQtArgumentFrame* _freeListNext;
105+
106+
static PythonQtArgumentFrame* _freeListHead;
173107
};
174108

175109
#endif

‎src/PythonQtSlot.cpp

+4-22
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,12 @@
5656
#include <cxxabi.h>
5757
#endif
5858

59-
#define PYTHONQT_MAX_ARGS 32
60-
61-
6259
bool PythonQtCallSlot(PythonQtClassInfo* classInfo, QObject* objectToCall, PyObject* args, bool strict, PythonQtSlotInfo* info, void* firstArgument, PyObject** pythonReturnValue, void** directReturnValuePointer, PythonQtPassThisOwnershipType* passThisOwnershipToCPP)
6360
{
64-
static unsigned int recursiveEntry = 0;
65-
6661
if (directReturnValuePointer) {
6762
*directReturnValuePointer = NULL;
6863
}
69-
// store the current storage position, so that we can get back to this state after a slot is called
70-
// (do this locally, so that we have all positions on the stack
71-
PythonQtValueStoragePosition globalValueStoragePos;
72-
PythonQtValueStoragePosition globalPtrStoragePos;
73-
PythonQtValueStoragePosition globalVariantStoragePos;
74-
PythonQtConv::global_valueStorage.getPos(globalValueStoragePos);
75-
PythonQtConv::global_ptrStorage.getPos(globalPtrStoragePos);
76-
PythonQtConv::global_variantStorage.getPos(globalVariantStoragePos);
77-
78-
recursiveEntry++;
64+
PythonQtArgumentFrame* frame = PythonQtArgumentFrame::newFrame();
7965

8066
// the arguments that are passed to qt_metacall
8167
void* argList[PYTHONQT_MAX_ARGS];
@@ -115,7 +101,7 @@ bool PythonQtCallSlot(PythonQtClassInfo* classInfo, QObject* objectToCall, PyObj
115101
}
116102
for (int i = 1 + instanceDecoOffset; i<argc && ok; i++) {
117103
const PythonQtSlotInfo::ParameterInfo& param = params.at(i);
118-
argList[i] = PythonQtConv::ConvertPythonToQt(param, PyTuple_GET_ITEM(args, i - 1 - instanceDecoOffset), strict, classInfo);
104+
argList[i] = PythonQtConv::ConvertPythonToQt(param, PyTuple_GET_ITEM(args, i - 1 - instanceDecoOffset), strict, classInfo, NULL, frame);
119105
if (argList[i]==NULL) {
120106
ok = false;
121107
break;
@@ -143,7 +129,7 @@ bool PythonQtCallSlot(PythonQtClassInfo* classInfo, QObject* objectToCall, PyObj
143129
// create empty default value for the return value
144130
if (!directReturnValuePointer) {
145131
// create empty default value for the return value
146-
argList[0] = PythonQtConv::CreateQtReturnValue(returnValueParam);
132+
argList[0] = PythonQtConv::CreateQtReturnValue(returnValueParam, frame);
147133
if (argList[0]==NULL) {
148134
// return value could not be created, maybe we have a registered class with a default constructor, so that we can construct the pythonqt wrapper object and
149135
// pass its internal pointer
@@ -249,12 +235,8 @@ bool PythonQtCallSlot(PythonQtClassInfo* classInfo, QObject* objectToCall, PyObj
249235
ok = false;
250236
}
251237
}
252-
recursiveEntry--;
253238

254-
// reset the parameter storage position to the stored pos to "pop" the parameter stack
255-
PythonQtConv::global_valueStorage.setPos(globalValueStoragePos);
256-
PythonQtConv::global_ptrStorage.setPos(globalPtrStoragePos);
257-
PythonQtConv::global_variantStorage.setPos(globalVariantStoragePos);
239+
PythonQtArgumentFrame::deleteFrame(frame);
258240

259241
*pythonReturnValue = result;
260242

0 commit comments

Comments
 (0)