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

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->printcr("\nShared classes:\n"); +      loginfo(cds)("\nShared classes:\n"); SystemDictionary::printshared(tty); } if (archiveloadingfailed) { -      tty->printcr("archive is invalid"); +      logerror(cds)("archive is invalid"); vmexit(1); } else { -      tty->printcr("archive is valid"); +      loginfo(cds)("archive is valid"); vmexit(0); } } I think this part should use printcr, 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 loginfo changes and leave the logerror there? I think the logerror 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 loginfo 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 logerror(cds) instead of tty->printcr for CDS error messages. Also converted 2 CDS info messages to loginfo(cds). Testing: mach5 hs-tier{1,2,3} thanks, Calvin

More information about the hotspot-runtime-dev mailing list