[PATCH RFC] Run create_plot_info in a separate thread

[Dirk Hohndel]

On Sun, 2013-11-24 at 11:58 +0100, Anton Lundin wrote:
> This is a crude attempt to reduce the gui lag while we are calculating
> things for the plot, as tts/deco.
> 
> This will delay the whole plot a while, but the gui won't freeze
> meanwhile.
> 
> This is more a rough cut of what could be done than whats a real
> solution.

I like the idea per se. But the implementation isn't quite robust
enough. It appears to expose a dependency issue in our build system. If
I don't do a make clean I had Subsurface crash in the
ProfileGraphicsView constructor.

But even solving that, if you scroll fast enough, you can outrun the
thread and it will access stale data and crash...

create_plot_info calls down into it's slower helpers (in this case
analyze_plot_info_minmax_minute()) and the entry-> pointer ends up being
invalid. I haven't spent much time trying to figure out why it ends up
pointing to an invalid address... but having the thread rely on the fact
that the main process isn't messing with the data structures it relies
on seems a bit optimistic

/D

> ---
>  qt-ui/profilegraphics.cpp | 65 ++++++++++++++++++++++++++++++++++++-----------
>  qt-ui/profilegraphics.h   |  8 ++++++
>  2 files changed, 58 insertions(+), 15 deletions(-)
> 
> diff --git a/qt-ui/profilegraphics.cpp b/qt-ui/profilegraphics.cpp
> index f2d8e74..8912624 100644
> --- a/qt-ui/profilegraphics.cpp
> +++ b/qt-ui/profilegraphics.cpp
> @@ -19,6 +19,7 @@
>  #include <qtextdocument.h>
>  #include <QMessageBox>
>  #include <limits>
> +#include <QtGui>
>  
>  #include "../color.h"
>  #include "../display.h"
> @@ -365,9 +366,54 @@ void ProfileGraphicsView::plot(struct dive *d, bool forceRedraw)
>  		dc = fake_dc(dc);
>  	}
>  
> -	QString nick = get_dc_nickname(dc->model, dc->deviceid);
> +	/*
> +	 * Set up limits that are independent of
> +	 * the dive computer
> +	 */
> +	calculate_max_limits(dive, diveDC, &gc);
> +
> +	QRectF profile_grid_area = scene()->sceneRect();
> +	gc.maxx = (profile_grid_area.width() - 2 * profile_grid_area.x());
> +	gc.maxy = (profile_grid_area.height() - 2 * profile_grid_area.y());
> +
> +
> +	/* Hack tu run create_plot_info async */
> +	if (future.isRunning()) {
> +		printf("future is running\n");
> +		future.cancel();
> +		future.waitForFinished();
> +	}
> +	if (watcher != NULL) {
> +		printf("watcher not NULL!\n");
> +		delete watcher;
> +		watcher = NULL;
> +	}
> +	printf("Scheduling\n");
> +	watcher = new QFutureWatcher<struct plot_info *>();
> +	future = QtConcurrent::run(create_plot_info, dive, dc, &gc);
> +	watcher->setFuture(future);
> +	QObject::connect(watcher, SIGNAL(finished()), this, SLOT(plot2()));
> +}
> +
> +void ProfileGraphicsView::plot2()
> +{
> +	delete watcher;
> +	watcher = NULL;
> +
> +	/* This is per-dive-computer */
> +	gc.pi = *future.result();
> +
> +	/* Did we cancel? */
> +	if (future.isCanceled())
> +		return;
> +
> +	gc.pi = *create_plot_info(dive, diveDC, &gc);
> +
> +	printf("future returned a pi\n");
> +
> +	QString nick = get_dc_nickname(diveDC->model, diveDC->deviceid);
>  	if (nick.isEmpty())
> -		nick = QString(dc->model);
> +		nick = QString(diveDC->model);
>  
>  	if (nick.isEmpty())
>  		nick = tr("unknown divecomputer");
> @@ -378,18 +424,7 @@ void ProfileGraphicsView::plot(struct dive *d, bool forceRedraw)
>  		mode = DIVE;
>  	}
>  
> -	/*
> -	 * Set up limits that are independent of
> -	 * the dive computer
> -	 */
> -	calculate_max_limits(dive, dc, &gc);
> -
>  	QRectF profile_grid_area = scene()->sceneRect();
> -	gc.maxx = (profile_grid_area.width() - 2 * profile_grid_area.x());
> -	gc.maxy = (profile_grid_area.height() - 2 * profile_grid_area.y());
> -
> -	/* This is per-dive-computer */
> -	gc.pi = *create_plot_info(dive, dc, &gc);
>  
>  	/* Bounding box */
>  	QPen pen = defaultPen;
> @@ -400,7 +435,7 @@ void ProfileGraphicsView::plot(struct dive *d, bool forceRedraw)
>  
>  	/* Depth profile */
>  	plot_depth_profile();
> -	plot_events(dc);
> +	plot_events(diveDC);
>  
>  	if (rulerEnabled && !printMode)
>  		create_ruler();
> @@ -458,7 +493,7 @@ void ProfileGraphicsView::plot(struct dive *d, bool forceRedraw)
>  	}
>  
>  	if (!printMode)
> -		addControlItems(d);
> +		addControlItems(dive);
>  
>  	if (rulerEnabled && !printMode)
>  		add_ruler();
> diff --git a/qt-ui/profilegraphics.h b/qt-ui/profilegraphics.h
> index 059b667..e4d8140 100644
> --- a/qt-ui/profilegraphics.h
> +++ b/qt-ui/profilegraphics.h
> @@ -9,6 +9,8 @@
>  #include <QDoubleSpinBox>
>  #include <QPushButton>
>  #include <QGraphicsProxyWidget>
> +#include <QFuture>
> +#include <QFutureWatcher>
>  
>  struct text_render_options;
>  struct graphics_context;
> @@ -154,6 +156,8 @@ public slots:
>  	void hideEvents();
>  	void removeEvent();
>  	void addBookmark();
> +private slots:
> +	void plot2();
>  private:
>  	void plot_depth_profile();
>  	QGraphicsItemGroup *plot_text(text_render_options_t *tro, const QPointF& pos, const QString &text, QGraphicsItem *parent = 0);
> @@ -211,6 +215,10 @@ private:
>  	GraphicsTextEditor *timeEditor;
>  
>  	enum Mode mode;
> +
> +	//Used to call plot2
> +	QFutureWatcher<struct plot_info *> *watcher = NULL;
> +	QFuture<struct plot_info *> future;
>  };
>  
>  #endif