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 "Bobrovsky, Konstantin S" <ko...@intel.com> on 2009/11/25 13:55:47 UTC

RE: Apache Kato release - comments please

Hi Stuart, all

As we are approaching the end of the public review, I once again reviewed the specification, the results are below. These are some my older comments which have not yet been anyhow resolved plus few new. I could volunteer to develop some of the suggestions further and prepare a complete source updates for the parts which I suggest to change. BTW, I also entered these comments into the JCP comments page for this JSR:
http://wiki.jcp.org/boards/index.php?t=2852

Thanks,
Konst
 
Intel Novosibirsk
Closed Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park, 
17 Krylatskaya Str., Bldg 4, Moscow 121614, 
Russian Federation

=== interface ImageSymbol ===
1)
Add 'public int getSize()' method which would return a number of bytes occupied by the symbol in the image.

=== interface ImageStackFrame ===
1)
API also needs to support 'split' stack frames: e.g. on Itanium every frame consists of a traditinal memory stack frame and additionally, a register stack frame where Register Stack Engine (RSE) spills current register frame during a call (and restores from upon return). The easiest way probably is to mention in the spec something like "implementations can extend this interface to provide data specific to a particular platform, such as register stack frame location on Itanium (r), or frame size").

=== interface JavaField ===
1)
During VM crash debugging it is sometimes important to know the offset of a particular field from object start. So, something like
	/** @return for instance fields, offset of the field from object's beginning in
* the memory; -1 if unknown or for static fields */
public int getOffset()


=== interface JavaLocation ===
1)
The spec says that it "Represents a point of execution within a Java method", however it does not provide access to bytecode index of currently executed Java instruction, which is the only available (sensible) current Java code position description for interpreted frames, and an invaluable complement for compiled method's current code address. So, I would suggest to also have
	/** @return bytecode index corresponding to position in the code represented by
* this JavaLocation instance */
	public int getBytecodeIndex()

2)
getCompilationLevel() suits more to the piece of code which encloses this JavaLocation, and does not seem to fit logically well here

3)
JavaMethod getMethod() does not provide easy way to identify piece of code this location belongs to. Another method
	/** @return a description of a logical chunk of code this location belongs to */
	public CodeStub getContainingCodeStub()
would achieve this. Additionally, I think that a "piece of generated code" is important concept within any JVM to dispense a specific interface for it - CodeStub. 

  interface CodeStub {
    String getName();
    ImageSection getCode();
  }
CodeStub which a compiled method would extend from it:

CompiledMethodCode extends CodeStub {
	JavaMethod getMethod();
	int getCompilationLevel();
	/** @return (for OSR compilations - bytecode index of the entry point into
 * this method) */
	int getEntryPointBytecodeIndex();
	// plus maybe GC maps (all implementaions) and other implementation-specific info
}

=== interface JavaMethod ===
1)
	/** Get the set of ImageSections containing the compiled code of this method. */
List getCompiledSections()
          

I would suggest to change to 
	/** Get the set of CompiledMethodCode objects containing the compiled code of
 *  this method. */
List getCompiledSections()


=== interface JavaStackFrame ===
1)
Usually, Java stack frames contain information about Java monitors held in this frame. This is necessary because Java spec requires "forced" release of all monitors held by a method in case it "completes abruptly". So something like
	/** @return list of JavaObject instances representing monitors held by
 * this method activation */
List getHeldMonitors()
would be really useful.

2)
Important case of JavaStackFrame is interpreted frames - in this case current position in the machine code is not that important (this position is not unique to a method) as current position in the bytecode being executed. So it would be good to distinguish compiled frames from interpreted ones (IMO, this is implementable for any JVM)
	public boolean isCompiled();
	public boolean isInterpreted();
In case of interpreted frames, more information might be available.

3)
It would be of great help to an engineer investigating a crash in compiled code or some Java-level runtime error to know the JVM virtual state (in JVMS sense) for all Java stack frames, i.e.:
-	held monitors
-	current bytecode index
-	values of all local variables
-	content of the expression stack

and some JVMs can provide such info even for compiled frames. I think it is desirable to add something like (removing getHeldMonitors):

	public JVMState getCurrentJVMState();


=== interface JavaReference ===
1)
HEAP_ROOT_*, REFERENCE_* and other constants are omitted from the PDF document 

2)
This API defines heap roots returned by getHeapRoots queries (e.g. JavaStackFrame.getHeapRoots) to be JavaReferences. Sometimes it is very helpful to know where the heap root actually resides - the corresponding CPU register and/or memory address (in a memory stack frame or register stack frame), and this info is usually available in Java runtimes. Something like

    /** @return a CPU register name where the reference's target is located or
     * null if such information is not available */
    public ImageRegister getRegister();

    /** @return  a location within this image where the reference's target is
     * located or null if such information is not available */
    public ImagePointer.getImageLocation();

would be helpful.

=== interface JavaRuntime ===

1) It should be possible in the API to iterate trough generated code sequences other than managed methods - so-called 'stubs'. Each stub is similar to a method: it has a name, (possibly, multiple) code sections. This interface (JavaRuntime) seems an appropriate place for this API:
	/** @return  a list of CodeStub objects representing code sequences
 * generated internally by the JVM */
	public List getJVMCodeStubs();

(2) Not all methods within a Java runtime are necessarily compiled. Many may remain interpreted, others might not even have a bytecode. So I would suggest to replace 'getCompiledMethods' with
	/** @return a list of JavaMethod objects representing managed methods known
 * to the system. Each JavaMethod may be either interpreted or compiled or
   native. Method's code for compiled methods can be quired via @see
  #JavaMethod.getCompiledSections() */
public List getJavaMethods();

=== Common Register Names ===
1) 
'Long' or 'Integer' is too vague for specifying register type. It is better to specify the width of the register - 256 (future Intel architectures) / 128 (SSE registers) / 80 (FP registers in ia32, Intel64, IA64)/ 64 / 32 bit - plus an additional characteristic such as 'general purpose/floating point/application/flags/...'

2)
The table title "Table A.2. IA64 Register Names" is wrong, it should read either "Intel64 Register Names" or "AMD64 Register Names", as "IA64" is Itanium architecture.

3)
For the complete list of recommended register names and their specifications for ia32 and Intel64 architectures I suggest add a link to

Intel(r) 64 and IA-32 Architectures Software Developer's Manual (September 2009), Volume 1:Basic Architecture, Sections "3.4 BASIC PROGRAM EXECUTION REGISTERS" and "3.5 INSTRUCTION POINTER"
http://www.intel.com/Assets/PDF/manual/253665.pdf

4)
I suggest to add a new section "IA64 Register Names" with a link to the official Itanium(r) 2 specification for the recommended IA64 register names and their specifications, which is

Intel(r) Itanium(r) Architecture Software Developer's Manual (rev 2.2), Volume 1: Application Architecture, Section 3.1 "Application Register State"
http://download.intel.com/design/Itanium/manuals/24531705.pdf


>-----Original Message-----
>From: Stuart Monteith [mailto:stukato@stoo.me.uk]
>Sent: Friday, November 20, 2009 11:29 PM
>To: kato-dev@incubator.apache.org
>Subject: Apache Kato release - comments please
>
>Hello all,
>     Steve and I have been working on getting the codebase ready for a
>release. This is a non-trivial activity with lots of things to get right.
>
>Given the state of the project as it is just now, we can't claim to be
>doing a 1.0 release, instead I'm following the Milestone convention.
>
>I'm not proposing that what I've assembled is submitted as a release
>candidate just yet, as there is still work to do on it.
>
>If you look here, you'll find the files that would make up a release:
>     http://people.apache.org/~monteith/kato/apache-kato-M1-incubating-RC1/
>
>I've also created a tag:
>
>https://svn.apache.org/repos/asf/incubator/kato/tags/apache-kato-M1-
>incubating-RC1/
>
>
>Obvious things that are wrong:
>     apache-kato-M1-incubating-tomcat.demos - packages are undocumented
>and missing some files.
>     The documentation - it needs more work, but I'd be interested in
>some cursory feedback.
>     The CJVMTI agent needs work under Linux - it's missing some
>information.
>     The TCK is still under development.
>     Some filtering needs to be applied to the rat reports.
>     There are some spurious files, like readme.html.
>
>I have found this a very useful exercise, as this is the first that
>we've packaged the projects contents and seen what it actually includes,
>so this should give everyone a good opportunity to have a look and see.
>
>
>Regards,
>     Stuart
>
>--
>Stuart Monteith
>http://blog.stoo.me.uk/


