Request for reviews (L): 6939207: refactor constant pool index processing

[Christian Thalinger]

On Fri, 2010-04-23 at 15:52 -0700, John Rose wrote:
> On Apr 23, 2010, at 7:06 AM, Christian Thalinger wrote:
> 
> > I have to say I was a little confused about the get_index1,
> > get_index2, ... naming as it looks like you were running out of method
> > names and just append 1, 2, ... ;-)
> 
> You are right, it could look that way at first.  Can you suggest some
> comments that would short-circuit that impression quickly?

Well, you could rename it to get_index_u{1,2,4} which would correspond
to the other Bytes::get* functions.

If you want to keep them maybe add a simple comment like:

// get_{index,offset,constant}{1,2,4} methods return an
// index/offset/constant of the size of the appended number in bytes 
//(1, 2, or 4 bytes).

You may rephrase for better English.

I just see that ciStreams.hpp has comments on most of these methods.

> 
> > The change is very big.  I take a look at it again later.
> 
> That will be just fine.

src/share/vm/prims/jvmtiClassFileReconstituter.cpp:

Copyright year update missing.

src/share/vm/prims/methodComparator.cpp:

-      if (_s_old->get_index_big() != _s_new->get_index_big())
+      if (Bytes::get_Java_u2(_s_old->bcp() + 1) != Bytes::get_Java_u2(_s_new->bcp() + 1))

Why can't you use get_index2() here?  What is the difference to:

case Bytecodes::_sipush    :
-    if (_s_old->get_index_big() != _s_new->get_index_big())
+    if (_s_old->get_index2() != _s_new->get_index2())

Otherwise looks good.

-- Christian