Reputation: 332
I'm trying to speed up my (qt c++ opencv) program which should count number of colors in my photos for future filtering. There is no leak in single-threaded approach, but it is very slow.
With adding 8 threads I've already speed up this process up to 5x times.
The problem starts when I switch my program to multithreading.
There is a HUGE memory leak! http://snag.gy/cHRrS.jpg
Following this advises (https://stackoverflow.com/a/12859444) I prevented from subclassing QThread and implementing run().
Here is a for loop for counting each pixel in every new image shifted by 1 pixel:
ColorCounterController *cntrl[arrSize];
for (int i = 0; i < box; i++)//x
{
for (int j = 0; j < box; ++j)//y
{
cv::Mat res=process(image,i,j);
//Using 1 core
//colors=ColorDetectController::getInstance()->colorsCount(res);
//Using all 8 cores
cntrl[cnt2%arrSize]= new ColorCounterController(res,this);
++cnt2;
}
++cnt;
emit setStatusProgressSignal((int)(cnt/amnt*100));
}
delete[] *cntrl;
Comments:
When using 1 core (above code) I have singleton to run colorsCount(res) function. In case of 8 cores I use almost the same function but called from ColorCounterController.
class ColorCounterController : public QObject{
Q_OBJECT
private:
QThread thread;
ColorCounter *colorCntr;
Pixalate *pixelate;
private slots:
void freecolorCntr(){
delete colorCntr;
}
public:
ColorCounterController(const cv::Mat &image,Pixalate *pxobj) {
colorCntr= new ColorCounter();
colorCntr->setimageThread(image);
colorCntr->moveToThread(&thread);
connect(&thread, SIGNAL(started()), colorCntr, SLOT(colorsCountThread()));
connect(colorCntr, SIGNAL(finished()), &thread, SLOT(quit()));
connect(colorCntr, SIGNAL(finished()), colorCntr, SLOT(deleteLater()));
connect(colorCntr, SIGNAL(results(int)), pxobj, SLOT(results(int)));
thread.start();
}
~ColorCounterController() {
thread.quit();
thread.wait();
qDebug() << QString("Controller quit wait");
//delete colorCntr; //err
}
I suppose that leak is in ColorCounterController constructor:
colorCntr= new ColorCounter();
But how to avoid it? This code causing an error. in a destructor:
//delete colorCntr; //err
and in a constructor:
//connect(&thread, SIGNAL(finished()), &thread, SLOT(deleteLater()));
Please help!
P.S.
I changed this
delete[] *cntrl;
to this
for (int i = 0; i < arrSize; i++){
if (cntrl[i])
delete cntrl[i];
}
and NULL for all pointers at the beginning before cntrl[cnt2%arrSize]
Nothing changed
P.P.S. In case you want to contribute to this question: https://github.com/ivanesses/curiosity
Upvotes: 0
Views: 439
Reputation: 1401
2 problems cause leakage:
pointers to new ColorCounterController objects will be lost forever (and their memory leaked) as the body of the "for" loop will run N times (N=box*box) creating N ColorCounterController objects but only pointers to 8 of them will fit into the array which you later use to delete objects.
cntrl is an array of pointers. You need to iterate through it and call delete (plain delete, not delete[]) on each of it's elements.
as found by the OP: must use imageThread.release(); instead of imageThread.deallocate()
Upvotes: 1