Re: Apache Kato release - comments please

Posted by Steve Poole <sp...@googlemail.com>.
Konst -  thanks for your review.

I'll go through what you've written over the weekend and post a detailed
reply here on this mailing list and post another reply to your JCP comments
email pointing at it.


On Wed, Nov 25, 2009 at 1:55 PM, Bobrovsky, Konstantin S <
konstantin.s.bobrovsky@intel.com> wrote:

> Hi Stuart, all
>
> As we are approaching the end of the public review, I once again reviewed
> the specification, the results are below. These are some my older comments
> which have not yet been anyhow resolved plus few new. I could volunteer to
> develop some of the suggestions further and prepare a complete source
> updates for the parts which I suggest to change. BTW, I also entered these
> comments into the JCP comments page for this JSR:
> http://wiki.jcp.org/boards/index.php?t=2852
>
> Thanks,
> Konst
>
> Intel Novosibirsk
> Closed Joint Stock Company Intel A/O
> Registered legal address: Krylatsky Hills Business Park,
> 17 Krylatskaya Str., Bldg 4, Moscow 121614,
> Russian Federation
>
> === interface ImageSymbol ===
> 1)
> Add 'public int getSize()' method which would return a number of bytes
> occupied by the symbol in the image.
>
> === interface ImageStackFrame ===
> 1)
> API also needs to support 'split' stack frames: e.g. on Itanium every frame
> consists of a traditinal memory stack frame and additionally, a register
> stack frame where Register Stack Engine (RSE) spills current register frame
> during a call (and restores from upon return). The easiest way probably is
> to mention in the spec something like "implementations can extend this
> interface to provide data specific to a particular platform, such as
> register stack frame location on Itanium (r), or frame size").
>
> === interface JavaField ===
> 1)
> During VM crash debugging it is sometimes important to know the offset of a
> particular field from object start. So, something like
>        /** @return for instance fields, offset of the field from object's
> beginning in
> * the memory; -1 if unknown or for static fields */
> public int getOffset()
>
>
> === interface JavaLocation ===
> 1)
> The spec says that it "Represents a point of execution within a Java
> method", however it does not provide access to bytecode index of currently
> executed Java instruction, which is the only available (sensible) current
> Java code position description for interpreted frames, and an invaluable
> complement for compiled method's current code address. So, I would suggest
> to also have
>        /** @return bytecode index corresponding to position in the code
> represented by
> * this JavaLocation instance */
>        public int getBytecodeIndex()
>
> 2)
> getCompilationLevel() suits more to the piece of code which encloses this
> JavaLocation, and does not seem to fit logically well here
>
> 3)
> JavaMethod getMethod() does not provide easy way to identify piece of code
> this location belongs to. Another method
>        /** @return a description of a logical chunk of code this location
> belongs to */
>        public CodeStub getContainingCodeStub()
> would achieve this. Additionally, I think that a "piece of generated code"
> is important concept within any JVM to dispense a specific interface for it
> - CodeStub.
>
>  interface CodeStub {
>    String getName();
>    ImageSection getCode();
>  }
> CodeStub which a compiled method would extend from it:
>
> CompiledMethodCode extends CodeStub {
>        JavaMethod getMethod();
>        int getCompilationLevel();
>        /** @return (for OSR compilations - bytecode index of the entry
> point into
>  * this method) */
>        int getEntryPointBytecodeIndex();
>        // plus maybe GC maps (all implementaions) and other
> implementation-specific info
> }
>
> === interface JavaMethod ===
> 1)
>        /** Get the set of ImageSections containing the compiled code of
> this method. */
> List getCompiledSections()
>
>
> I would suggest to change to
>        /** Get the set of CompiledMethodCode objects containing the
> compiled code of
>  *  this method. */
> List getCompiledSections()
>
>
> === interface JavaStackFrame ===
> 1)
> Usually, Java stack frames contain information about Java monitors held in
> this frame. This is necessary because Java spec requires "forced" release of
> all monitors held by a method in case it "completes abruptly". So something
> like
>        /** @return list of JavaObject instances representing monitors held
> by
>  * this method activation */
> List getHeldMonitors()
> would be really useful.
>
> 2)
> Important case of JavaStackFrame is interpreted frames - in this case
> current position in the machine code is not that important (this position is
> not unique to a method) as current position in the bytecode being executed.
> So it would be good to distinguish compiled frames from interpreted ones
> (IMO, this is implementable for any JVM)
>        public boolean isCompiled();
>        public boolean isInterpreted();
> In case of interpreted frames, more information might be available.
>
> 3)
> It would be of great help to an engineer investigating a crash in compiled
> code or some Java-level runtime error to know the JVM virtual state (in JVMS
> sense) for all Java stack frames, i.e.:
> -       held monitors
> -       current bytecode index
> -       values of all local variables
> -       content of the expression stack
>
> and some JVMs can provide such info even for compiled frames. I think it is
> desirable to add something like (removing getHeldMonitors):
>
>        public JVMState getCurrentJVMState();
>
>
> === interface JavaReference ===
> 1)
> HEAP_ROOT_*, REFERENCE_* and other constants are omitted from the PDF
> document
>
> 2)
> This API defines heap roots returned by getHeapRoots queries (e.g.
> JavaStackFrame.getHeapRoots) to be JavaReferences. Sometimes it is very
> helpful to know where the heap root actually resides - the corresponding CPU
> register and/or memory address (in a memory stack frame or register stack
> frame), and this info is usually available in Java runtimes. Something like
>
>    /** @return a CPU register name where the reference's target is located
> or
>     * null if such information is not available */
>    public ImageRegister getRegister();
>
>    /** @return  a location within this image where the reference's target
> is
>     * located or null if such information is not available */
>    public ImagePointer.getImageLocation();
>
> would be helpful.
>
> === interface JavaRuntime ===
>
> 1) It should be possible in the API to iterate trough generated code
> sequences other than managed methods - so-called 'stubs'. Each stub is
> similar to a method: it has a name, (possibly, multiple) code sections. This
> interface (JavaRuntime) seems an appropriate place for this API:
>        /** @return  a list of CodeStub objects representing code sequences
>  * generated internally by the JVM */
>        public List getJVMCodeStubs();
>
> (2) Not all methods within a Java runtime are necessarily compiled. Many
> may remain interpreted, others might not even have a bytecode. So I would
> suggest to replace 'getCompiledMethods' with
>        /** @return a list of JavaMethod objects representing managed
> methods known
>  * to the system. Each JavaMethod may be either interpreted or compiled or
>   native. Method's code for compiled methods can be quired via @see
>  #JavaMethod.getCompiledSections() */
> public List getJavaMethods();
>
> === Common Register Names ===
> 1)
> 'Long' or 'Integer' is too vague for specifying register type. It is better
> to specify the width of the register - 256 (future Intel architectures) /
> 128 (SSE registers) / 80 (FP registers in ia32, Intel64, IA64)/ 64 / 32 bit
> - plus an additional characteristic such as 'general purpose/floating
> point/application/flags/...'
>
> 2)
> The table title "Table A.2. IA64 Register Names" is wrong, it should read
> either "Intel64 Register Names" or "AMD64 Register Names", as "IA64" is
> Itanium architecture.
>
> 3)
> For the complete list of recommended register names and their
> specifications for ia32 and Intel64 architectures I suggest add a link to
>
> Intel(r) 64 and IA-32 Architectures Software Developer's Manual (September
> 2009), Volume 1:Basic Architecture, Sections "3.4 BASIC PROGRAM EXECUTION
> REGISTERS" and "3.5 INSTRUCTION POINTER"
> http://www.intel.com/Assets/PDF/manual/253665.pdf
>
> 4)
> I suggest to add a new section "IA64 Register Names" with a link to the
> official Itanium(r) 2 specification for the recommended IA64 register names
> and their specifications, which is
>
> Intel(r) Itanium(r) Architecture Software Developer's Manual (rev 2.2),
> Volume 1: Application Architecture, Section 3.1 "Application Register State"
> http://download.intel.com/design/Itanium/manuals/24531705.pdf
>
>
> >-----Original Message-----
> >From: Stuart Monteith [mailto:stukato@stoo.me.uk]
> >Sent: Friday, November 20, 2009 11:29 PM
> >To: kato-dev@incubator.apache.org
> >Subject: Apache Kato release - comments please
> >
> >Hello all,
> >     Steve and I have been working on getting the codebase ready for a
> >release. This is a non-trivial activity with lots of things to get right.
> >
> >Given the state of the project as it is just now, we can't claim to be
> >doing a 1.0 release, instead I'm following the Milestone convention.
> >
> >I'm not proposing that what I've assembled is submitted as a release
> >candidate just yet, as there is still work to do on it.
> >
> >If you look here, you'll find the files that would make up a release:
> >
> http://people.apache.org/~monteith/kato/apache-kato-M1-incubating-RC1/<http://people.apache.org/%7Emonteith/kato/apache-kato-M1-incubating-RC1/>
> >
> >I've also created a tag:
> >
> >https://svn.apache.org/repos/asf/incubator/kato/tags/apache-kato-M1-
> >incubating-RC1/
> >
> >
> >Obvious things that are wrong:
> >     apache-kato-M1-incubating-tomcat.demos - packages are undocumented
> >and missing some files.
> >     The documentation - it needs more work, but I'd be interested in
> >some cursory feedback.
> >     The CJVMTI agent needs work under Linux - it's missing some
> >information.
> >     The TCK is still under development.
> >     Some filtering needs to be applied to the rat reports.
> >     There are some spurious files, like readme.html.
> >
> >I have found this a very useful exercise, as this is the first that
> >we've packaged the projects contents and seen what it actually includes,
> >so this should give everyone a good opportunity to have a look and see.
> >
> >
> >Regards,
> >     Stuart
> >
> >--
> >Stuart Monteith
> >http://blog.stoo.me.uk/
>
>


