RFR(S): 8056124 Hotspot should use PICL interface to get cacheline size on SPARC

[Igor Veresov]

Thanks, Vladimir!

igor

On Sep 4, 2014, at 9:15 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> Good. We need to backport it to 8u40 too.
> 
> Thanks,
> Vladimir
> 
> On 9/4/14 9:09 PM, Igor Veresov wrote:
>> Ok.
>> 
>> Webrev: http://cr.openjdk.java.net/~iveresov/8056124/webrev.03/
>> 
>> igor
>> 
>> On Sep 4, 2014, at 8:55 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> 
>>> Yes, it is better. Can you create separate 'state' for L1 and L2 instead of resetting (using copy constructor)? You don't save memory - they are stack local.
>>> 
>>> UniqueValueVisitor L1_state;
>>> ...
>>> UniqueValueVisitor L2_state;
>>> 
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 9/4/14 8:02 PM, Igor Veresov wrote:
>>>> Vladimir,
>>>> 
>>>> Yeah, I agree it’s hard to follow.. How about the following code. I’ve refactored most of the logic and state into a visitor, I think it became easier to follow (at least the state machine is more transparent). What do you think?
>>>> 
>>>> Webrev:  http://cr.openjdk.java.net/~iveresov/8056124/webrev.02
>>>> 
>>>> Thanks,
>>>> igor
>>>> 
>>>> On Sep 4, 2014, at 4:17 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>> 
>>>>> Put "{" at the end of previous line:
>>>>> 
>>>>> +   static int get_cache_line_sizes(picl_nodehdl_t nodeh, void *result)
>>>>> +   {
>>>>> 
>>>>> Next checks (and for L2) are not obvious. Please, add comment.
>>>>> 
>>>>> cpu_cache_sizes._L1_data != PROP_INCONSISTENT && core_cache_sizes._L1_data != PROP_INCONSISTENT
>>>>> 
>>>>> The value could be 0, a number, PROP_INCONSISTENT and PROP_MISSING.
>>>>> So you rely on the fact that call back function get_int_property_and_ensure_consistency() will not be called when
>>>>> class name does not match and the value stays 0. And only one class name "cpu" or "core" will be match.
>>>>> 
>>>>> May be add an assert that resulted _L1_data and _L2 >= 0.
>>>>> 
>>>>> Thanks,
>>>>> Vladimir
>>>>> 
>>>>> On 9/4/14 3:27 PM, Igor Veresov wrote:
>>>>>> Hi guys,
>>>>>> 
>>>>>> Sorry for the noise, but I didn’t try the change on Fujitsu and it didn’t work.
>>>>>> Fujitsu has cache line under a “core” class instead of the “cpu” class on Sun/Oracle, so I have to try both.
>>>>>> 
>>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8056124/webrev.01/
>>>>>> 
>>>>>> 
>>>>>> Thanks!
>>>>>> igor
>>>>>> 
>>>> 
>>