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

[Jon Masamitsu]

On 4/29/2015 4:31 PM, Jungwoo Ha wrote:
> Alright. I'll send it to runtime.

Jungwoo,

Let me suggest hotspot-dev.  But if you've already sent it,
runtime is OK too.

Jon

>
> On Wed, Apr 29, 2015 at 4:27 PM, Kim Barrett <kim.barrett at oracle.com 
> <mailto:kim.barrett at oracle.com>> wrote:
>
>     On Apr 29, 2015, at 5:46 PM, Jungwoo Ha <jwha at google.com
>     <mailto:jwha at google.com>> wrote:
>     >
>     > Oops. I meant to copy BUG-8079091
>     > http://cr.openjdk.java.net/~jwha/8079091/webrev.00/
>     <http://cr.openjdk.java.net/%7Ejwha/8079091/webrev.00/>
>     >
>     >
>     > On Wed, Apr 29, 2015 at 2:40 PM, Jungwoo Ha <jwha at google.com
>     <mailto:jwha at google.com>> wrote:
>     > BUG: https://bugs.openjdk.java.net/browse/JDK-8079091
>     > Webrev: http://cr.openjdk.java.net/~jwha/8075288/webrev.00/
>     <http://cr.openjdk.java.net/%7Ejwha/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".
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150429/95533610/attachment.htm>