-- 
Steve

Re: Apache Kato release - comments please

Posted by Steve Poole <sp...@googlemail.com>.
On Wed, Dec 2, 2009 at 5:49 AM, Bobrovsky, Konstantin S <
konstantin.s.bobrovsky@intel.com> wrote:

> Steve,
>
> My comments follow.
>
> Thanks,
> Konst
>
> Intel Novosibirsk
> Closed Joint Stock Company Intel A/O
> Registered legal address: Krylatsky Hills Business Park,
> 17 Krylatskaya Str., Bldg 4, Moscow 121614,
> Russian Federation
>
>
> >> === interface JavaField ===
> >> 1)
> >> During VM crash debugging it is sometimes important to know the offset
> of
> >a
> >> particular field from object start. So, something like
> >>        /** @return for instance fields, offset of the field from
> object's
> >> beginning in
> >> * the memory; -1 if unknown or for static fields */
> >> public int getOffset()
> >>
> >> This makes sense too -  though I think it would be better to  have
> >something like this instead :
> >
> >public ImagePointer getOffset(JavaObject )
> >
> >Gives you more flexibility in the implementation and returns something
> >you're going to need anyway.   The ImagePointer can then be used to look
> at
> >the contents of the object directly.
>
> I'm not quite comfortable with ImagePointer representing an offset of a
> JavaField. 'Offset' is a number of bytes between object start and field
> reference location within the object, whereas ImpagePointer is an absolute
> address within the image. Also, 'offset' is independent entity from the
> process image concept, and can be used without image API. On the other hand,
> unlike Hotspot JVM, the offset of given field can be different in objects of
> different types (subclasses of the class where the field is declared), so I
> like the addition of the JavaObject parameter.
>
> Understood, so  it becomes
 public int getOffset(JavaObject )

>> === interface JavaLocation ===
> ...
> >>        /** @return bytecode index corresponding to position in the code
> >> represented by
> >> * this JavaLocation instance */
> >>        public int getBytecodeIndex()
> >>
> >> Agreed -  perhaps its should be called  getProgramCounter()  or
> something
> >similar?
>
> Sounds good to me.
>
> >> Konst - I'm not sure I really understand the benefit of this item over
> >what
> >already exists.  Could you start a separate thread and provide some more
> >info about sort of usecase you had in mind?
>
> OK, I'll do that.
>
> >> === interface JavaStackFrame ===
> ...
> >> So it would be good to distinguish compiled frames from interpreted ones
> >> (IMO, this is implementable for any JVM)
> >>        public boolean isCompiled();
> >>        public boolean isInterpreted();
> >> In case of interpreted frames, more information might be available.
> >>
> >> Thats a good idea. Do we need both?   What about levels of compilation.
> >You suggested moving getCompilationLevel() to here so can't we use that
> >with
> >perhaps 0 = interpreted?
>
> I suggested to move getCompilationLevel() to a new CompiledMethodCode (or
> something like it), as JavaStackFrame does not have a 'compilation level' -
> it is the code which does. To be strict, 'isCompiled/isInterpreted' is not
> quite accurate too, something like
> 'belongsToCompiledMethod/belongsToInterpretedMethod' better reflects the
> sense, but is pretty awkward.
>
> I agree that JavaMethod is both static and dynamic.  We do need to find
some sort of separation.   Not sure how yet though :-)

