[platform-debug-dev] Memory View Infrastructure Review: Part 1

[Darin Wright <Darin_Wright@xxxxxxxxxx>]

Samantha,

I have read/reviewed the support in org.eclipse.debug.core relating to the 
memory view infrastructure: i.e. extension point, classes and interfaces. 
I want to be sure that I have a clear understanding of the core 
infrastructure before I move on to the UI portion of the infrastructure. 
Since there may be other interested parties, I suggest we conduct the 
review on this mailing list in an open fashion.

To start, here's my understanding of the "memory view model": Arbitrary 
memory blocks can be retrieved from debuggees (this was always part of the 
model). A memory block can be rendered in N different ways (N >= 0). A 
memory block has a data type associated with it which defines the type of 
data contained in the memory block - for example, character data, 
numerical data, or image data. The types of memory are open ended and 
client defined (however, the platform could provide some renderings for 
common types such as ASCII). The monitoring of memory blocks can be 
expensive (in terms of size and communication with a debuggee). Thus, a 
memory block manager is used to manage actively monitored memory blocks. 
Similarly, since the mapping of memory block types to possible renderings 
is 1:N, there is a memory block rendering manager that manages the active 
renderings of currently monitored memory blocks. A memory block 
implementation can be responsible for change notification itself (i.e. 
notification of changes in its bytes), or it can let the client (memory 
view) manage/compute changes.

Here's a list of design issues that need to be considered (please let me 
know your opinion):
* The memory renderings extension point uses the fully qualified name of a 
Java class to identify types of memory blocks. This is used to bind memory 
blocks to default renderings and available renderings. Using a class name 
is fragile and inflexible (i.e. changing an implementation class name 
breaks extension points). It is also complex since the current 
implementation uses the class/interface hierarchy to determine all 
renderings for a memory block. Instead we should define an additional 
method IExtendedMemoryBlock.getMemoryBlockType(), which returns a string 
uniquely identifying the memory block type (i.e. its content/data type). 
Is there a reason that we need complexity of multiple inheritance to 
define memory block types?
* There is confusion in the naming scheme for the memory renderings 
extension point and implementation interfaces/classes. The extension point 
is called "memoryRenderings", however the IMemoryRendering interface is an 
actual instance of a rendering, not an extension. I think the "type 
pattern" which is used commonly in the debug platform should also be used 
here. That is, the extension point should be renamed to 
"memoryRenderingTypes" to represent a class of renderings. Then, a memory 
rendering type can have a factory to create renderings (instances) of its 
type. IMemoryRendering will represent an actual rendering (as it does 
now), and IMemoryRenderingInfo will be renamed to IMemoryRenderingType.
* The rendering_property element is a sub-element of a rendering element, 
and it is thus redundant to specify a renderingId (it is inherent in the 
rendering element it is a child of). The attribute should be removed. The 
memory renderings manager treats the property elements as both top-level 
elements and children elements when initializing extension point 
information - but only one format should be allowed (i.e. only as a child 
element of a rendering element). 
* Memory blocks have an undocumented lifecycle (which should be 
documented). The delete() method is called when the block is removed from 
the memory block manager, but there is no corresponding init() when the 
block is added (this method should be added, and to be consistent with 
other lifecycle methods, delete() should be renamed to dispose()). Also 
doc that memory blocks are automatically disposed (removed) from the 
manager when their target terminates. Memory renderings have a similar 
lifecycle that needs to be documented - i.e. renderings are automatically 
removed for terminated targets, and for removed memory blocks. Should 
renderings have lifecycle methods too?

Other questions:
* Are rendering properties optional in a rendering extension?
* What does it mean to use a default rendering factory for a rendering 
extension? (Answer from reading code: it means that the rendering is 
created with the same memory block as the original. I suppose this means 
that a factory can change the bytes in the memory block? Is that the sole 
purpose of a rendering factory?)
* The factory specified for hex renderings is ill-specified in plug-in XML 
? it refers to a non-existant class 
(org.eclipse.debug.internal.ui.views.memoryHexRenderingFactory - note the 
missing "." between memory and HexRenderingFactory). This means the 
default factory is being used. However the rendering that the hex factory 
creates (HexRendering) is no different than a default rendering. So why 
does it exist?
* Can one provide non-textual renderings? For example, render bytes as an 
image? (I didn't get to the UI code yet, but I presume this is possible)
* What types of rendering properties are used/supported? What are dynamic 
renderings? So far, I can see only this explicit use of properties, but 
it?s not clear to me what a dynamic rendering is.

I did not change any code yet, but I modifed the memoryRenderings schema 
to reflect my understanding of things:
* I modified the wording in the memoryRenderings schema to note that the 
extension point becomes public in 3.1, and added references to the memory 
block type identifier. Deprecated the memoryBlockClass attribute of the 
default renderings and rendering binding elements. Removed renderingId 
from the renderingProperty element.

Other review notes:
* As a matter of style/consistency with other extension points, the names 
of elements in the memoryRenderings extension point should be changed from 
"rendering_property" to "renderingProperty", etc.
* An example extension should be included in the schema (in an Example 
section), and API information should be provided in the API section - i.e. 
describing the API references from the example.
* The "Supplied Implementation" section should describe what the debug 
platform provides - i.e. a HEX rendering.
* I have other review notes for the code (i.e. written on printouts), not 
included here


Thanks,

Darin