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