>> 2)
> >> This API defines heap roots returned by getHeapRoots queries (e.g.
> >> JavaStackFrame.getHeapRoots) to be JavaReferences. Sometimes it is very
> >> helpful to know where the heap root actually resides - the corresponding
> >CPU
> >> register and/or memory address (in a memory stack frame or register
> stack
> >> frame), and this info is usually available in Java runtimes. Something
> >like
> >>
> >>    /** @return a CPU register name where the reference's target is
> >located
> >> or
> >>     * null if such information is not available */
> >>    public ImageRegister getRegister();
> >>
> >> I understand why you want this  - is it only restricted to references?
> >Are there other places where knowing which register was used is important?
>
> Yes, there are. What comes to mind is:
> - calling convention (in a broad sense) for compiled and interpreted
> managed methods. I.e. where input/output parameters or return value reside
> (which register or stack location) + (the "broad" part - which is entirely
> VM-specific) where additional information is kept, such as current thread
> pointer, method object pointer, stack pointer, etc.
> - calling convention for native methods and VM functions
>
> In some cases knowing these details helps crash analysis.
>
> There are several other places I can think of, but they are very VM
> specific and should probably not be a part of the API.
>
> BTW, "calling convention" is a part of so-called "Application Binary
> Interface" which is unique to a given platform (CPU+OS combination). There
> are other parts in a usual ABI which might be worth abstracting in the Kato
> API
>        - various alignments (code entries, values of various data types,
> method's stack frame). Misalignments are usually a good sign of an error. To
> develop this little further, a JavaRuntime created from a process image dump
> could have a "verify" method which would check all known "invariants" -
> alignments, argument types, range-checks for objects (must be in a heap),
> code pieces (must be in some "code cache"), etc.
>        - ...
>
> >
> >   /** @return  a location within this image where the reference's target
> >is
> >>     * located or null if such information is not available */
> >>    public ImagePointer.getImageLocation();
> >>
> >> Agree with this idea  -  I would probably go with something a little
> more
> >generic though :
> >
> >     public ImagePointer getAddress();
> >
>
> Yes, sounds good.
>
> >-----Original Message-----
> >From: Steve Poole [mailto:spoole167@googlemail.com]
> >Sent: Monday, November 30, 2009 5:32 PM
> >To: kato-spec@incubator.apache.org
> >Subject: Re: Apache Kato release - comments please
> >
> >Konst - by replies to you comments below.
> >
> >
> >On Wed, Nov 25, 2009 at 1:55 PM, Bobrovsky, Konstantin S <
> >konstantin.s.bobrovsky@intel.com> wrote:
> >
> >> Hi Stuart, all
> >>
> >> As we are approaching the end of the public review, I once again
> reviewed
> >> the specification, the results are below. These are some my older
> >comments
> >> which have not yet been anyhow resolved plus few new. I could volunteer
> >to
> >> develop some of the suggestions further and prepare a complete source
> >> updates for the parts which I suggest to change. BTW, I also entered
> >these
> >> comments into the JCP comments page for this JSR:
> >> http://wiki.jcp.org/boards/index.php?t=2852
> >>
> >> Thanks,
> >> Konst
> >>
> >> Intel Novosibirsk
> >> Closed Joint Stock Company Intel A/O
> >> Registered legal address: Krylatsky Hills Business Park,
> >> 17 Krylatskaya Str., Bldg 4, Moscow 121614,
> >> Russian Federation
> >>
> >> === interface ImageSymbol ===
> >> 1)
> >> Add 'public int getSize()' method which would return a number of bytes
> >> occupied by the symbol in the image.
> >>
> >> That does make sense -  perhaps we should also consider adding in a
> >getType()  method ?
> >
> >=== interface ImageStackFrame ===
> >> 1)
> >> API also needs to support 'split' stack frames: e.g. on Itanium every
> >frame
> >> consists of a traditinal memory stack frame and additionally, a register
> >> stack frame where Register Stack Engine (RSE) spills current register
> >frame
> >> during a call (and restores from upon return). The easiest way probably
> >is
> >> to mention in the spec something like "implementations can extend this
> >> interface to provide data specific to a particular platform, such as
> >> register stack frame location on Itanium (r), or frame size").
> >>
> >> Thanks - you've given as an easy way out :-)
> >
> >
> >> === interface JavaField ===
> >> 1)
> >> During VM crash debugging it is sometimes important to know the offset
> of
> >a
> >> particular field from object start. So, something like
> >>        /** @return for instance fields, offset of the field from
> object's
> >> beginning in
> >> * the memory; -1 if unknown or for static fields */
> >> public int getOffset()
> >>
> >> This makes sense too -  though I think it would be better to  have
> >something like this instead :
> >
> >public ImagePointer getOffset(JavaObject )
> >
> >Gives you more flexibility in the implementation and returns something
> >you're going to need anyway.   The ImagePointer can then be used to look
> at
> >the contents of the object directly.
> >
> >
> >> === interface JavaLocation ===
> >> 1)
> >> The spec says that it "Represents a point of execution within a Java
> >> method", however it does not provide access to bytecode index of
> >currently
> >> executed Java instruction, which is the only available (sensible)
> current
> >> Java code position description for interpreted frames, and an invaluable
> >> complement for compiled method's current code address. So, I would
> >suggest
> >> to also have
> >>        /** @return bytecode index corresponding to position in the code
> >> represented by
> >> * this JavaLocation instance */
> >>        public int getBytecodeIndex()
> >>
> >> Agreed -  perhaps its should be called  getProgramCounter()  or
> something
> >similar?
> >
> >2)
> >> getCompilationLevel() suits more to the piece of code which encloses
> this
> >> JavaLocation, and does not seem to fit logically well here
> >>
> >> Agreed -  see comments later about this.
> >
> >
> >> 3)
> >> JavaMethod getMethod() does not provide easy way to identify piece of
> >code
> >> this location belongs to. Another method
> >>        /** @return a description of a logical chunk of code this
> location
> >> belongs to */
> >>        public CodeStub getContainingCodeStub()
> >> would achieve this. Additionally, I think that a "piece of generated
> >code"
> >> is important concept within any JVM to dispense a specific interface for
> >it
> >> - CodeStub.
> >>
> >>  interface CodeStub {
> >>    String getName();
> >>    ImageSection getCode();
> >>  }
> >> CodeStub which a compiled method would extend from it:
> >>
> >> CompiledMethodCode extends CodeStub {
> >>        JavaMethod getMethod();
> >>        int getCompilationLevel();
> >>        /** @return (for OSR compilations - bytecode index of the entry
> >> point into
> >>  * this method) */
> >>        int getEntryPointBytecodeIndex();
> >>        // plus maybe GC maps (all implementaions) and other
> >> implementation-specific info
> >> }
> >>
> >> Konst - I'm not sure I really understand the benefit of this item over
> >what
> >already exists.  Could you start a separate thread and provide some more
> >info about sort of usecase you had in mind?
> >
> >
> >> === interface JavaMethod ===
> >> 1)
> >>        /** Get the set of ImageSections containing the compiled code of
> >> this method. */
> >> List getCompiledSections()
> >>
> >>
> >> I would suggest to change to
> >>        /** Get the set of CompiledMethodCode objects containing the
> >> compiled code of
> >>  *  this method. */
> >> List getCompiledSections()
> >>
> >> Understood -  this is tied up with the item above so we can resolve them
> >> together.
> >>
> >
> >
> >> === interface JavaStackFrame ===
> >> 1)
> >> Usually, Java stack frames contain information about Java monitors held
> >in
> >> this frame. This is necessary because Java spec requires "forced"
> release
> >of
> >> all monitors held by a method in case it "completes abruptly". So
> >something
> >> like
> >>        /** @return list of JavaObject instances representing monitors
> >held
> >> by
> >>  * this method activation */
> >> List getHeldMonitors()
> >> would be really useful.
> >>
> >> Agreed - this would be very useful.
> >
> >
> >> 2)
> >> Important case of JavaStackFrame is interpreted frames - in this case
> >> current position in the machine code is not that important (this
> position
> >is
> >> not unique to a method) as current position in the bytecode being
> >executed.
> >> So it would be good to distinguish compiled frames from interpreted ones
> >> (IMO, this is implementable for any JVM)
> >>        public boolean isCompiled();
> >>        public boolean isInterpreted();
> >> In case of interpreted frames, more information might be available.
> >>
> >> Thats a good idea. Do we need both?   What about levels of compilation.
> >You suggested moving getCompilationLevel() to here so can't we use that
> >with
> >perhaps 0 = interpreted?
> >
> >
> >
> >> 3)
> >> It would be of great help to an engineer investigating a crash in
> >compiled
> >> code or some Java-level runtime error to know the JVM virtual state (in
> >JVMS
> >> sense) for all Java stack frames, i.e.:
> >> -       held monitors
> >> -       current bytecode index
> >> -       values of all local variables
> >> -       content of the expression stack
> >>
> >> and some JVMs can provide such info even for compiled frames. I think it
> >is
> >> desirable to add something like (removing getHeldMonitors):
> >>
> >>        public JVMState getCurrentJVMState();
> >>
> >>
> >> === interface JavaReference ===
> >> 1)
> >> HEAP_ROOT_*, REFERENCE_* and other constants are omitted from the PDF
> >> document
> >>
> >> ok - thanks for spotting that.
> >
> >
> >> 2)
> >> This API defines heap roots returned by getHeapRoots queries (e.g.
> >> JavaStackFrame.getHeapRoots) to be JavaReferences. Sometimes it is very
> >> helpful to know where the heap root actually resides - the corresponding
> >CPU
> >> register and/or memory address (in a memory stack frame or register
> stack
> >> frame), and this info is usually available in Java runtimes. Something
> >like
> >>
> >>    /** @return a CPU register name where the reference's target is
> >located
> >> or
> >>     * null if such information is not available */
> >>    public ImageRegister getRegister();
> >>
> >> I understand why you want this  - is it only restricted to references?
> >Are there other places where knowing which register was used is important?
> >
> >   /** @return  a location within this image where the reference's target
> >is
> >>     * located or null if such information is not available */
> >>    public ImagePointer.getImageLocation();
> >>
> >> Agree with this idea  -  I would probably go with something a little
> more
> >generic though :
> >
> >     public ImagePointer getAddress();
> >
> >
> >
> >> would be helpful.
> >>
> >> === interface JavaRuntime ===
> >>
> >> 1) It should be possible in the API to iterate trough generated code
> >> sequences other than managed methods - so-called 'stubs'. Each stub is
> >> similar to a method: it has a name, (possibly, multiple) code sections.
> >This
> >> interface (JavaRuntime) seems an appropriate place for this API:
> >>        /** @return  a list of CodeStub objects representing code
> >sequences
> >>  * generated internally by the JVM */
> >>        public List getJVMCodeStubs();
> >>
> >> (2) Not all methods within a Java runtime are necessarily compiled. Many
> >> may remain interpreted, others might not even have a bytecode. So I
> would
> >> suggest to replace 'getCompiledMethods' with
> >>        /** @return a list of JavaMethod objects representing managed
> >> methods known
> >>  * to the system. Each JavaMethod may be either interpreted or compiled
> >or
> >>   native. Method's code for compiled methods can be quired via @see
> >>  #JavaMethod.getCompiledSections() */
> >> public List getJavaMethods();
> >>
> >> === Common Register Names ===
> >> 1)
> >> 'Long' or 'Integer' is too vague for specifying register type. It is
> >better
> >> to specify the width of the register - 256 (future Intel architectures)
> /
> >> 128 (SSE registers) / 80 (FP registers in ia32, Intel64, IA64)/ 64 / 32
> >bit
> >> - plus an additional characteristic such as 'general purpose/floating
> >> point/application/flags/...'
> >>
> >> Understood
> >
> >
> >> 2)
> >> The table title "Table A.2. IA64 Register Names" is wrong, it should
> read
> >> either "Intel64 Register Names" or "AMD64 Register Names", as "IA64" is
> >> Itanium architecture.
> >>
> >> Understood
> >
> >> 3)
> >> For the complete list of recommended register names and their
> >> specifications for ia32 and Intel64 architectures I suggest add a link
> to
> >>
> >> Intel(r) 64 and IA-32 Architectures Software Developer's Manual
> >(September
> >> 2009), Volume 1:Basic Architecture, Sections "3.4 BASIC PROGRAM
> EXECUTION
> >> REGISTERS" and "3.5 INSTRUCTION POINTER"
> >> http://www.intel.com/Assets/PDF/manual/253665.pdf
> >>
> >> Agreed
> >
> >> 4)
> >> I suggest to add a new section "IA64 Register Names" with a link to the
> >> official Itanium(r) 2 specification for the recommended IA64 register
> >names
> >> and their specifications, which is
> >>
> >> Intel(r) Itanium(r) Architecture Software Developer's Manual (rev 2.2),
> >> Volume 1: Application Architecture, Section 3.1 "Application Register
> >State"
> >> http://download.intel.com/design/Itanium/manuals/24531705.pdf
> >>
> >> Agreed
> >>
> >
> >
> >> >-----Original Message-----
> >> >From: Stuart Monteith [mailto:stukato@stoo.me.uk]
> >> >Sent: Friday, November 20, 2009 11:29 PM
> >> >To: kato-dev@incubator.apache.org
> >> >Subject: Apache Kato release - comments please
> >> >
> >> >Hello all,
> >> >     Steve and I have been working on getting the codebase ready for a
> >> >release. This is a non-trivial activity with lots of things to get
> >right.
> >> >
> >> >Given the state of the project as it is just now, we can't claim to be
> >> >doing a 1.0 release, instead I'm following the Milestone convention.
> >> >
> >> >I'm not proposing that what I've assembled is submitted as a release
> >> >candidate just yet, as there is still work to do on it.
> >> >
> >> >If you look here, you'll find the files that would make up a release:
> >> >
> >> http://people.apache.org/~monteith/kato/apache-kato-M1-incubating-<http://people.apache.org/%7Emonteith/kato/apache-kato-M1-incubating->
> >RC1/<http://people.apache.org/%7Emonteith/kato/apache-kato-M1-incubating-
> >RC1/>
> >> >
> >> >I've also created a tag:
> >> >
> >> >https://svn.apache.org/repos/asf/incubator/kato/tags/apache-kato-M1-
> >> >incubating-RC1/
> >> >
> >> >
> >> >Obvious things that are wrong:
> >> >     apache-kato-M1-incubating-tomcat.demos - packages are undocumented
> >> >and missing some files.
> >> >     The documentation - it needs more work, but I'd be interested in
> >> >some cursory feedback.
> >> >     The CJVMTI agent needs work under Linux - it's missing some
> >> >information.
> >> >     The TCK is still under development.
> >> >     Some filtering needs to be applied to the rat reports.
> >> >     There are some spurious files, like readme.html.
> >> >
> >> >I have found this a very useful exercise, as this is the first that
> >> >we've packaged the projects contents and seen what it actually
> includes,
> >> >so this should give everyone a good opportunity to have a look and see.
> >> >
> >> >
> >> >Regards,
> >> >     Stuart
> >> >
> >> >--
> >> >Stuart Monteith
> >> >http://blog.stoo.me.uk/
> >>
> >>
> >
> >
> >--
> >Steve
>



