Changing lots of files - mainly GC code (original) (raw)

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.

  1. 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

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.

#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

More information about the hotspot-dev mailing list