You are viewing a plain text version of this content. The canonical link for it is here.
Posted to kato-spec@incubator.apache.org by Stuart Monteith <st...@stoo.me.uk> on 2009/12/09 18:51:54 UTC
Andrew Johnson's comments
Hi,
Here are Andrew Johnson's comments (adjusted for context). The
grammar and spelling errors have been corrected already.
I'll correct the doc when there are undistib
I'll do this by page. Comments to follow in separate email.
Each number is listed like (1)., (2)., etc.
Chapter 3
=========
Page 7
======
Lists
-----
(1).
Instead of:
List<ManagedRuntime> ImageProcess.getRuntimes()
Do we need:
List<? extends ManagedRuntime>
- would allow List<PHPJavaRuntime> getRuntimes()
-----
(2).
Does List equality hold?
E.g. heap.getObjects().equals(heap.getObjects()
-------
(3).
Which methods returning lists throw exceptions?
-------
(4).
Predictable order via different VMs?
Chapter 6
=========
Page 24
=======
(5).
Please have method as page header/footer.
(6).
Walk GC graph using JSR 326.
References: \/---------------------------\
JavaObject \ |
JavaClass | |
JavaStackFrame }- getReferences() getTarget()
JavaThread? |
JavaClassLoader? / <-----------------getSource()
Have interface for classes having getReferences.
Page 25
=======
(7).
re. `services directory'
. This text file should contain a single line which is the package
qualified name of the ImageFactory....
^^^^^^
Multiple factories?
How would multiple factories work in one jar?
(8).
re. Table 6.3 FactoryRegistry Methods
public Iterator FactoryRegistry.iterator()
Iterator<ImageFactory> or List?
(9).
public ImageFactory getFactories()
should be:
public ImageFactory[] getFactories()
(10).
FactoryRegistry.getFactories()
Returns the factories in the registry as an array
Copies?
(I presume Andrew means that the array is mutable without ill effect)
(11).
public Image getImage(File file)
Throws
IllegalArgumentException if file is null
Why not throw NPE?
(12).
List<ImageFactory> sniff file type!
get suggested factories for a file or files:
/ getFactories(File file)
\ getFactories(File file, File file)
package javax.tools.diagnostics.image
======================================
Page 27
=======
(13).
Choose a better order to list the classes.
e.g. ImageFactory, Image, ImageAddressSpace, ImagePointer, ImageSection
(14).
Table 6.4. Interface Summary
Image - This class represents an entire operating system image (e.g. ??
The text after "(e.g." is completely missing.
(15).
Table 6.5. Class Summary
DataUnavailable should be DataUnavailableException
DU extends DUE?
Page 28
=======
(16).
interface ImageModule
--> running header
(I guess Andrew means that the header for the page should have the name
of the interface.
Table 6.6
=========
(17).
public List getSymbols()
should be
public List<ImageSymbol> getSymbols()
(The API has generics added now to the Image API. However, the document
generation needs to be modified.
I won't add any further comments about it.)
Page 29
=======
Table 6.7. ImageMethods
getAddressSpaces()
Also segmented arch?
Page 30
=======
getProcessorCount()
Returns the number of CPUs running in the system on which the image was
running.
System = partition?
(I guess Andrew is suggesting "System" should be described as a
"partition" in a virtual machine, z/Arch LPAR, pSeries LPAR, etc.?)
I'll continue from page 31 soon.
Regards,
Stuart
--
Stuart Monteith
http://blog.stoo.me.uk/
Re: Andrew Johnson's comments
Posted by Andrew Johnson <an...@uk.ibm.com>.
Steve Poole <sp...@googlemail.com> wrote on 10/12/2009 23:04:43:
> > (3).
> >
> > Which methods returning lists throw exceptions?
> >
> > Why is this relevent?
This was just for consistency in the API - having some methods throwing
exceptions rather than returning empty lists or lists with corrupt data
is confusing. There should be good reasons for as deviation from the
norm.
> > -------
> >
> > (4).
> >
> > Predictable order via different VMs?
> >
> > What does this mean ?
>
If you have a list of items then this implies an order. If you have the
same implementation of Kato, but process the dump using two different
VMs to run Kato, should the results come out in the same order? Should
the results come out in the same order from two different runs?
HashSets and HashMaps have unpredictable orders, especially with two
different VMs. If dump processing was multi-threaded then an
implementation could happen to generate results in a different order.
Using an index into a list then becomes less useful.
> > Page 25
> > =======
> > (7).
> >
> > re. `services directory'
> > . This text file should contain a single line which is the package
> > qualified name of the ImageFactory....
> > ^^^^^^
> >
> > Multiple factories?
> > How would multiple factories work in one jar?
> >
> > For each factory you add a new line in the service file. We can make
that
> more clear.
>
>
> > (11).
> >
> > public Image getImage(File file)
> >
> > Throws
> > IllegalArgumentException if file is null
> >
> > Why not throw NPE?
> >
> > Don't like NPEs - they can happen unexpectedly. Throwing a IAE makes
it
> deliberate. Thats what its for,
There are huge arguments on this one!
http://stackoverflow.com/questions/3881/illegalargumentexception-or-nullpointerexception-for-a-null-parameter
Consistency in the API is also important. Do we check for null everywhere?
Is getImage(nullArg) more common than other errors?
> (12).
> >
> > List<ImageFactory> sniff file type!
> >
> > get suggested factories for a file or files:
> >
> > / getFactories(File file)
> > \ getFactories(File file, File file)
> >
> > I think that's too specific - if 2 files why not 3 or 4... If its a
list
> of files what happens if multiple factories can handle one or more of
the
> files?
>
>
>
>
> > (15).
> >
> > Table 6.5. Class Summary
> >
> > DataUnavailable should be DataUnavailableException
> >
> >
> > DU extends DUE?
This was more a style issue - normally exceptions have "Exception" in
their
name, and errors have "Error" in their name. FindBugs thinks it is bad
style
not to. DataUnavailable sounds more like the corresponding Interface to
CorruptData for bad things from lists.
>
>
> > Page 29
> > =======
> >
> > Table 6.7. ImageMethods
> >
> > getAddressSpaces()
> >
> > Also segmented arch?
> >
> > Can you explain some more - I can't guess what you mean.
>
I'm not sure how we should represent dumps from segmented architectures.
Even Windows sometimes does this - having FS or GS mapping to some
thread specific storage, so FS:[0] is always points to the exception
handler chain for that thread.
What's more confusing is that the FS value is
the same for each thread, but as part of a thread switch the mapping
of the FS register is changed.
I think the way to solve this is to have an ImageRegister.getPointer()
method.
You could then do
ImageRegister FS;
FS.getPointer().getIntAt(8);
For an ordinary register then getPointer() returns the value
of the register treated as an address in the process address space.
For more complex cases such as FS:0 then this could help.
Fortunately in Windows then FS has a base address in the same address
space as everything else in the process, so FS.getPointer() returns
a pointer in the standard address space, but each thread's
FS would return a different address in the address space even though
getValue returned the same Number.
Also see ESA/370 access registers:
http://researchweb.watson.ibm.com/journal/rd/464/plambeck.pdf
ESA/370
The ESA/370 architecture retained the address size and
other attributes of 370-XA, added 16 new 32-bit access
registers, and extended the address-space control to two
bits to support two additional translation modes: homespace
mode and access-register mode. In the home-space
mode, instruction and operand addresses are translated
using the home STD in CR13. This was originally used
by the ESA/370 control program (MVS*) and now is used
also for Linux** application programs. In the access-register
(AR) mode, instructions are in the primary address
space, and operands are in AR-specified address spaces.
The instruction field specifying a base GR for address
generation for a storage operand also selects the samenumbered
access register. The access register is loadable
by the application program and contains an access-listentry
token (ALET) that designates, by means of an
access-list-entry number (ALEN) an entry in an access
list. The access-list entry in turn points to an ASTE
which contains an STD that is used by DAT to translate
the virtual address for the storage-operand access. Special
ALET values are defined as designating the current
primary STD and current secondary STD. This
architecture allows a program to associate a different
address space with each base register used for storageoperand
accesses. The mapping of ALETs to STDs, known
as access-register translation, is depicted in Figure 4. This
mapping allows for precise control of access to shared
address spaces by various programs as well as
simultaneous and efficient access to multiple address
spaces by a single program.
For this system:
ImageRegister R1;
R1.getPointer().getIntAt(8);
the Kato implementation would know that R1 was associated
with the address space in AR1 and return an appropriate
pointer in another address space.
Should we have:
ImageRegister.getPointer(long offset)
or perhaps??
ImageRegister.getPointer(Number n)
ImageRegister R1, R2;
R1.getPointer(R2.getValue().intValue()).getIntAt(8);
Is that worth the convenience over this?
R1.getPointer().add(R2.getValue().intValue()).getIntAt(8);
>
> > Page 30
> > =======
> >
> > getProcessorCount()
> >
> > Returns the number of CPUs running in the system on which the image
was
> > running.
> >
> > System = partition?
> >
> > (I guess Andrew is suggesting "System" should be described as a
"partition"
> > in a virtual machine, z/Arch LPAR, pSeries LPAR, etc.?)
> >
> >
> > I don't want too much big iron terminology here. The end user is
mostly
> going to be intel or equivilent based. Partition is something on a disk
to
> them. (Or maybe I'm just getting old)
> Any public proof of the best common terminology to use would be welcome.
>
I guess the system is what the operating system is running on, rather than
the hypervisor's view.
Andrew Johnson
(personal view)
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Andrew Johnson's comments
Posted by Steve Poole <sp...@googlemail.com>.
Andrew - thanks for the comments and thanks to Stuart to writing them up.
As usual answers embeded.
I'm going to comment now and later on create a wiki page to capture all the
changes suggested that need actions.
On Wed, Dec 9, 2009 at 6:51 PM, Stuart Monteith <st...@stoo.me.uk> wrote:
> Hi,
> Here are Andrew Johnson's comments (adjusted for context). The grammar
> and spelling errors have been corrected already.
> I'll correct the doc when there are undistib
>
> I'll do this by page. Comments to follow in separate email.
> Each number is listed like (1)., (2)., etc.
>
> Chapter 3
> =========
>
> Page 7
> ======
>
> Lists
> -----
>
> (1).
>
> Instead of:
> List<ManagedRuntime> ImageProcess.getRuntimes()
> Do we need:
> List<? extends ManagedRuntime>
> - would allow List<PHPJavaRuntime> getRuntimes()
>
>
Good point makes perfect sense!
> -----
>
> (2).
>
> Does List equality hold?
>
> E.g. heap.getObjects().equals(heap.getObjects()
>
> I think it should - but I can imagine this causing some discussion.
-------
> (3).
>
> Which methods returning lists throw exceptions?
>
> Why is this relevent?
> -------
>
> (4).
>
> Predictable order via different VMs?
>
> What does this mean ?
> Chapter 6
> =========
> Page 24
> =======
>
> (5).
>
> Please have method as page header/footer.
>
> Good idea but can't promise it happening quickly.
> (6).
>
> Walk GC graph using JSR 326.
>
> References: \/---------------------------\
> JavaObject \ |
> JavaClass | |
> JavaStackFrame }- getReferences() getTarget()
> JavaThread? |
> JavaClassLoader? / <-----------------getSource()
>
> Have interface for classes having getReferences.
>
> Yes, good suggestion.
> Page 25
> =======
> (7).
>
> re. `services directory'
> . This text file should contain a single line which is the package
> qualified name of the ImageFactory....
> ^^^^^^
>
> Multiple factories?
> How would multiple factories work in one jar?
>
> For each factory you add a new line in the service file. We can make that
more clear.
> (8).
>
> re. Table 6.3 FactoryRegistry Methods
>
> public Iterator FactoryRegistry.iterator()
> Iterator<ImageFactory> or List?
>
> Good question. Supose it should be List for consistency
> (9).
>
> public ImageFactory getFactories()
>
> should be:
>
> public ImageFactory[] getFactories()
>
>
Agreed
>
> (10).
>
> FactoryRegistry.getFactories()
>
> Returns the factories in the registry as an array
>
> Copies?
>
> (I presume Andrew means that the array is mutable without ill effect)
>
> Thats a good question. We should make it so if its not now.
> (11).
>
> public Image getImage(File file)
>
> Throws
> IllegalArgumentException if file is null
>
> Why not throw NPE?
>
> Don't like NPEs - they can happen unexpectedly. Throwing a IAE makes it
deliberate. Thats what its for,
(12).
>
> List<ImageFactory> sniff file type!
>
> get suggested factories for a file or files:
>
> / getFactories(File file)
> \ getFactories(File file, File file)
>
> I think that's too specific - if 2 files why not 3 or 4... If its a list
of files what happens if multiple factories can handle one or more of the
files?
package javax.tools.diagnostics.image
> ======================================
> Page 27
> =======
>
> (13).
> Choose a better order to list the classes.
>
> e.g. ImageFactory, Image, ImageAddressSpace, ImagePointer, ImageSection
>
May be able to do that - at the mercy of the doc tooling I suspect.
>
> (14).
>
> Table 6.4. Interface Summary
>
> Image - This class represents an entire operating system image (e.g. ??
>
> The text after "(e.g." is completely missing.
>
> Understood
> (15).
>
> Table 6.5. Class Summary
>
> DataUnavailable should be DataUnavailableException
>
>
> DU extends DUE?
>
> Page 28
> =======
>
> (16).
>
> interface ImageModule
> --> running header
>
> (I guess Andrew means that the header for the page should have the name of
> the interface.
>
> Understood
> Table 6.6
> =========
> (17).
> public List getSymbols()
>
> should be
>
> public List<ImageSymbol> getSymbols()
>
> (The API has generics added now to the Image API. However, the document
> generation needs to be modified.
> I won't add any further comments about it.)
>
> Agreed though
> Page 29
> =======
>
> Table 6.7. ImageMethods
>
> getAddressSpaces()
>
> Also segmented arch?
>
> Can you explain some more - I can't guess what you mean.
> Page 30
> =======
>
> getProcessorCount()
>
> Returns the number of CPUs running in the system on which the image was
> running.
>
> System = partition?
>
> (I guess Andrew is suggesting "System" should be described as a "partition"
> in a virtual machine, z/Arch LPAR, pSeries LPAR, etc.?)
>
>
> I don't want too much big iron terminology here. The end user is mostly
going to be intel or equivilent based. Partition is something on a disk to
them. (Or maybe I'm just getting old)
Any public proof of the best common terminology to use would be welcome.
> I'll continue from page 31 soon.
>
>
> Regards,
> Stuart
>
>
> --
> Stuart Monteith
> http://blog.stoo.me.uk/
>
>
--
Steve