Difference between revisions of "Progress Reporting"

From Eclipsepedia

Jump to: navigation, search
(Cleaning-up as this is impossible to edit.)
Line 1: Line 1:
<p style="margin: 0.0px 0.0px 14.0px 0.0px; font: 18.0px Helvetica"><b>Are you working with Progress Monitors, and wish you did not have too?</b><br/>
+
I recently got to investigate issues in p2 why the progress bar did not move while it was very clear that things where happening as subtasks changed the text. Looking into what was going on made me find a new Progress Monitor anti pattern.
<span style="font:10.0px Helvetica">''by Henrik Lindberg - originally posted on [http://henrik-eclipse.blogspot.com/2009/05/progress-monitor-patterns.html Eclipse by Planatery Transitions]''</span></p>
+
 
<p style="margin: 0.0px 0.0px 12.0px 0.0px; font: 12.0px Helvetica">I recently got to investigate issues in p2 why the progress bar did not move while it was very clear that things where happening as subtasks changed the text. Looking into what was going on made me find a new Progress Monitor anti pattern.</p>
+
The problem is how to handle a case where you need to do something like this:
<p style="margin: 0.0px 0.0px 12.0px 0.0px; font: 12.0px Helvetica">The problem is how to handle a case where you need to do something like this:</p>
+
 
<code>
+
<source lang="java">
<table border="0" cellpadding="3" cellspacing="0" bgcolor="#FFFFFF">
+
for (int i = 0; i < candidates.length; i++) {
<tr>
+
  if (loadCandidate1(i)) {
<td  valign="top" align="left"><font color="#FFFFFF">&nbsp;&nbsp;&nbsp;&nbsp;</font><font color="#7F0055"><b>for</b></font> <font color="#000000">(</font><font color="#7F0055"><b>int</b></font> <font color="#000000">i =</font> <font color="#990000">0</font><font color="#000000">; i &lt; candidates.length; i++</font><font color="#000000">)</font><br />
+
    break;
<font color="#FFFFFF">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</font><font color="#7F0055"><b>if</b></font> <font color="#000000">(</font><font color="#000000">loadCandidate1</font><font color="#000000">(</font><font color="#000000">i</font><font color="#000000">))</font><br />
+
  }
<font color="#FFFFFF">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</font><font color="#7F0055"><b>break</b></font><font color="#000000">;</font></td>
+
}
</tr>
+
</source>
</table>
+
 
</code>
+
Where a call to <code>loadCandidate(int)</code> is potentially a long running task. The first loaded candidate means we are done.
<!-- ======================================================== -->
+
 
<p style="margin: 0.0px 0.0px 12.0px 0.0px; font: 12.0px Helvetica"><br /></p>
+
==Antipattern==
<p style="margin: 0.0px 0.0px 12.0px 0.0px; font: 12.0px Helvetica">Where a call to <span style="font: 12.0px Monaco">loadCandidate</span> is potentially a long running task. The first loaded candidate means we are done.</p>
+
Here is an implementation of the above example using the antipattern - i.e. "don't do this":
<p style="margin: 0.0px 0.0px 15.0px 0.0px; font: 14.0px Helvetica"><b>Antipattern<br /></b></p>
+
 
<p style="margin-top: 0px; margin-right: 0px; margin-bottom: 12px; margin-left: 0px; font: 12px Helvetica;"><span style="font-size: 12px; font-weight: normal;">Here is an implementation of the above example using the antipattern - i.e. "don't do this":</span></p>
+
<source lang="java">
<code>
+
public void antiPattern(IProgressMonitor monitor) {
<table border="0" cellpadding="3" cellspacing="0" bgcolor="#FFFFFF">
+
  SubMonitor sub = SubMonitor.convert(monitor, candidates.length);
<tr>
+
  for (int i = 0; i < candidates.length; i++) {
 +
    if (loadCandidate(i, sub.newChild(1))) {
 +
      break;
 +
    }
 +
  }
 +
}
 +
 
 +
public boolean loadCandidate(int index, IProgressMonitor monitor) {
 +
  SubMonitor sub = SubMonitor.convert(monitor, 1000);
 +
  try {
 +
    for (int i = 0; i < 1000; i++) {
 +
      sub.worked(1);
 +
    }
 +
    // ...
 +
    return true;
 +
  } finally {
 +
    monitor.done();
 +
  }
 +
  return false;
 +
}
 +
</source>
 +
 
 
<!-- start source code -->
 
<!-- start source code -->
 
<td nowrap="nowrap" valign="top" align="left"><font color="#FFFFFF">&nbsp;&nbsp;</font><font color="#7F0055"><b>public</b></font> <font color="#7F0055"><b>void</b></font> <font color="#000000">antiPattern</font><font color="#000000">(</font><font color="#000000">IProgressMonitor monitor</font><font color="#000000">) {</font><br />
 
<td nowrap="nowrap" valign="top" align="left"><font color="#FFFFFF">&nbsp;&nbsp;</font><font color="#7F0055"><b>public</b></font> <font color="#7F0055"><b>void</b></font> <font color="#000000">antiPattern</font><font color="#000000">(</font><font color="#000000">IProgressMonitor monitor</font><font color="#000000">) {</font><br />

Revision as of 18:33, 11 May 2009

I recently got to investigate issues in p2 why the progress bar did not move while it was very clear that things where happening as subtasks changed the text. Looking into what was going on made me find a new Progress Monitor anti pattern.

The problem is how to handle a case where you need to do something like this:

for (int i = 0; i < candidates.length; i++) {
  if (loadCandidate1(i)) {
    break;
  }
}

Where a call to loadCandidate(int) is potentially a long running task. The first loaded candidate means we are done.

Antipattern

Here is an implementation of the above example using the antipattern - i.e. "don't do this":

public void antiPattern(IProgressMonitor monitor) {
  SubMonitor sub = SubMonitor.convert(monitor, candidates.length);
  for (int i = 0; i < candidates.length; i++) {
    if (loadCandidate(i, sub.newChild(1))) {
      break;
    }
  }
}
 
public boolean loadCandidate(int index, IProgressMonitor monitor) {
  SubMonitor sub = SubMonitor.convert(monitor, 1000);
  try {
    for (int i = 0; i < 1000; i++) {
      sub.worked(1);
    }
    // ...
    return true;
  } finally {
    monitor.done();
  }
  return false;
}

<td nowrap="nowrap" valign="top" align="left">  public void antiPattern(IProgressMonitor monitor) {
    SubMonitor sub = SubMonitor.convert(monitor, candidates.length);
    for (int i = 0; i < candidates.length; i++) {
      if (loadCandidate(i, sub.newChild(1)))
        break;
    }
  }

  public boolean loadCandidate(int index, IProgressMonitor monitor) {
    SubMonitor sub = SubMonitor.convert(monitor, 1000);
    try {
      for (int i = 0; i < 1000; i++)
        sub.worked(1);
      // ...
      return true;
    } finally {
      monitor.done();
    }
    return false;
  }</td> </tr> </table> </code>

Well, what is wrong with this, you may ask… Well, the length of the progress bar will be divided into as many slots as there are candidates, and if the first candidate succeeds and uses its 1000 ticks, and the remaining candidates are never considered, we will end up reporting the 1000 ticks on a fraction of the overall progress bar. This means that for 10 candidates, you will see the progress-bar slowly go to about 10% of the overall length, to suddenly jump to 100%.


Good pattern

Here is the good pattern that makes use of the full progress bar:

  public void goodPattern(IProgressMonitor monitor) {

    SubMonitor sub = SubMonitor.convert(monitor, 1000);
    for (int i = 0; i < candidates.length; i++) {
      sub.setWorkRemaining(1000);
      if (goodLoadCandidate(i, sub.newChild(1000)))
        break;
    }
  }

  public boolean goodLoadCandidate(int index, IProgressMonitor monitor) {
    SubMonitor sub = SubMonitor.convert(monitor, 1000);
    sub.beginTask(null, 1000);
    try {
      // … code that loads
      for (int i = 0; i < 1000; i++)
        sub.worked(1);

      return true;
    } finally {
      // ignore errors
    }
    return false;

  }

Notice how “setWorkRemaining” is called each time in the loop. This reallocates the remaining ticks, but there is no need to compute how many that actually remains, the child allocation of 1000 ticks will always give the child 1000 ticks to report, even if there were not enough ticks left in the parent. All the scaling is performed by the SubMonitor, so you don’t have to worry about it.

Now, let’s say that the routine you are calling do need to perform a bit of work even if it does not need all of its ticks. Don’t worry, that will work too as long as it is a small portion of the allocation. If you risk consuming a larger part, you may be better off doing a true partitioning of the progress-bar as shown in the next section.


Good pattern - regular loop

Here is the good pattern for a regular loop where each iteration does consume ticks

  public void goodLoopPattern(IProgressMonitor monitor) {

    SubMonitor sub = SubMonitor.convert(monitor, candidates.length*100);
    for (int i = 0; i < candidates.length; i++) {
      if (goodLoopCandidate(i, sub.newChild(100)))
        break;
    }
  }

  public boolean goodLoopCandidate(int index, IProgressMonitor monitor) {
    SubMonitor sub = SubMonitor.convert(monitor, 1000);
    sub.beginTask(null, 1000);
    try {
      // … code that loads
      for (int i = 0; i < 1000; i++)
        sub.worked(1);
      return true;
    } finally {
      // ignore errors
    }
    return false;

  }

Here I simply use SubMonitor’s newChild, this works without calling “done” because the next call to newChild (or “done” for that matter) will consume the ticks allocated for the child.


Rules of Thumb

  1. Use SubMonitor
  2. Always begin by converting the monitor you get to a SubMonitor
  3. Use newChild(n) to create a sub monitor to pass to methods that take an IProgressMonitor as argument.
  4. Never call “done” - but document that the caller must do so unless they used a SubMonitor