views:

541

answers:

11

A certain form in our application displays a graphical view of a model. The user can, amongst loads of other stuff, initiate a transformation of the model that can take quite some time. This transformation sometimes proceeds without any user interaction, at other times frequent user input is necessary. While it lasts the UI should be disabled (just showing a progress dialog) unless user input is needed.

Possible Approaches:

  1. Ignore the issue, just put the transformation code in a procedure and call that. Bad because the app seems hung in cases where the transformation needs some time but requires no user input.
  2. Sprinkle the code with callbacks: This is obtrusive - you’d have to put a lot of these calls in the transformation code - as well as unpredictable - you could never be sure that you’d found the right spots.
  3. Sprinkle the code with Application.ProcessMessages: Same problems as with callbacks. Additionally you get all the issues with ProcessMessages.
  4. Use a thread: This relieves us from the “obtrusive and unpredictable” part of 2. and 3. However it is a lot of work because of the “marshalling” that is needed for the user input - call Synchronize, put any needed parameters in tailor-made records etc. It’s also a nightmare to debug and prone to errors.

//EDIT: Our current solution is a thread. However it's a pain in the a** because of the user input. And there can be a lot of input code in a lot of routines. This gives me a feeling that a thread is not the right solution.

I'm going to embarass myself and post an outline of the unholy mix of GUI and work code that I've produced:

type
  // Helper type to get the parameters into the Synchronize'd routine:
  PGetSomeUserInputInfo = ^TGetSomeUserInputInfo;
  TGetSomeUserInputInfo = record
    FMyModelForm: TMyModelForm;
    FModel: TMyModel;
    // lots of in- and output parameters
    FResult: Boolean;
  end;

{ TMyThread }

function TMyThread.GetSomeUserInput(AMyModelForm: TMyModelForm;
  AModel: TMyModel; (* the same parameters as in TGetSomeUserInputInfo *)): Boolean;
var
  GSUII: TGetSomeUserInputInfo;
begin
  GSUII.FMyModelForm := AMyModelForm;
  GSUII.FModel := AModel;
  // Set the input parameters in GSUII

  FpCallbackParams := @GSUII; // FpCallbackParams is a Pointer field in TMyThread
  Synchronize(DelegateGetSomeUserInput);
  // Read the output parameters from GSUII
  Result := GSUII.FResult;
end;

procedure TMyThread.DelegateGetSomeUserInput;
begin
  with PGetSomeUserInputInfo(FpCallbackParams)^ do
    FResult := FMyModelForm.DoGetSomeUserInput(FModel, (* the params go here *));
end;

{ TMyModelForm }

function TMyModelForm.DoGetSomeUserInput(Sender: TMyModel; (* and here *)): Boolean;
begin
  // Show the dialog
end;

function TMyModelForm.GetSomeUserInput(Sender: TMyModel; (* the params again *)): Boolean;
begin
  // The input can be necessary in different situations - some within a thread, some not.
  if Assigned(FMyThread) then
    Result := FMyThread.GetSomeUserInput(Self, Sender, (* the params *))
  else
    Result := DoGetSomeUserInput(Sender, (* the params *));
end;

Do you have any comments?

+1  A: 

Process asynchronously by sending a message to a queue and have the listener do the processing. The controller sends an ACK message to the user that says "We've received your request for processing. Please check back later for results." Give the user a mailbox or link to check back and see how things are progressing.

duffymo
+4  A: 