-- 
Steve

RE: Apache Kato release - comments please

Posted by "Bobrovsky, Konstantin S" <ko...@intel.com>.
Steve,

My comments follow.

Thanks,
Konst

Intel Novosibirsk
Closed Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation


>> === interface JavaField ===
>> 1)
>> During VM crash debugging it is sometimes important to know the offset of
>a
>> particular field from object start. So, something like
>>        /** @return for instance fields, offset of the field from object's
>> beginning in
>> * the memory; -1 if unknown or for static fields */
>> public int getOffset()
>>
>> This makes sense too -  though I think it would be better to  have
>something like this instead :
>
>public ImagePointer getOffset(JavaObject )
>
>Gives you more flexibility in the implementation and returns something
>you're going to need anyway.   The ImagePointer can then be used to look at
>the contents of the object directly.

I'm not quite comfortable with ImagePointer representing an offset of a JavaField. 'Offset' is a number of bytes between object start and field reference location within the object, whereas ImpagePointer is an absolute address within the image. Also, 'offset' is independent entity from the process image concept, and can be used without image API. On the other hand, unlike Hotspot JVM, the offset of given field can be different in objects of different types (subclasses of the class where the field is declared), so I like the addition of the JavaObject parameter.

>> === interface JavaLocation ===
...
>>        /** @return bytecode index corresponding to position in the code
>> represented by
>> * this JavaLocation instance */
>>        public int getBytecodeIndex()
>>
>> Agreed -  perhaps its should be called  getProgramCounter()  or something
>similar?

Sounds good to me.

>> Konst - I'm not sure I really understand the benefit of this item over
>what
>already exists.  Could you start a separate thread and provide some more
>info about sort of usecase you had in mind?

OK, I'll do that.

>> === interface JavaStackFrame ===
...
>> So it would be good to distinguish compiled frames from interpreted ones
>> (IMO, this is implementable for any JVM)
>>        public boolean isCompiled();
>>        public boolean isInterpreted();
>> In case of interpreted frames, more information might be available.
>>
>> Thats a good idea. Do we need both?   What about levels of compilation.
>You suggested moving getCompilationLevel() to here so can't we use that
>with
>perhaps 0 = interpreted?

I suggested to move getCompilationLevel() to a new CompiledMethodCode (or something like it), as JavaStackFrame does not have a 'compilation level' - it is the code which does. To be strict, 'isCompiled/isInterpreted' is not quite accurate too, something like 'belongsToCompiledMethod/belongsToInterpretedMethod' better reflects the sense, but is pretty awkward.

>> 2)
>> This API defines heap roots returned by getHeapRoots queries (e.g.
>> JavaStackFrame.getHeapRoots) to be JavaReferences. Sometimes it is very
>> helpful to know where the heap root actually resides - the corresponding
>CPU
>> register and/or memory address (in a memory stack frame or register stack
>> frame), and this info is usually available in Java runtimes. Something
>like
>>
>>    /** @return a CPU register name where the reference's target is
>located
>> or
>>     * null if such information is not available */
>>    public ImageRegister getRegister();
>>
>> I understand why you want this  - is it only restricted to references?
>Are there other places where knowing which register was used is important?

