Changing lots of files - mainly GC code

Krystal Mok rednaxelafx at gmail.com
Mon Jan 13 10:36:53 PST 2014


Hi Jesper,

Thanks! I took a look and the change was fine.

I do have a couple of other comment fixes that I'd like to hitchhike:

1. oops/method.hpp

The comments say ConstMethod* and MethodData* are oops, which they aren't
anymore.
I'm not sure if the wording for "putting oops and method_size first for
better gc cache locality" still matters. It probably doesn't matter
anymore, since GC doesn't have to mark through these fields with the
PermGen removal.

2. ci/ciField.hpp, ci/ciField.cpp

The comments say in order to consider a field as a constant, it cannot hold
an non-perm-space oop. I believe this limitation has been removed. Could
someone from the compiler team verify if that's true?

The diff is at the end of this mail.

Thanks,
Kris

diff -r 9d39e8a8ff61 src/share/vm/ci/ciField.cpp
--- a/src/share/vm/ci/ciField.cpp Fri Dec 27 07:51:07 2013 -0800
+++ b/src/share/vm/ci/ciField.cpp Mon Jan 13 10:31:17 2014 -0800
@@ -202,13 +202,9 @@
     }

     // This field just may be constant.  The only cases where it will
-    // not be constant are:
+    // not be constant is:
     //
-    // 1. The field holds a non-perm-space oop.  The field is, strictly
-    //    speaking, constant but we cannot embed non-perm-space oops into
-    //    generated code.  For the time being we need to consider the
-    //    field to be not constant.
-    // 2. The field is a *special* static&final field whose value
+    // 1. The field is a *special* static&final field whose value
     //    may change.  The three examples are java.lang.System.in,
     //    java.lang.System.out, and java.lang.System.err.

diff -r 9d39e8a8ff61 src/share/vm/ci/ciField.hpp
--- a/src/share/vm/ci/ciField.hpp Fri Dec 27 07:51:07 2013 -0800
+++ b/src/share/vm/ci/ciField.hpp Mon Jan 13 10:31:17 2014 -0800
@@ -130,9 +130,7 @@
   //   1. The field is both static and final
   //   2. The canonical holder of the field has undergone
   //      static initialization.
-  //   3. If the field is an object or array, then the oop
-  //      in question is allocated in perm space.
-  //   4. The field is not one of the special static/final
+  //   3. The field is not one of the special static/final
   //      non-constant fields.  These are java.lang.System.in
   //      and java.lang.System.out.  Abomination.
   //
diff -r 9d39e8a8ff61 src/share/vm/oops/method.hpp
--- a/src/share/vm/oops/method.hpp Fri Dec 27 07:51:07 2013 -0800
+++ b/src/share/vm/oops/method.hpp Mon Jan 13 10:31:17 2014 -0800
@@ -38,13 +38,11 @@
 #include "utilities/accessFlags.hpp"
 #include "utilities/growableArray.hpp"

-// A Method* represents a Java method.
+// A Method represents a Java method.
 //
 // Memory layout (each line represents a word). Note that most
applications load thousands of methods,
 // so keeping the size of this structure small has a big impact on
footprint.
 //
-// We put all oops and method_size first for better gc cache locality.
-//
 // The actual bytecodes are inlined after the end of the Method struct.
 //
 // There are bits in the access_flags telling whether inlined tables are
present.
@@ -64,17 +62,17 @@
 // | header                                               |
 // | klass                                                |
 // |------------------------------------------------------|
-// | ConstMethod*                   (oop)                 |
+// | ConstMethod*                   (metadata)            |
 // |------------------------------------------------------|
-// | methodData                     (oop)                 |
-// | methodCounters                                       |
+// | MethodData*                    (metadata)            |
+// | MethodCounters                                       |
 // |------------------------------------------------------|
 // | access_flags                                         |
 // | vtable_index                                         |
 // |------------------------------------------------------|
 // | result_index (C++ interpreter only)                  |
 // |------------------------------------------------------|
-// | method_size             |   intrinsic_id|   flags    |
+// | method_size             | intrinsic_id  |   flags    |
 // |------------------------------------------------------|
 // | code                           (pointer)             |
 // | i2i                            (pointer)             |




On Mon, Jan 13, 2014 at 9:07 PM, Jesper Wilhelmsson <
jesper.wilhelmsson at oracle.com> wrote:

> Hi Kris!
>
> No problem, I'll add it to the change. Please verify that it looks OK in
> the updated webrev (same link as before).
>
> http://cr.openjdk.java.net/~jwilhelm/8025856/webrev.1/
>
> /Jesper
>
>
> Krystal Mok skrev 10/1/14 7:17 PM:
>
>> Hi Jesper,
>>
>> Nice to see cleaner code!
>>
>> Hitchhike: It'd be nice if you could include this typo in:
>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-
>> dev/2014-January/012944.html
>> I didn't really like sending one-liner comment typo fix anyway...with a
>> lot of
>> other typos, the story is different ;-)
>>
>> Thanks,
>> Kris
>>
>>
>>
>> On Fri, Jan 10, 2014 at 11:54 PM, Coleen Phillmore <
>> coleen.phillimore at oracle.com
>> <mailto:coleen.phillimore at oracle.com>> wrote:
>>
>>     Seems fine with me also.  Could you find typos in the comments in the
>>     runtime code "by accident" too? :)
>>     thanks,
>>     Coleen
>>
>>
>>     On 1/10/2014 10:39 AM, Daniel D. Daugherty wrote:
>>
>>         On 1/10/14 5:49 AM, Jesper Wilhelmsson wrote:
>>
>>             Hi,
>>
>>             I have a change out for review that fixes a huge pile of
>> typos in
>>             the comments in the GC code. The RFR was sent to the GC list,
>> but I
>>             want to give a heads up in case anyone else is changing GC
>> code and
>>             want to avoid merge conflicts.
>>
>>             The patch: http://cr.openjdk.java.net/~__
>> jwilhelm/8025856/webrev.1/
>>
>>             <http://cr.openjdk.java.net/~jwilhelm/8025856/webrev.1/>
>>
>>
>>             There are also a few files where I happened to find a few
>> typos "by
>>             accident" in code that is not strictly GC code. These are:
>>
>>             src/share/vm/memory/heap.cpp
>>             src/share/vm/memory/heap.hpp
>>             src/share/vm/memory/__allocation.hpp
>>             src/share/vm/memory/__resourceArea.hpp
>>             src/share/vm/runtime/thread.__cpp
>>
>>
>>             There is a total of eight typos fixed in these files so I
>> think the
>>             risk of merge conflicts here is minimal. Are there any
>> objections to
>>             including these fixes in the change?
>>
>>
>>         Vote: go for it!
>>
>>         Dan
>>
>>
>>             Thanks,
>>             /Jesper
>>
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140114/ddd939f8/attachment-0001.html 


More information about the hotspot-compiler-dev mailing list