Definitely go for a threaded option (even after your edit, saying you find it complex). The solution that duffymo suggests is, in my opinion, very poor UI design (even though it's not explicitly about the appearance, it is about how the user interfaces with your application). Programs that do this are annoying, because you have no idea how long the task will take, when it will complete, etc. The only way this approach could be made better would be by stamping the results with the generation date/time, but even then you require the user to remember when they started the process.

Take the time/effort and make the application useful, informative and less frustrating for your end user.

James Burgess
+2  A: 

TThread is perfect and easy to use.

Develope and debug your slow function.

if it is ready, put the call into the tthread execute method. Use the onThreadTerminate Event to find out the end of you function.

for user feedback use syncronize!

Bernd Ott
That's roughly what we're doing now but due to the amount of possible user interaction the synchronization get's really annoying. :-)
Ulrich Gerhardt
But what kind of interaction can happen?normaly there is a progressbar and a cancel button.
Bernd Ott
That depends - sometimes really just a progress bar. At other times tons of questions: "Do you want this here or there?" "Do you want an X or an Y?" Or: "Attention - you specified option ABC once ago which crashes with your most recent changes. Which shall it be?"
Ulrich Gerhardt
I like Delphi threads, but to say "TThread is perfect" is a great exaggeration.
JosephStyons
Uli, then you should think about a redesign of that task.Let the Thread run until the first question is coming up.return to the main-vcl thread, do the form stuff and then restart the thread or create a new one.
Bernd Ott
@bott: What's the point then of using a thread at all?
mghie
* not freezing the gui - most important - because user can think application is crashed and they maybe terminate the process with taskmanager* use benefits of hyperthreading/multi-cpu machines
Bernd Ott
The first can be achieved with Application.ProcessMessages() and check for cancel request or execution in Application.OnIdle() as well. The second is not true if the GUI thread is only waiting for a single worker thread to finish. Also TThread.Synchronize(), that spawn from hell, is going to kill...
mghie
any benefit of hyperthreading or multi-core machines by forcing everything through one giant serialization mechanism, which is also extremely error-prone to use. For more information search for 'Ways to get into avoidable trouble with threads, V1.2' at http://www.delphigroups.info/2/4/208119.html.
mghie
+2  A: 

For an optimal solution you will have to analyse your code anyway, and find all the places to check whether the user wants to cancel the long-running operation. This is true both for a simple procedure and a threaded solution - you want the action to finish after a few tenths of a second to have your program appear responsive to the user.

Now what I would do first is to create an interface (or abstract base class) with methods like:

IModelTransformationGUIAdapter = interface
  function isCanceled: boolean;
  procedure setProgress(AStep: integer; AProgress, AProgressMax: integer);
  procedure getUserInput1(...);
  ....
end;

and change the procedure to have a parameter of this interface or class:

procedure MyTransformation(AGuiAdapter: IModelTransformationGUIAdapter);

Now you are prepared to implement things in a background thread or directly in the main GUI thread, the transformation code itself will not need to be changed, once you have added code to update the progress and check for a cancel request. You only implement the interface in different ways.

I would definitely go without a worker thread, especially if you want to disable the GUI anyway. To make use of multiple processor cores you can always find parts of the transformation process that are relatively separated and process them in their own worker threads. This will give you much better throughput than a single worker thread, and it is easy to accomplish using AsyncCalls. Just start as many of them in parallel as you have processor cores.

Edit:

IMO this answer by Rob Kennedy is the most insightful yet, as it does not focus on the details of the implementation, but on the best experience for the user. This is surely the thing your program should be optimised for.

If there really is no way to either get all information before the transformation is started, or to run it and patch some things up later, then you still have the opportunity to make the computer do more work so that the user has a better experience. I see from your various comments that the transformation process has a lot of points where the execution branches depending on user input. One example that comes to mind is a point where the user has to choose between two alternatives (like horizontal or vertical direction) - you could simply use AsyncCalls to initiate both transformations, and there are chances that the moment the user has chosen his alternative both results are already calculated, so you can simply present the next input dialog. This would better utilise multi-core machines. Maybe an idea to follow up on.

mghie
The transformation itself can't be easily parallelized. It really is a series of steps that depend on each other. We use the thread just as vehicle to keep the app responsive without. Stupid idea?
Ulrich Gerhardt
Well, with the outlined design you can switch between them and decide for yourself. But as I said, I'd not use a thread for that if your only GUI in this time is a modal progress indicator. Multithreading works best if other work can be done in parallel, else there are easier (to debug) ways.
mghie
IIUYC the GuiAdapter approach is similar to what we already have. However we didn't use a centralized interface, but a bunch of events. E.g. TMyModelForm.GetSomeUserInput from my original posting is hooked to an event property in the model which gets called during the transformation.
Ulrich Gerhardt
It's indeed similar, I just prefer the interface because it's more straightforward, easier to setup, and easier to test. You see that in your snippet you have more indirection, and you needed to write more glue code. I'd also not use Synchronize() if it can be avoided, see my comments to bott.
mghie
I accepted Rob's answer. Hope you don't mind. :-)
Ulrich Gerhardt
Of course not, as I already said it has been the most insightful answer! I hope you find a good solution to your problem.
mghie
+4  A: 