Yes, there are. What comes to mind is:
- calling convention (in a broad sense) for compiled and interpreted managed methods. I.e. where input/output parameters or return value reside (which register or stack location) + (the "broad" part - which is entirely VM-specific) where additional information is kept, such as current thread pointer, method object pointer, stack pointer, etc.
- calling convention for native methods and VM functions

In some cases knowing these details helps crash analysis.

There are several other places I can think of, but they are very VM specific and should probably not be a part of the API.

BTW, "calling convention" is a part of so-called "Application Binary Interface" which is unique to a given platform (CPU+OS combination). There are other parts in a usual ABI which might be worth abstracting in the Kato API
        - various alignments (code entries, values of various data types, method's stack frame). Misalignments are usually a good sign of an error. To develop this little further, a JavaRuntime created from a process image dump could have a "verify" method which would check all known "invariants" - alignments, argument types, range-checks for objects (must be in a heap), code pieces (must be in some "code cache"), etc.
        - ...

>
>   /** @return  a location within this image where the reference's target
>is
>>     * located or null if such information is not available */
>>    public ImagePointer.getImageLocation();
>>
>> Agree with this idea  -  I would probably go with something a little more
>generic though :
>
>     public ImagePointer getAddress();
>

Yes, sounds good.

>-----Original Message-----
>From: Steve Poole [mailto:spoole167@googlemail.com]
>Sent: Monday, November 30, 2009 5:32 PM
>To: kato-spec@incubator.apache.org
>Subject: Re: Apache Kato release - comments please
>
>Konst - by replies to you comments below.
>
>
>On Wed, Nov 25, 2009 at 1:55 PM, Bobrovsky, Konstantin S <
>konstantin.s.bobrovsky@intel.com> wrote:
>
>> Hi Stuart, all
>>
>> As we are approaching the end of the public review, I once again reviewed
>> the specification, the results are below. These are some my older
>comments
>> which have not yet been anyhow resolved plus few new. I could volunteer
>to
>> develop some of the suggestions further and prepare a complete source
>> updates for the parts which I suggest to change. BTW, I also entered
>these
>> comments into the JCP comments page for this JSR:
>> http://wiki.jcp.org/boards/index.php?t=2852
>>
>> Thanks,
>> Konst
>>
>> Intel Novosibirsk
>> Closed Joint Stock Company Intel A/O
>> Registered legal address: Krylatsky Hills Business Park,
>> 17 Krylatskaya Str., Bldg 4, Moscow 121614,
>> Russian Federation
>>
>> === interface ImageSymbol ===
>> 1)
>> Add 'public int getSize()' method which would return a number of bytes
>> occupied by the symbol in the image.
>>
>> That does make sense -  perhaps we should also consider adding in a
>getType()  method ?
>
>=== interface ImageStackFrame ===
>> 1)
>> API also needs to support 'split' stack frames: e.g. on Itanium every
>frame
>> consists of a traditinal memory stack frame and additionally, a register
>> stack frame where Register Stack Engine (RSE) spills current register
>frame
>> during a call (and restores from upon return). The easiest way probably
>is
>> to mention in the spec something like "implementations can extend this
>> interface to provide data specific to a particular platform, such as
>> register stack frame location on Itanium (r), or frame size").
>>
>> Thanks - you've given as an easy way out :-)
>
>
>> === interface JavaField ===
>> 1)
>> During VM crash debugging it is sometimes important to know the offset of
>a
>> particular field from object start. So, something like
>>        /** @return for instance fields, offset of the field from object's
>> beginning in
>> * the memory; -1 if unknown or for static fields */
>> public int getOffset()
>>
>> This makes sense too -  though I think it would be better to  have
>something like this instead :
>
>public ImagePointer getOffset(JavaObject )
>
>Gives you more flexibility in the implementation and returns something
>you're going to need anyway.   The ImagePointer can then be used to look at
>the contents of the object directly.
>
>
>> === interface JavaLocation ===
>> 1)
>> The spec says that it "Represents a point of execution within a Java
>> method", however it does not provide access to bytecode index of
>currently
>> executed Java instruction, which is the only available (sensible) current
>> Java code position description for interpreted frames, and an invaluable
>> complement for compiled method's current code address. So, I would
>suggest
>> to also have
>>        /** @return bytecode index corresponding to position in the code
>> represented by
>> * this JavaLocation instance */
>>        public int getBytecodeIndex()
>>
>> Agreed -  perhaps its should be called  getProgramCounter()  or something
>similar?
>
>2)
>> getCompilationLevel() suits more to the piece of code which encloses this
>> JavaLocation, and does not seem to fit logically well here
>>
>> Agreed -  see comments later about this.
>
>
>> 3)
>> JavaMethod getMethod() does not provide easy way to identify piece of
>code
>> this location belongs to. Another method
>>        /** @return a description of a logical chunk of code this location
>> belongs to */
>>        public CodeStub getContainingCodeStub()
>> would achieve this. Additionally, I think that a "piece of generated
>code"
>> is important concept within any JVM to dispense a specific interface for
>it
>> - CodeStub.
>>
>>  interface CodeStub {
>>    String getName();
>>    ImageSection getCode();
>>  }
>> CodeStub which a compiled method would extend from it:
>>
>> CompiledMethodCode extends CodeStub {
>>        JavaMethod getMethod();
>>        int getCompilationLevel();
>>        /** @return (for OSR compilations - bytecode index of the entry
>> point into
>>  * this method) */
>>        int getEntryPointBytecodeIndex();
>>        // plus maybe GC maps (all implementaions) and other
>> implementation-specific info
>> }
>>
>> Konst - I'm not sure I really understand the benefit of this item over
>what
>already exists.  Could you start a separate thread and provide some more
>info about sort of usecase you had in mind?
>
>
>> === interface JavaMethod ===
>> 1)
>>        /** Get the set of ImageSections containing the compiled code of
>> this method. */
>> List getCompiledSections()
>>
>>
>> I would suggest to change to
>>        /** Get the set of CompiledMethodCode objects containing the
>> compiled code of
>>  *  this method. */
>> List getCompiledSections()
>>
>> Understood -  this is tied up with the item above so we can resolve them
>> together.
>>
>
>
>> === interface JavaStackFrame ===
>> 1)
>> Usually, Java stack frames contain information about Java monitors held
>in
>> this frame. This is necessary because Java spec requires "forced" release
>of
>> all monitors held by a method in case it "completes abruptly". So
>something
>> like
>>        /** @return list of JavaObject instances representing monitors
>held
>> by
>>  * this method activation */
>> List getHeldMonitors()
>> would be really useful.
>>
>> Agreed - this would be very useful.
>
>
>> 2)
>> Important case of JavaStackFrame is interpreted frames - in this case
>> current position in the machine code is not that important (this position
>is
>> not unique to a method) as current position in the bytecode being
>executed.
>> So it would be good to distinguish compiled frames from interpreted ones
>> (IMO, this is implementable for any JVM)
>>        public boolean isCompiled();
>>        public boolean isInterpreted();
>> In case of interpreted frames, more information might be available.
>>
>> Thats a good idea. Do we need both?   What about levels of compilation.
>You suggested moving getCompilationLevel() to here so can't we use that
>with
>perhaps 0 = interpreted?
>
>
>
>> 3)
>> It would be of great help to an engineer investigating a crash in
>compiled
>> code or some Java-level runtime error to know the JVM virtual state (in
>JVMS
>> sense) for all Java stack frames, i.e.:
>> -       held monitors
>> -       current bytecode index
>> -       values of all local variables
>> -       content of the expression stack
>>
>> and some JVMs can provide such info even for compiled frames. I think it
>is
>> desirable to add something like (removing getHeldMonitors):
>>
>>        public JVMState getCurrentJVMState();
>>
>>
>> === interface JavaReference ===
>> 1)
>> HEAP_ROOT_*, REFERENCE_* and other constants are omitted from the PDF
>> document
>>
>> ok - thanks for spotting that.
>
>
>> 2)
>> This API defines heap roots returned by getHeapRoots queries (e.g.
>> JavaStackFrame.getHeapRoots) to be JavaReferences. Sometimes it is very
>> helpful to know where the heap root actually resides - the corresponding
>CPU
>> register and/or memory address (in a memory stack frame or register stack
>> frame), and this info is usually available in Java runtimes. Something
>like
>>
>>    /** @return a CPU register name where the reference's target is
>located
>> or
>>     * null if such information is not available */
>>    public ImageRegister getRegister();
>>
>> I understand why you want this  - is it only restricted to references?
>Are there other places where knowing which register was used is important?
>
>   /** @return  a location within this image where the reference's target
>is
>>     * located or null if such information is not available */
>>    public ImagePointer.getImageLocation();
>>
>> Agree with this idea  -  I would probably go with something a little more
>generic though :
>
>     public ImagePointer getAddress();
>
>
>
>> would be helpful.
>>
>> === interface JavaRuntime ===
>>
>> 1) It should be possible in the API to iterate trough generated code
>> sequences other than managed methods - so-called 'stubs'. Each stub is
>> similar to a method: it has a name, (possibly, multiple) code sections.
>This
>> interface (JavaRuntime) seems an appropriate place for this API:
>>        /** @return  a list of CodeStub objects representing code
>sequences
>>  * generated internally by the JVM */
>>        public List getJVMCodeStubs();
>>
>> (2) Not all methods within a Java runtime are necessarily compiled. Many
>> may remain interpreted, others might not even have a bytecode. So I would
>> suggest to replace 'getCompiledMethods' with
>>        /** @return a list of JavaMethod objects representing managed
>> methods known
>>  * to the system. Each JavaMethod may be either interpreted or compiled
>or
>>   native. Method's code for compiled methods can be quired via @see
>>  #JavaMethod.getCompiledSections() */
>> public List getJavaMethods();
>>
>> === Common Register Names ===
>> 1)
>> 'Long' or 'Integer' is too vague for specifying register type. It is
>better
>> to specify the width of the register - 256 (future Intel architectures) /
>> 128 (SSE registers) / 80 (FP registers in ia32, Intel64, IA64)/ 64 / 32
>bit
>> - plus an additional characteristic such as 'general purpose/floating
>> point/application/flags/...'
>>
>> Understood
>
>
>> 2)
>> The table title "Table A.2. IA64 Register Names" is wrong, it should read
>> either "Intel64 Register Names" or "AMD64 Register Names", as "IA64" is
>> Itanium architecture.
>>
>> Understood
>
>> 3)
>> For the complete list of recommended register names and their
>> specifications for ia32 and Intel64 architectures I suggest add a link to
>>
>> Intel(r) 64 and IA-32 Architectures Software Developer's Manual
>(September
>> 2009), Volume 1:Basic Architecture, Sections "3.4 BASIC PROGRAM EXECUTION
>> REGISTERS" and "3.5 INSTRUCTION POINTER"
>> http://www.intel.com/Assets/PDF/manual/253665.pdf
>>
>> Agreed
>
>> 4)
>> I suggest to add a new section "IA64 Register Names" with a link to the
>> official Itanium(r) 2 specification for the recommended IA64 register
>names
>> and their specifications, which is
>>
>> Intel(r) Itanium(r) Architecture Software Developer's Manual (rev 2.2),
>> Volume 1: Application Architecture, Section 3.1 "Application Register
>State"
>> http://download.intel.com/design/Itanium/manuals/24531705.pdf
>>
>> Agreed
>>
>
>
>> >-----Original Message-----
>> >From: Stuart Monteith [mailto:stukato@stoo.me.uk]
>> >Sent: Friday, November 20, 2009 11:29 PM
>> >To: kato-dev@incubator.apache.org
>> >Subject: Apache Kato release - comments please
>> >
>> >Hello all,
>> >     Steve and I have been working on getting the codebase ready for a
>> >release. This is a non-trivial activity with lots of things to get
>right.
>> >
>> >Given the state of the project as it is just now, we can't claim to be
>> >doing a 1.0 release, instead I'm following the Milestone convention.
>> >
>> >I'm not proposing that what I've assembled is submitted as a release
>> >candidate just yet, as there is still work to do on it.
>> >
>> >If you look here, you'll find the files that would make up a release:
>> >
>> http://people.apache.org/~monteith/kato/apache-kato-M1-incubating-
>RC1/<http://people.apache.org/%7Emonteith/kato/apache-kato-M1-incubating-
>RC1/>
>> >
>> >I've also created a tag:
>> >
>> >https://svn.apache.org/repos/asf/incubator/kato/tags/apache-kato-M1-
>> >incubating-RC1/
>> >
>> >
>> >Obvious things that are wrong:
>> >     apache-kato-M1-incubating-tomcat.demos - packages are undocumented
>> >and missing some files.
>> >     The documentation - it needs more work, but I'd be interested in
>> >some cursory feedback.
>> >     The CJVMTI agent needs work under Linux - it's missing some
>> >information.
>> >     The TCK is still under development.
>> >     Some filtering needs to be applied to the rat reports.
>> >     There are some spurious files, like readme.html.
>> >
>> >I have found this a very useful exercise, as this is the first that
>> >we've packaged the projects contents and seen what it actually includes,
>> >so this should give everyone a good opportunity to have a look and see.
>> >
>> >
>> >Regards,
>> >     Stuart
>> >
>> >--
>> >Stuart Monteith
>> >http://blog.stoo.me.uk/
>>
>>
>
>
>--
>Steve

