RFR (S): 8075288: Remove dictionary NULL check on common path of BlockFreeList methods

[Kim Barrett]

On Apr 29, 2015, at 5:46 PM, Jungwoo Ha <jwha at google.com> wrote:
> 
> Oops. I meant to copy BUG-8079091
> http://cr.openjdk.java.net/~jwha/8079091/webrev.00/
> 
> 
> On Wed, Apr 29, 2015 at 2:40 PM, Jungwoo Ha <jwha at google.com> wrote:
> BUG: https://bugs.openjdk.java.net/browse/JDK-8079091
> Webrev: http://cr.openjdk.java.net/~jwha/8075288/webrev.00/
> 
> Can someone sponsor this change?
> I've seen ~3% speed up on DaCapo benchmarks. (results attached on the bug page.)
> Regardless of the speed up, I think the changes are straight-forward improvement.
> 
> --
> Jungwoo Ha

I'm not sure of this, but I think metaspace belongs to runtime rather
than gc. If so, this should go through the hs-rt repository and have
runtime folks involved in the review.

> I've seen ~3% speed up on DaCapo benchmarks. (results attached on the bug page.)

Wow!

Hm.  There are a couple of benchmarks that are a little bit slower.  I
don't know enough about that suite of benchmarks to know how important
that might be.

Since the dictionary is always being created at construction time, the
null check in ~BlockFreelist is no longer needed either.

It seems like a further improvement might be obtained by making the
dictionary a direct member subobject of BlockFreelist, rather than
allocating it as a separate object, thereby eliminating an
indirection.

Pre-existing: Why is there a cast in SpaceManager::block_freelists()?

Pre-existing: I found the private BlockFreelist::dictionary() member
function distracting rather than helpful, and would have preferred
just using the private data member directly. But maybe that's just me.
If _dictionary is changed to a subobject rather than a pointer, I'd
suggest changing a bunch of "dictionary()->XXX" to "_dictionary.XXX".