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

[Jungwoo Ha]

Good point.
However, it seems that ChunkManager also has the ChunkTreeDictionary
_humongous_dictionary embedded.
My original fix was keeping it as a pointer and allocate it during
BlockFreelist constructor.
See: http://cr.openjdk.java.net/~jwha/8079091/webrev.00/


On Wed, Apr 29, 2015 at 11:20 PM, David Holmes <david.holmes at oracle.com>
wrote:

> On 30/04/2015 4:05 PM, Jungwoo Ha wrote:
>
>> http://cr.openjdk.java.net/~jwha/8079091/webrev.01
>>
>
> I may be missing something but you seem to have embedded an object that is
> defined to be CHeapObj !
>
> David
>
>
>  PTAL.
>>
>> On Wed, Apr 29, 2015 at 10:22 PM, Jungwoo Ha <jwha at google.com> wrote:
>>
>>  The slower ones have higher error bars (error bars are 95% confidence
>>> interval), and you can see that those are not statistically significant.
>>> I'll reflect your suggestions and upload the new webrev.
>>>
>>> On Wed, Apr 29, 2015 at 10:00 PM, Kim Barrett <kim.barrett at oracle.com>
>>> wrote:
>>>
>>>  On Apr 29, 2015, at 7:34 PM, Jungwoo Ha <jwha at google.com> wrote:
>>>>
>>>>>
>>>>> BUG: https://bugs.openjdk.java.net/browse/JDK-8079091
>>>>> Webrev: http://cr.openjdk.java.net/~jwha/8079091/webrev.00/
>>>>>
>>>>> Can someone sponsor this change?
>>>>> 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".
>>>>
>>>>
>>>>
>>>