RFR(xs) 8186988: use log_warning() and log_error() instead of tty->print_cr for CDS warning and error messages

Calvin Cheung calvin.cheung at oracle.com
Thu Sep 12 00:45:38 UTC 2019


Hi David,

Thanks for taking a look.

On 9/11/19 4:19 PM, David Holmes wrote:
> Hi Calvin,
>
> On 12/09/2019 4:53 am, Ioi Lam wrote:
>> I agree that as long as the message itself it unchanged, having the 
>> extra UL decoration should be OK.
>
> I would agree with that too.
>
> Given there are already uses of log_error and log_warning for CDS 
> messages, converting the remaining tty uses seems appropriate. Though 
> as always with UL I find it hard to clearly identify the appropriate 
> tags to use e.g. just cds, or cds+X for some x.
>
>> For the "Preload Warning", I think it's prudent to keep it. I 
>> withdraw my request to remove it.
>
> I keep flipping between agreeing and not agreeing with this - why 
> should this case be different? 

I believe what Ioi meant was keeping the "Preload Warning" text in the 
warning message such as the following:

[0.286s][warning][cds] Preload Warning: Cannot find nonjdk/myPackage/MyClass

instead of removing it like the following:

[0.286s][warning][cds] Cannot find nonjdk/myPackage/MyClass

> I could argue that anyone doing -Xshare:dump is very unlikely to 
> explicitly configure UL at the same time and so by default we would 
> see all warnings and errors on stdout, just as now. But on the other 
> hand when all the VM is doing is dumping and terminating you may not 
> want to risk losing important warnings just because the user has UL 
> "mis-configured". Your call. :)

The UL logging level is set to "info" by default which includes both 
warning and error levels. In order to suppress the error and warning 
messages, one needs to specify -Xlog:cds=off which I think is a very 
unlikely scenario.

thanks,

Calvin

>
> Thanks,
> David
> -----
>
>> Thanks
>> - Ioi
>>
>>
>> On 9/11/2019 11:25 AM, Calvin Cheung wrote:
>>> Hi Thomas,
>>>
>>> Thanks for your review.
>>>
>>> On 9/11/19 10:41 AM, Thomas Stüfe wrote:
>>>> Hi Calvin,
>>>>
>>>> looks okay. This will cause the messages to be printed with UL 
>>>> decorations, right?
>>>
>>> Yes, after the change, an error message would look like:
>>>
>>> [0.337s][error][cds] Error: non-empty directory 
>>> '/scratch/jtreg_test/JTwork/scratch/0/mods/com.simple/
>>>
>>> Before change:
>>>
>>> Error: non-empty directory 
>>> '/scratch/jtreg_test/JTwork/scratch/21/mods/com.simple/
>>>
>>>> Is that okay or could this cause backward compatibility problems?
>>>
>>> Probably okay as long as the message stays the same.
>>>
>>> thanks,
>>>
>>> Calvin
>>>
>>>>
>>>> Cheers, Thomas
>>>>
>>>> On Wed, Sep 11, 2019 at 6:29 PM Calvin Cheung 
>>>> <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>> wrote:
>>>>
>>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8186988
>>>>
>>>>     webrev: http://cr.openjdk.java.net/~ccheung/8186988/webrev.00/
>>>>
>>>>     Please review this simple change for replacing the use of
>>>>     tty->print_cr
>>>>     with log_waring(cds) for CDS warning messages and with 
>>>> log_error(cds)
>>>>     for error messages.
>>>>
>>>>     Ran tier1 - 3 testing.
>>>>
>>>>     thanks,
>>>>
>>>>     Calvin
>>>>
>>


More information about the hotspot-runtime-dev mailing list