[10] RFR: 8179224: Cache strlen of Flag::_name

[Claes Redestad]

Hi Ioi,

On 2017-04-24 17:54, Ioi Lam wrote:
> Hi Claes,
>
> The code looks good. 

Thanks!

> A small nit: maybe str_equal() should be a member of the Flag class?

I've no idea what difference it would make and thus prefer not to - 
unless someone has the
time and patience to educate me (offline)... :-)

>
> Also, if you want to avoid adding the _name_len to save on the 
> footprint, how about
>
>  inline bool str_equal(const char* s, const char* q, size_t q_len) {
>     if (s[0] != q[0]) {
>        return false;
>     } else {
>       return strncmp(s, q, q_len) == 0;
>     }
> }

The footprint increase we're dealing with here seem somewhat negligible, 
<6Kb, and I'd
rather encourage work on JDK-8178991 than spending more time optimizing 
this quick fix.

Thanks!

/Claes

>
> Thanks
> - Ioi
>
> On 4/24/17 10:12 PM, Claes Redestad wrote:
>> Hi,
>>
>> startup profiling shows we're spending a surprising amount of cycles 
>> checking flag constraints and value ranges due to scanning the list 
>> of flags once for every range/constraint, which means we do more than 
>> 300k calls to Flag::find_flag.  A complete and elegant fix to this 
>> should consider rewriting the algorithm, e.g., emitting lists of 
>> flags, ranges and constraints in lexical order to allow for linear 
>> scans, or even better inline the range and constraint structs into 
>> the Flag struct[1].
>>
>> However, it turns out most (>80%) of the measured overhead at startup 
>> can be avoided by simply caching the result of doing 
>> strlen(Flag::_name), thus I propose this partial (and temporary?) fix:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8179224
>> Webrev: http://cr.openjdk.java.net/~redestad/8179224/hotspot.01/
>>
>> Testing: various hotspot command line tests locally and in rbt
>>
>> Thanks!
>>
>> /Claes
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8178991
>>
>>
>