[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [platform-debug-dev] Memory View Infrastructure Review: Part 1
|
Hi -
My responses are below, enclosed by the following tag:
=======================================
<Samantha>
=======================================
Thanks
Regards,
----------------------------------------------------------------
Samantha Chan
IBM Toronto Lab
Darin
Wright/Ottawa/IBM
@IBMCA To
Sent by: platform-debug-dev@xxxxxxxxxxx
platform-debug-de cc
v-admin@eclipse.o
rg Subject
[platform-debug-dev] Memory View
Infrastructure Review: Part 1
10/06/2004 03:36
PM
Please respond to
platform-debug-de
v
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?
=======================================
<Samantha>
I agree with you that having the class name in the extension is fragile
and that changing the classname breaks the extension point.
The reason why the classname is used is that I wanted to allow someone to
define a rendering for a particular class. For example, someone might come
in and want to develop a rendering for all types of memory blocks. If we
take
the approach of matching it with a string from getMemoryBlockType, then
we will have to bind all the known memory block types to this new memory
rendering type. If we are matching it to a classname, then we can simply
bind the new rendering with IMemoryBlock. All memory blocks can now take
advantage of this new rendering type.
I know having a rendering type bound to IMemoryBlock is an extreme case.
However
if a debug adapter has multiple types of memory block, then this
may be useful for contributing a basic rendering type to a base memory
block type. And have
more advanced rendering types only contributed to some of the memory
blocks. We cannot
do this type of rendering definition by simply matching it with a string
from
calling IExtendedMemoryBlock.getMemoryBlockType().
Would returning an array of strings from
IExtendedMemoryBlock.getMemoryBlockTypes() a good
alternative? Implementers of IExtendedMemoryBlcok should always call
getMemoryBlockTypes() from its super
class to obtain a list of memory block types. We can have renderings
contributed to specific type of renderings and
have hierarchy taken into account while making it more flexible. (Changing
the classname
won't break the extension.)
=======================================
* 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.
=======================================
<Samantha>
I agree, I think it's a good idea.
=======================================
* 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).
=======================================
<Samantha>
Is there any reason why we should only allow the properties to be defined
as a child
element to a rendering element? What if someone has defined a rendering and
some other parties want to reuse that and extend from that rendering type
by providing additional properties? This is why I allowed the properties to
be defined externally, is to allow the rendering types to be extended.
=======================================
* 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?
=======================================
<Samantha>
The memory block is created by the debug target. I don't see why an init
method is
needed. Is it for consistency? Also, when should the function get called,
right after the
manager has added the memory block to the list?
I agree that it could be useful to have lifecycle methods for
IMemoryRendering too.
=======================================
Other questions:
* Are rendering properties optional in a rendering extension?
=======================================
<Samantha>
Yes, properties are optional and can be defined externally.
Rendering view developers have the responsibility to define what properties
are needed for it to successfully display the renderings.
=======================================
* 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?)
=======================================
<Samantha>
There are two purposes for defining a rendering factory. The first one
being
that the factory gets called to create the rendering, giving the
implementers
a chance to verify things or initialize the renderings.
The second reason is to allow rendering views to allow for object action
contributions.
For example, consider that a view can show two types of renderings:
Rendering A and Rendering B.
There may be actions that you only want them to show up in a view for
Rendering A and not the other rendering.
The factory can create two different types of objects that implement
IMemoryRendering interface.
Since the rendering objects are of different types, now we can have actions
only contributed to
one type of rendering and not the other.
The default rendering factory is a dummy factory that creates a generic
object that implements IMemoryRendering. It is only used when a rendering
factory
is not defined in the extension.
The rendering factory cannot change the bytes in a memory block. It is for
creating
an object that implements IMemoryRendering.
To clarify things, maybe I should have made the rendering factory a
mandatory attribute?
=======================================
* 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?
=======================================
<Samantha>
Since the Memory view only shows one type of rendering, the Hex Rendering.
It does
not really matter if the rendering factory is defined. However, if another
view (a view
that can display multiple rendering types) wants to reuse this rendering
and display it,
then having this attribute defined becomes important. I didn't realize
that I missed a "."
in my extension. Thanks for pointing this out.
=======================================
* 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)
=======================================
<Samantha>
Yes, you can provide non-textual renderings. It's up to rendering view
developers
to figure out what a rendering type really means and define what is needed
to display a rendering type.
=======================================
* 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.
=======================================
<Samantha>
Dynamic renderings should be renamed to dynamic rendering types in the new
scheme.
This is a property for indicating if this rendering type is dynamic,
meaning that
there could be n-number of children rendering types defined under it. It is
not
known at the time of definition how many there are. All children rendering
types
share the same rendering factory and properties defined by this base
rendering type
definition. DynamicRenderingFactory is the only property supported and used
by the
memory rendering manager.
=======================================
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
_______________________________________________
platform-debug-dev mailing list
platform-debug-dev@xxxxxxxxxxx
http://dev.eclipse.org/mailman/listinfo/platform-debug-dev
