Request for review: JDK-8004728 Hotspot support for parameter reflection

[John Rose]

On Jan 7, 2013, at 12:04 PM, Eric McCorkle wrote:

> Done.  [PL designer]It'd be nice if languages had a "usable only once"
> declaration specifier[/PL designer]
> 
> However, as Coleen pointed out Method* is not an oop.

Sure enough.  Now it's a different kind of managed pointer.

>> I have some qualms about constMethod...
> 
> I've discussed this code at length with Karen.  It is indeed very
> fragile, and I already did your suggested fault injection in the course
> of debugging (only it wasn't intentional).  If something is added in the
> wrong order, the code fails, and not in any predictable or obvious way.

The ideal would be to have "fail fast" with a diagnostic that would quickly point to the defect.  Often we can arrange our asserts to do this, so that the error shakes out quickly in a fastdebug test run.  But in this case it's tricky.

> I have ideas about how to revise it, but that should probably be
> addressed after SE8.

Yes, I suppose so.  IMO, the simplest way to go (with comparable or better compression) would be to have a bitmask, associated with a compressed offset or length table, followed by the compressed data itself, in a big block at the various derived offsets.  Accessing any of the optional data streams would require unpacking the complete offset table, an operation on 1-2 cache lines.  One advantage of this scheme would be that the order-sensitive aspects of the compression would be centralized in the offset table unpack logic.  Also, a stream-oriented scheme would allow CompressedStreams to be used (for everything except maybe the bitmask), which would be a decisive density benefit.  (Decisive for suitably chosen microbenchmarks, of course.)  The usual FooStream idioms would make it easy and cheap to access the data.

> RE parameter list: I'd re-indented them with emacs (which puts
> them all at the same column), but for some reason, the diff didn't seem
> to pick them up.  I'll sort out the tabs issue prior to push.

The webrev diff sometimes adds the "-w" or "-b" option, which obscures whitespace only changes.  That's one reason I often prefer the *.patch file in the webrev.  But the *.patch file is murked up by tabs.  Probably the webrev script should refuse to process diffs containing tabs, since jcheck will refuse them eventually.

I use Emacs in the same way.  Sometimes if a multi-line parameter list is (say) past column 40 I will exdent manually, for the sake of readability.  But, there are only a few rules written down about whitespace; consistency is usually the safest and most tasteful bet.

— John