Opened 12 years ago

Closed 9 years ago

#47 closed feature request (done)

Implement an engine for debugging purposes

Reported by: swagner Owned by: epitzer
Priority: medium Milestone: HeuristicLab 3.3.3
Component: DebugEngine Version: 3.3.3
Keywords: Cc:

Description

Developing complex algorithms is a quite difficult and error-prone task. Therefore a special engine for debugging algorithms would be very helpful. This engine should offer more insights into the operator graph and the scope tree during the execution of an algorithm.

Change History (66)

comment:1 Changed 12 years ago by swagner

  • Type changed from enhancement to feature request

comment:2 Changed 12 years ago by swagner

  • Milestone changed from HeuristicLab 3.0 to HeuristicLab 3.1
  • Version changed from 3.0 to 3.1

comment:3 Changed 11 years ago by swagner

  • Milestone changed from 3.1 to 3.2
  • Version changed from 3.1 to 3.2

comment:4 Changed 11 years ago by swagner

  • Milestone changed from 3.2 to Iteration 0

Milestone 3.2 deleted

comment:5 Changed 11 years ago by swagner

  • Milestone changed from Iteration 1 to Iteration 99

comment:6 Changed 10 years ago by swagner

  • Priority changed from critical to trivial

comment:7 Changed 9 years ago by swagner

  • Version changed from 3.2 to 3.3

comment:8 Changed 9 years ago by epitzer

  • Owner changed from swagner to epitzer
  • Priority changed from lowest to low
  • Status changed from new to accepted
  • Version changed from 3.3 to 3.3.1

comment:9 Changed 9 years ago by epitzer

Preliminary version of debug engine with stepping, execution stack view and current operator scope view (r4743)

comment:10 Changed 9 years ago by epitzer

Correct reference hint path (r4744)

comment:11 Changed 9 years ago by epitzer

Show and link operator parameters and variables (r4746)

comment:12 Changed 9 years ago by epitzer

add license (r4747)

comment:13 Changed 9 years ago by swagner

  • Component changed from General to DebugEngine
  • Milestone changed from Iteration 99 to Iteration 4
  • Version changed from 3.3.1 to branch

comment:14 Changed 9 years ago by epitzer

Show expected parameter values and include type information in tool tip (r4753)

comment:15 Changed 9 years ago by epitzer

navigate execution contexts show active scopes (r4759)

comment:16 Changed 9 years ago by epitzer

Show parameters of active execution context in DebugEngine (r4765)

comment:17 Changed 9 years ago by epitzer

Prevent stepping execution on empty execution stack (r4827)

comment:18 Changed 9 years ago by epitzer

Refactoring and modularization of DebugEngine (r4871)

comment:19 Changed 9 years ago by epitzer

Complete overhaul of DebugEngine (r4876)

comment:20 Changed 9 years ago by epitzer

  • Milestone changed from HeuristicLab x.x.x to HeuristicLab 3.3.3
  • Priority changed from low to lowest

Many small improvements(r4903)

  • suppress logging during execution
  • add refresh button
  • optionally skip over execution stack operators
  • expand all tree views and scroll to top node
  • show operator on click on atomic operation
  • add build.cmd

comment:21 Changed 9 years ago by epitzer

  • Priority changed from lowest to medium

comment:22 Changed 9 years ago by epitzer

Add operator trace view (r4904)

comment:23 Changed 9 years ago by epitzer

Several GUI improvements (r4909)

  • add icons and tool tips
  • add support for suspending the operator trace view
  • faster skipping of stack-only operations
  • remove log view and execution time view

comment:24 Changed 9 years ago by epitzer

  • Owner changed from epitzer to swagner
  • Status changed from accepted to reviewing

comment:25 Changed 9 years ago by swagner

  • Owner changed from swagner to mkommend

mkommend, please review this first and return it to me for a second review afterwards. Thanks.

comment:26 Changed 9 years ago by epitzer

Better display and highlighting of scopes (r4930)

comment:27 Changed 9 years ago by epitzer

Add tool tip for current scope (r4931)

comment:28 Changed 9 years ago by mkommend

Reviewing comments

  • DebugEngine
    • Is it always possible to cast an IOperation to an IExecutionContext? Otherwise checks must be inserted.
    • IOperation.Operator could be null.
    • In my opition the ignoreNextBreakPoint flag should be removed as this adds additional complexity.

comment:29 Changed 9 years ago by mkommend

  • Owner changed from mkommend to epitzer
  • Status changed from reviewing to assigned

Minor changes in DebugEngine with r4946.

comment:30 Changed 9 years ago by epitzer

  • Owner changed from epitzer to mkommend
  • Status changed from assigned to reviewing

Simpler breakpoint handling, fix cloning, prevent stepping while running (r4947)

