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

Jungwoo Ha jwha at google.com
Thu Apr 30 07:37:26 UTC 2015


My original change (not embedding CHeapObj):
http://cr.openjdk.java.net/~jwha/8079091/webrev.00/
Jon's suggestion (from hotspot-gc-dev):
http://cr.openjdk.java.net/~jwha/8079091/webrev.02/
<http://cr.openjdk.java.net/~jwha/8079091/webrev.00/>

Please take a look on .02.

On Thu, Apr 30, 2015 at 12:05 AM, Jungwoo Ha <jwha at google.com> wrote:

> 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".
>>>>>
>>>>>
>>>>>
>>>>
>


More information about the hotspot-runtime-dev mailing list