incubator-kato-spec mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steve Poole <spoole...@googlemail.com>
Subject Re: Andrew Johnson's comments
Date Thu, 10 Dec 2009 23:04:43 GMT
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 <stukato@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message