RFR(S): 8209598: Use log_error for error message in CDS code

David Holmes david.holmes at oracle.com
Wed Oct 24 04:46:32 UTC 2018


Hi Calvin,

I hadn't actually looked at the changes. Now I have. :) I don't think it 
makes sense to convert tty->print to UL when it precedes a vm_exit(1). 
These things should be treated as another variation of 
vm_exit_during_initialization IMHO (vm_exit_during_dumping?). (And it's 
not good that we have these exits dotted all over the place to begin with!).

Cheers,
David

On 24/10/2018 2:36 PM, Calvin Cheung wrote:
> Ioi, David,
> 
> Here's an updated webrev:
>      http://cr.openjdk.java.net/~ccheung/8209598/webrev.01/
> 
> I've run those 2 tests locally on linux-x64.
> 
> thanks,
> Calvin
> 
> On 10/23/18, 8:50 PM, Ioi Lam wrote:
>>
>>
>> On 10/23/18 7:27 PM, Calvin Cheung wrote:
>>>
>>>
>>> On 10/23/18, 5:18 PM, Ioi Lam wrote:
>>>> Hi Calvin,
>>>>
>>>>    if (PrintSharedArchiveAndExit) {
>>>>      if (PrintSharedDictionary) {
>>>> -      tty->print_cr("\nShared classes:\n");
>>>> +      log_info(cds)("\nShared classes:\n");
>>>>        SystemDictionary::print_shared(tty);
>>>>      }
>>>>      if (_archive_loading_failed) {
>>>> -      tty->print_cr("archive is invalid");
>>>> +      log_error(cds)("archive is invalid");
>>>>        vm_exit(1);
>>>>      } else {
>>>> -      tty->print_cr("archive is valid");
>>>> +      log_info(cds)("archive is valid");
>>>>        vm_exit(0);
>>>>      }
>>>>    }
>>>>
>>>> I think this part should use print_cr, because the option says 
>>>> "Print ...". It shouldn't be necessary to explicitly set -Xlog:cds 
>>>> in order to get the printed message.
>>> Should I just revert log_info changes and leave the log_error there?
>>
>> I think the log_error should be reverted as well, because it would 
>> look out of place with the rest of the output.
>>
>> Thanks
>> - Ioi
>>>>
>>>> If you revert this, I think the two test cases also can be reverted.
>>> Yes, the changes to the two tests were due to the log_info changes.
>>>>
>>>> The rest of the changes  look OK.
>>> Thanks for your review.
>>>
>>> Calvin
>>>>
>>>> Thanks
>>>>
>>>> - Ioi
>>>>
>>>>
>>>> On 10/23/18 2:45 PM, Calvin Cheung wrote:
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8209598
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8209598/webrev.00/
>>>>>
>>>>> Use log_error(cds) instead of tty->print_cr for CDS error messages. 
>>>>> Also converted 2 CDS info messages to log_info(cds).
>>>>>
>>>>> Testing: mach5 hs-tier{1,2,3}
>>>>>
>>>>> thanks,
>>>>> Calvin
>>>>
>>


More information about the hotspot-runtime-dev mailing list