Re: Apache Kato release - comments please

Posted by Steve Poole <sp...@googlemail.com>.
Konst - by replies to you comments below.


On Wed, Nov 25, 2009 at 1:55 PM, Bobrovsky, Konstantin S <
konstantin.s.bobrovsky@intel.com> wrote:

> Hi Stuart, all
>
> As we are approaching the end of the public review, I once again reviewed
> the specification, the results are below. These are some my older comments
> which have not yet been anyhow resolved plus few new. I could volunteer to
> develop some of the suggestions further and prepare a complete source
> updates for the parts which I suggest to change. BTW, I also entered these
> comments into the JCP comments page for this JSR:
> http://wiki.jcp.org/boards/index.php?t=2852
>
> Thanks,
> Konst
>
> Intel Novosibirsk
> Closed Joint Stock Company Intel A/O
> Registered legal address: Krylatsky Hills Business Park,
> 17 Krylatskaya Str., Bldg 4, Moscow 121614,
> Russian Federation
>
> === interface ImageSymbol ===
> 1)
> Add 'public int getSize()' method which would return a number of bytes
> occupied by the symbol in the image.
>
> That does make sense -  perhaps we should also consider adding in a
getType()  method ?

=== interface ImageStackFrame ===
> 1)
> API also needs to support 'split' stack frames: e.g. on Itanium every frame
> consists of a traditinal memory stack frame and additionally, a register
> stack frame where Register Stack Engine (RSE) spills current register frame
> during a call (and restores from upon return). The easiest way probably is
> to mention in the spec something like "implementations can extend this
> interface to provide data specific to a particular platform, such as
> register stack frame location on Itanium (r), or frame size").
>
> Thanks - you've given as an easy way out :-)


> === interface JavaField ===
> 1)
> During VM crash debugging it is sometimes important to know the offset of a
> particular field from object start. So, something like
>        /** @return for instance fields, offset of the field from object's
> beginning in
> * the memory; -1 if unknown or for static fields */
> public int getOffset()
>
> This makes sense too -  though I think it would be better to  have
something like this instead :

public ImagePointer getOffset(JavaObject )

Gives you more flexibility in the implementation and returns something
you're going to need anyway.   The ImagePointer can then be used to look at
the contents of the object directly.


> === interface JavaLocation ===
> 1)
> The spec says that it "Represents a point of execution within a Java
> method", however it does not provide access to bytecode index of currently
> executed Java instruction, which is the only available (sensible) current
> Java code position description for interpreted frames, and an invaluable
> complement for compiled method's current code address. So, I would suggest
> to also have
>        /** @return bytecode index corresponding to position in the code
> represented by
> * this JavaLocation instance */
>        public int getBytecodeIndex()
>
> Agreed -  perhaps its should be called  getProgramCounter()  or something
similar?

2)
> getCompilationLevel() suits more to the piece of code which encloses this
> JavaLocation, and does not seem to fit logically well here
>
> Agreed -  see comments later about this.


