RFR: 8255489: Unify the parsing of @lambda-proxy and @lambda-form-invokers tags in a classlist

[Yumin Qi]

On Thu, 29 Oct 2020 20:44:59 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Looks good to me. There are two issues in question:
>> 1) _indy_items is specific name, it was designed for indy before and now it includes lambda_form_invokers. I don't know if we should use a more generic name here.
>> 2) This part seems not necessary since we already done this before calling parse_at_tag:
>> 211   } else if (strcmp(_indy_items->at(0), LAMBDA_FORM_TAG) == 0) {
>> 212     for (int i = _line_len; i >= 0; i--) {
>> 213       if (_line[i] == '\0') {
>> 214         _line[i] = ' ';
>> 215       }
>> 216     }
>
>> Looks good to me. There are two issues in question:
>> 
>> 1. _indy_items is specific name, it was designed for indy before and now it includes lambda_form_invokers. I don't know if we should use a more generic name here.
> 
> Maybe name it _lambda_items?
> 
> I've another idea. Add another function to separate the tag and the rest of the line. Within the `if-else` block, only the rest of the line needs to be processed since the tag is not needed for loading the class and we can avoid code like the following in LambdaFormInvokers::regenerate_holder_classes:
> `record += strlen(LAMBDA_FORM_TAG) + 1; // skip the @lambda_form_invoker prefix`
> Also, the _indy_items name doesn't need to be changed since it will be only for the @lambda-proxy tag.
> 
>> 2. This part seems not necessary since we already done this before calling parse_at_tag:
>>    211   } else if (strcmp(_indy_items->at(0), LAMBDA_FORM_TAG) == 0) {
>>    212     for (int i = _line_len; i >= 0; i--) {
>>    213       if (_line[i] == '\0') {
>>    214         _line[i] = ' ';
>>    215       }
>>    216     }
> 
> I don't see why the above is not necessary as the following code in classListParser.cpp has been removed with this patch:
> 
> 118     // Check if the line is output TRACE_RESOLVE
> 119     if (strncmp(_line, LambdaFormInvokers::lambda_form_invoker_tag(),
> 120                 strlen(LambdaFormInvokers::lambda_form_invoker_tag())) == 0) {
> 121       LambdaFormInvokers::append(os::strdup((const char*)_line, mtInternal));
> 122       continue;
> 123     }
> 
> The for loop is also necessary because the call to `split_tokens_by_whitespace()` has replaced some blank spaces with '\0'.

> > Looks good to me. There are two issues in question:
> > 
> > 1. _indy_items is specific name, it was designed for indy before and now it includes lambda_form_invokers. I don't know if we should use a more generic name here.
> 
> Maybe name it _lambda_items?
> 
> I've another idea. Add another function to separate the tag and the rest of the line. Within the `if-else` block, only the rest of the line needs to be processed since the tag is not needed for loading the class and we can avoid code like the following in LambdaFormInvokers::regenerate_holder_classes:
> `record += strlen(LAMBDA_FORM_TAG) + 1; // skip the @lambda_form_invoker prefix`
> Also, the _indy_items name doesn't need to be changed since it will be only for the @lambda-proxy tag.
> 
> > 1. This part seems not necessary since we already done this before calling parse_at_tag:
> >    211   } else if (strcmp(_indy_items->at(0), LAMBDA_FORM_TAG) == 0) {
> >    212     for (int i = _line_len; i >= 0; i--) {
> >    213       if (_line[i] == '\0') {
> >    214         _line[i] = ' ';
> >    215       }
> >    216     }
> 
> I don't see why the above is not necessary as the following code in classListParser.cpp has been removed with this patch:
> 
> ```
> 118     // Check if the line is output TRACE_RESOLVE
> 119     if (strncmp(_line, LambdaFormInvokers::lambda_form_invoker_tag(),
> 120                 strlen(LambdaFormInvokers::lambda_form_invoker_tag())) == 0) {
> 121       LambdaFormInvokers::append(os::strdup((const char*)_line, mtInternal));
> 122       continue;
> 123     }
> ```
> 
> The for loop is also necessary because the call to `split_tokens_by_whitespace()` has replaced some blank spaces with '\0'.

Maybe we don't need change name for _indy_items. First in parse_at_tag:
if (strcmp(_line, LAMBDA_FORM_TAG, strlen(LAMBDA_FORM_TAG)) ==0) {
  // this is lambda_form_invoker line
  // 
}

Then check if it is indy index format.

-------------

PR: https://git.openjdk.java.net/jdk/pull/942