comment:31 Changed 9 years ago by mkommend

  • Owner changed from mkommend to epitzer
  • Status changed from reviewing to assigned

review comments:

  • set contents of detail views to null if the DebugEngine is running
  • create seperate class for OperatorTrace
  • permanently highlight execution contexts' scopes
  • check if execution context has always the same parameters as the operator => remove paramters view

comment:32 Changed 9 years ago by epitzer

  • create own class for OperatorTrace
  • remove unnecessary event handlers
  • prevent flickering while stepping
  • permanently highlight execution context's scope

(r4993)

comment:33 Changed 9 years ago by epitzer

Move parent tracing to operator trace class (r4996)

comment:34 Changed 9 years ago by epitzer

  • Owner changed from epitzer to mkommend
  • Status changed from assigned to reviewing

remove resources, add license headers, update plug-in dependencies (r4998)

comment:35 follow-up: Changed 9 years ago by mkommend

review comments:

  • Is the CurrentOperator property really necessary? As far as I can see it only gets the value of CurrentOperation.Operator.
  • There should also be events for changes of the ExecutionStack, the OperatorTrace and the Log, similar to CurrentOperationChanged event. Another possibility would be to declare the DebugEngine as sealed class.
  • Is it possible to make the RegisterParenthood method private and to remove the call to it in DebugEngine line 283.
  • OperatorTrace.Generate should be static and return a new OperatorTrace object.
  • The Suspend- and ResumeUpdate methods in the OperatorTraceView and the ExecutionStackView are not needed anymore and should be removed.

comment:36 Changed 9 years ago by mkommend

  • Owner changed from mkommend to epitzer
  • Status changed from reviewing to assigned

comment:37 in reply to: ↑ 35 Changed 9 years ago by epitzer

Replying to mkommend:

Is the CurrentOperator property really necessary? As far as I can see it only gets the value of CurrentOperation.Operator.

The currentOperator field is only set during the execution of an operator and is used to abort the operator.

There should also be events for changes of the ExecutionStack, the OperatorTrace and the Log, similar to CurrentOperationChanged event. Another possibility would be to declare the DebugEngine as sealed class.

The references to these objects can never change during the lifetime of a DebugEngine (except during cloning and serialization), the setters will been marked private.

Is it possible to make the RegisterParenthood method private and to remove the call to it in DebugEngine line 283.

Tracking parenthood is one of the two core functionalities provided by this class and cannot be removed.

OperatorTrace.Generate should be static and return a new OperatorTrace object.

The call to Generate uses the collected parenthood information stored inside the OperatorTrace to repopulate itself and can, therefore, not be made static.

The Suspend- and ResumeUpdate methods in the OperatorTraceView and the ExecutionStackView are not needed anymore and should be removed.

This will fixed in the next commit.

comment:38 Changed 9 years ago by epitzer

private setters for ExecutionStack, OperatorTrace and Log, remove unneeded SuspendUpdate and ResumeUpdate methods (r5001)

Last edited 9 years ago by epitzer (previous) (diff)

comment:39 Changed 9 years ago by epitzer

unseal views, rename OperatorTrace.Generate to Regenerate, and remove questionable virtual from property with private setter (r5002)

Last edited 9 years ago by epitzer (previous) (diff)

comment:40 Changed 9 years ago by epitzer

replace parameter view with context menu to access variable values in execution context view (r5003)

comment:41 Changed 9 years ago by epitzer

  • Owner changed from epitzer to swagner
  • Status changed from assigned to reviewing

comment:42 Changed 9 years ago by epitzer

disable context menu "Show Value" if value is null or not an IContent (r5077)

comment:43 Changed 9 years ago by swagner

Integrated DebugEngine into trunk in r5114.

epitzer please review the trunk integration and delete the DebugEngine branch, if everything is ok. Thanks.

comment:44 Changed 9 years ago by swagner

  • Version changed from branch to 3.3.2

comment:45 Changed 9 years ago by swagner

Fixed project references in r5115.

comment:46 Changed 9 years ago by swagner

The DebugEngine is really great. Many thanks to epitzer for implementing it.

Some review comments on the UI:

  • Using a bold font for highlighting nodes in a tree view may lead to clipped node texts (see for example the "Global Scope" tree node). This probably is a windows forms bug. I recommend to avoid bold fonts. Instead only foreground or background colors should be used.
  • The text of the context menu in the execution context tree view should be "Show Actual Value" and not "show actual value".
  • When the user right clicks into the execution context tree view, the selected node should be changed. Otherwise the context menu is shown for the currently selected node and not for the clicked node.
  • The margins of controls inside group boxes are not equal. Some of them are too small (for example in the execution stack group box, the execution context group box, etc.).
  • The actual names of lookup parameters should be shown in the execution context tree view. I would recommend the format "<name>=<value>" for value parameters and "<name> (<actual name>)" for lookup parameters.
  • When stepping through an algorithm, in each second step the operation group box contains no data. What is the purpose of this behavior?
  • What is the purpose of the labels "Context", "Atomic" and "Collection"? It seems to me that these labels do not really provide much information. Maybe they can be removed?
  • Saving an algorithm with a debug engine takes a long time. Is it possible to improve the persistence of debug engines?