> 3)
> JavaMethod getMethod() does not provide easy way to identify piece of code
> this location belongs to. Another method
>        /** @return a description of a logical chunk of code this location
> belongs to */
>        public CodeStub getContainingCodeStub()
> would achieve this. Additionally, I think that a "piece of generated code"
> is important concept within any JVM to dispense a specific interface for it
> - CodeStub.
>
>  interface CodeStub {
>    String getName();
>    ImageSection getCode();
>  }
> CodeStub which a compiled method would extend from it:
>
> CompiledMethodCode extends CodeStub {
>        JavaMethod getMethod();
>        int getCompilationLevel();
>        /** @return (for OSR compilations - bytecode index of the entry
> point into
>  * this method) */
>        int getEntryPointBytecodeIndex();
>        // plus maybe GC maps (all implementaions) and other
> implementation-specific info
> }
>
> Konst - I'm not sure I really understand the benefit of this item over what
already exists.  Could you start a separate thread and provide some more
info about sort of usecase you had in mind?


> === interface JavaMethod ===
> 1)
>        /** Get the set of ImageSections containing the compiled code of
> this method. */
> List getCompiledSections()
>
>
> I would suggest to change to
>        /** Get the set of CompiledMethodCode objects containing the
> compiled code of
>  *  this method. */
> List getCompiledSections()
>
> Understood -  this is tied up with the item above so we can resolve them
> together.
>


> === interface JavaStackFrame ===
> 1)
> Usually, Java stack frames contain information about Java monitors held in
> this frame. This is necessary because Java spec requires "forced" release of
> all monitors held by a method in case it "completes abruptly". So something
> like
>        /** @return list of JavaObject instances representing monitors held
> by
>  * this method activation */
> List getHeldMonitors()
> would be really useful.
>
> Agreed - this would be very useful.


> 2)
> Important case of JavaStackFrame is interpreted frames - in this case
> current position in the machine code is not that important (this position is
> not unique to a method) as current position in the bytecode being executed.
> So it would be good to distinguish compiled frames from interpreted ones
> (IMO, this is implementable for any JVM)
>        public boolean isCompiled();
>        public boolean isInterpreted();
> In case of interpreted frames, more information might be available.
>
> Thats a good idea. Do we need both?   What about levels of compilation.
You suggested moving getCompilationLevel() to here so can't we use that with
perhaps 0 = interpreted?



> 3)
> It would be of great help to an engineer investigating a crash in compiled
> code or some Java-level runtime error to know the JVM virtual state (in JVMS
> sense) for all Java stack frames, i.e.:
> -       held monitors
> -       current bytecode index
> -       values of all local variables
> -       content of the expression stack
>
> and some JVMs can provide such info even for compiled frames. I think it is
> desirable to add something like (removing getHeldMonitors):
>
>        public JVMState getCurrentJVMState();
>
>
> === interface JavaReference ===
> 1)
> HEAP_ROOT_*, REFERENCE_* and other constants are omitted from the PDF
> document
>
> ok - thanks for spotting that.


> 2)
> This API defines heap roots returned by getHeapRoots queries (e.g.
> JavaStackFrame.getHeapRoots) to be JavaReferences. Sometimes it is very
> helpful to know where the heap root actually resides - the corresponding CPU
> register and/or memory address (in a memory stack frame or register stack
> frame), and this info is usually available in Java runtimes. Something like
>
>    /** @return a CPU register name where the reference's target is located
> or
>     * null if such information is not available */
>    public ImageRegister getRegister();
>
> I understand why you want this  - is it only restricted to references?
Are there other places where knowing which register was used is important?

   /** @return  a location within this image where the reference's target is
>     * located or null if such information is not available */
>    public ImagePointer.getImageLocation();
>
> Agree with this idea  -  I would probably go with something a little more
generic though :

     public ImagePointer getAddress();



> would be helpful.
>
> === interface JavaRuntime ===
>
> 1) It should be possible in the API to iterate trough generated code
> sequences other than managed methods - so-called 'stubs'. Each stub is
> similar to a method: it has a name, (possibly, multiple) code sections. This
> interface (JavaRuntime) seems an appropriate place for this API:
>        /** @return  a list of CodeStub objects representing code sequences
>  * generated internally by the JVM */
>        public List getJVMCodeStubs();
>
> (2) Not all methods within a Java runtime are necessarily compiled. Many
> may remain interpreted, others might not even have a bytecode. So I would
> suggest to replace 'getCompiledMethods' with
>        /** @return a list of JavaMethod objects representing managed
> methods known
>  * to the system. Each JavaMethod may be either interpreted or compiled or
>   native. Method's code for compiled methods can be quired via @see
>  #JavaMethod.getCompiledSections() */
> public List getJavaMethods();
>
> === Common Register Names ===
> 1)
> 'Long' or 'Integer' is too vague for specifying register type. It is better
> to specify the width of the register - 256 (future Intel architectures) /
> 128 (SSE registers) / 80 (FP registers in ia32, Intel64, IA64)/ 64 / 32 bit
> - plus an additional characteristic such as 'general purpose/floating
> point/application/flags/...'
>
> Understood


> 2)
> The table title "Table A.2. IA64 Register Names" is wrong, it should read
> either "Intel64 Register Names" or "AMD64 Register Names", as "IA64" is
> Itanium architecture.
>
> Understood

> 3)
> For the complete list of recommended register names and their
> specifications for ia32 and Intel64 architectures I suggest add a link to
>
> Intel(r) 64 and IA-32 Architectures Software Developer's Manual (September
> 2009), Volume 1:Basic Architecture, Sections "3.4 BASIC PROGRAM EXECUTION
> REGISTERS" and "3.5 INSTRUCTION POINTER"
> http://www.intel.com/Assets/PDF/manual/253665.pdf
>
> Agreed

> 4)
> I suggest to add a new section "IA64 Register Names" with a link to the
> official Itanium(r) 2 specification for the recommended IA64 register names
> and their specifications, which is
>
> Intel(r) Itanium(r) Architecture Software Developer's Manual (rev 2.2),
> Volume 1: Application Architecture, Section 3.1 "Application Register State"
> http://download.intel.com/design/Itanium/manuals/24531705.pdf
>
> Agreed
>


> >-----Original Message-----
> >From: Stuart Monteith [mailto:stukato@stoo.me.uk]
> >Sent: Friday, November 20, 2009 11:29 PM
> >To: kato-dev@incubator.apache.org
> >Subject: Apache Kato release - comments please
> >
> >Hello all,
> >     Steve and I have been working on getting the codebase ready for a
> >release. This is a non-trivial activity with lots of things to get right.
> >
> >Given the state of the project as it is just now, we can't claim to be
> >doing a 1.0 release, instead I'm following the Milestone convention.
> >
> >I'm not proposing that what I've assembled is submitted as a release
> >candidate just yet, as there is still work to do on it.
> >
> >If you look here, you'll find the files that would make up a release:
> >
> http://people.apache.org/~monteith/kato/apache-kato-M1-incubating-RC1/<http://people.apache.org/%7Emonteith/kato/apache-kato-M1-incubating-RC1/>
> >
> >I've also created a tag:
> >
> >https://svn.apache.org/repos/asf/incubator/kato/tags/apache-kato-M1-
> >incubating-RC1/
> >
> >
> >Obvious things that are wrong:
> >     apache-kato-M1-incubating-tomcat.demos - packages are undocumented
> >and missing some files.
> >     The documentation - it needs more work, but I'd be interested in
> >some cursory feedback.
> >     The CJVMTI agent needs work under Linux - it's missing some
> >information.
> >     The TCK is still under development.
> >     Some filtering needs to be applied to the rat reports.
> >     There are some spurious files, like readme.html.
> >
> >I have found this a very useful exercise, as this is the first that
> >we've packaged the projects contents and seen what it actually includes,
> >so this should give everyone a good opportunity to have a look and see.
> >
> >
> >Regards,
> >     Stuart
> >
> >--
> >Stuart Monteith
> >http://blog.stoo.me.uk/
>
>


-- 
Steve