I think as long as your long-running transformations require user interaction, you're not going to be truly happy with any answer you get. So let's back up for a moment: Why do you need to interrupt the transformation with requests for more information? Are these really questions you couldn't have anticipated before starting the transformation? Surely the users aren't too happy about the interruptions, either, right? They can't just set the transformation going and then go get a cup of coffee; they need to sit and watch the progress bar in case there's an issue. Ugh.

Maybe the issues the transformation encounters are things that could be "saved up" until the end. Does the transformation need to know the answers immediately, or could it finish everything else, and then just do some "fix-ups" afterward?

Rob Kennedy
That's about what I wanted to hear and what I feared to hear simultaneously. :-) I agree that the plethora of questions will annoy the users. But I'm not allowed to decide that. And with most of the questions I don't see how to move them to the start or the end of the transformation.
Ulrich Gerhardt
+1  A: 

While I don't completely understand what your trying to do, what I can offer is my view on a possible solution. My understanding is that you have a series of n things to do, and along the way decisions on one could cause one or more different things to be added to the "transformation". If this is the case, then I would attempt to separate (as much as possible) the GUI and decisions from the actual work that needs to be done. When the user kicks off the "transformation" I would (not in a thread yet) loop through each of the necessary decisions but not performing any work...just asking the questions required to do the work and then pushing the step along with the parameters into a list.

When the last question is done, spawn your thread passing it the list of steps to run along with the parameters. The advantage of this method is you can show a progress bar of 1 of n items to give the user an idea of how long it might take when they come back after getting their coffee.

skamradt
A: 

If you can split your transformation code into little chunks, then you can run that code when the processor is idle. Just create an event handler, hook it up to the Application.OnIdle event. As long as you make sure that each chunk of code is fairly short (the amount of time you want the application to be unresponsive...say 1/2 a second. The important thing is to set the done flag to false at the end of your handler :

procedure TMyForm .IdleEventHandler(Sender: TObject;
  var Done: Boolean);
begin
  {Do a small bit of work here}
  Done := false;
end;

So for example if you have a loop, instead of using a for loop, use a while loop, make sure the scope of the loop variable is at the form level. Set it to zero before setting the onIdle event, then for example perform 10 loops per onidle hit until you hit the end of the loop.

Count := 0;
Application.OnIdle := IdleEventHandler;

...
...

procedure TMyForm .IdleEventHandler(Sender: TObject;
  var Done: Boolean);
var
  LocalCount : Integer;
begin
  LocalCount := 0;

  while (Count < MaxCount) and (Count < 10) do
  begin
    {Do a small bit of work here}
    Inc(Count);
    Inc(LocalCount);
  end;
  Done := false;
end;
Steve
+2  A: 

I think your folly is thinking of the transformation as a single task. If user input is required as part of the calculation and the input asked for depends on the caclulation up to that point, then I would refactor the single task into a number of tasks.

You can then run a task, ask for user input, run the next task, ask for more input, run the next task, etc.

If you model the process as a workflow, it should become clear what tasks, decisions and user input is required.

I would run each task in a background thread to keep the user interface interactive, but without all the marshaling issues.

Daniel Paull
+1  A: 

I'd certainly go with threads. Working out how a thread will interact with the user is often difficult, but the solution that has worked well for me is to not have the thread interact with the user, but have the user side GUI interact with the thread. This solves the problem of updating the GUI using synchronize, and gives the user more responsive activity.

So, to do this, I use various variables in the thread, accessed by Get/Set routines that use critical sections, to contain status information. For starters, I'd have a "Cancelled" property for the GUI to set to ask the thread to stop please. Then a "Status"property that indicates if the thread is waiting, busy or complete. You might have a "human readable" status to indicate what is happening, or a percentage complete.

To read all this information, just use a timer on the form and update. I tend to have a "statusChanged" property too, which is set if one of the other items needs refreshing, which stops too much reading going on.

This has worked well for me in various apps, including one which displays the status of up to 8 threads in a list box with progress bars.

mj2008
A: 

Thank you all very much for your answers. You've provided me with a lot of options to think about and new views on facets where I got stuck.

Ulrich Gerhardt
+1  A: 

If you decide going with Threads, which I also find somewhat complex the way they are implemented in Delphi, I would recommend the OmniThreadLibrary by Primož Gabrijelčič or Gabr as he is known here at Stack Overflow.

It is the simplest to use threading library I know of. Gabr writes great stuff.

lkessler