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

[Stefan Karlsson]

On 2015-04-30 09:00, Jon Masamitsu wrote:
>
>
> On 4/29/2015 11:09 PM, Jungwoo Ha wrote:
>>
>>
>>     That style is my doing and my preference. There remains differing
>>     opinions in the
>>     group on the style and some consider the accessors wasted effort
>>     but I find them
>>     useful often enough to prefer them.
>>
>>     Jon
>>
>>
>>
>> I saw this after reading runtime thread. Things are getting confused.
>> We can do
>>
>> BlockFreelist *dictionary() { return &_dictionary; }
>>
>> and keep dictionary()->XXX
>> This is actually consistent with _humongous_blocklist.
>>
>> http://cr.openjdk.java.net/~jwha/8079091/webrev.01 
>> <http://cr.openjdk.java.net/%7Ejwha/8079091/webrev.01>
>> current one has
>> BlockFreelist &dictionary() { return _dictionary; }
>>
>> and changed it to dictionary().XXX
>
> Jungwoo,
>
> Sorry for the differing suggestions on our part.  I think Kim's point
> was to not have the accessors.   I vote to (1) keep the accessors (which
> are currently there) and  use (2)
>
>> BlockFreelist *dictionary() { return &_dictionary; }
>
> and the style dictionary()->XXX (to minimize changes).
>
> Kim,
>
> How about we let Jungwoo integrate with the style (1) and
> (2) I outline above so he is not caught in the middle.  You
> and I can talk about it on the side.

I'm fine with letting him integrate with your suggestions, but I prefer 
Kim's suggestion.

a) I don't see a need to expose a public accessor function to something 
that is private.

b) The current code mixes the usage of the accessor function and the 
instance variable, which makes the code non-obvious:

   if (dictionary() == NULL) {
    _dictionary = new BlockTreeDictionary();
   }

and forces the casual reader to go an look how dictionary() is implemented.

I'd much prefer if the code was written as:
   if (_dictionary == NULL) {
    _dictionary = new BlockTreeDictionary();
   }

or at least:
   if (dictionary() == NULL) {
    set_dictionary(new BlockTreeDictionary());
   }

StefanK


>
> Jon
>
>
>>
>> Whichever you guys prefer...
>

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