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