Last edited 9 years ago by swagner (previous) (diff)

comment:47 Changed 9 years ago by swagner

  • Owner changed from swagner to epitzer
  • Status changed from reviewing to assigned

comment:48 Changed 9 years ago by epitzer

  • fixed truncation of bold text in TreeView
  • changed "show actual value" initial caps
  • select node on right click
  • fixed margins
  • include actual name for parameters
  • remove unnecessary labels

(r5116)

comment:49 Changed 9 years ago by epitzer

(r5117)

  • Disable detailed logging while not stepping for drastic reduction in log size
  • Optionally disable operator trace for much faster execution and persistence (disabled by default)
  • Initially enable stepping over stack operations

comment:50 Changed 9 years ago by epitzer

Show empty operator trace when not enabled (r5121)

comment:51 Changed 9 years ago by epitzer

Enable showing actual values for parameters in non-current execution contexts (r5122)

comment:52 Changed 9 years ago by epitzer

Correctly select parent.tag instead of just parent when trying to show the actual value (r5124)

comment:53 Changed 9 years ago by epitzer

(r5146)

  • Show a different icon for breakpoints
  • Show an icon for the active operation

comment:54 Changed 9 years ago by epitzer

  • Owner changed from epitzer to swagner
  • Status changed from assigned to reviewing

@swagner: Please review final changes and consider for release.

comment:55 Changed 9 years ago by epitzer

deleted branch (r5167)

comment:56 Changed 9 years ago by epitzer

Correctly show whether operator trace is enabled (r5174)

comment:57 Changed 9 years ago by swagner

Great thanks. I reviewed the last changes and came across some additional comments:

  • It seems that nested operation collections are not correctly shown in the execution stack. For example, have a look at the execution stack after the SubScopesProcessor following the selector in the GA & TSP sample is executed:
Shown execution stack:
+ - 2 Operations
| + - SubScopesProcessor
+ - 2 Operations
  + - EmptyOperator
  + - ChildrenCreator

Correct execution stack:
+ - 2 Operations
  + - 2 Operations
  | + - EmptyOperator
  | + - ChildrenCreator
  + - SubScopesProcessor
  • It should be shown in the execution stack, if an operation collection is marked as parallel or not. I would recommend to simply change the tree node text to "x Parallel Operations" in that case.
  • Maybe single steps should also be executed asynchronously. Some steps really take some time and the UI freezes for a noticeable time in that case.
  • Please add spaces before and after equal signs and parentheses in the execution context view and the scope view (for example "Maximization = False" or "Quality (TSPTourLength) = ItemArray<DoubleValue>").
  • Is the operator trace list view correctly disabled, if the operator trace is not enabled? I would expect that it to be grayed out in that case.
  • null values of parameters should be visualized as "null" instead of displaying nothing.

comment:58 Changed 9 years ago by swagner

  • Owner changed from swagner to epitzer
  • Status changed from reviewing to assigned

comment:59 Changed 9 years ago by epitzer

  • Status changed from assigned to accepted

comment:60 Changed 9 years ago by epitzer

Properly show nesting in execution stack and remove unused suspension code and display "Parallel" for parallel operation collections (r5219)

Last edited 9 years ago by epitzer (previous) (diff)

comment:61 Changed 9 years ago by epitzer

Better spacing for parameters and display of "null" for null values in parameter view (r5220)

comment:62 Changed 9 years ago by epitzer

Disable list view control in operator trace if operator trace is itself disabled (r5221)

comment:63 Changed 9 years ago by epitzer

Use a background worker to execute single steps to prevent the GUI locking up for long-running operations (r5222)

comment:64 Changed 9 years ago by epitzer

  • Owner changed from epitzer to swagner
  • Status changed from accepted to reviewing

@swagner: Thanks for the feedback! All issues raised in #comment:57 should be fixed now.

comment:65 Changed 9 years ago by swagner

  • Owner changed from swagner to epitzer
  • Status changed from reviewing to readytorelease

Perfect, thanks.

comment:66 Changed 9 years ago by mkommend

  • Resolution set to done
  • Status changed from readytorelease to closed
  • Version changed from 3.3.2 to 3.3.3
Note: See TracTickets for help on using tickets.