RFR: 8267118: OutOfMemoryError cannot be caught as a Throwable [v3]

[Coleen Phillimore]

On Tue, 1 Jun 2021 21:47:29 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> src/hotspot/share/oops/method.cpp line 285:
>> 
>>> 283:         Klass* k = pool->klass_at(klass_index, THREAD);
>>> 284:         if (HAS_PENDING_EXCEPTION) {
>>> 285:         if (log_is_enabled(Debug, exceptions)) {
>> 
>> Technically, this would be interesting enough to be log_info level.
>
> Perhaps, but again making it fit and provide sufficient contextual information is awkward and the throw is already captured at the Info level. So I would hope that if someone saw the unexpected throw logging they would use that as a cue to turn on debug logging to get more information.
> 
> Logging is really, really subjective - painfully so.

ok.

>> src/hotspot/share/oops/method.cpp line 300:
>> 
>>> 298:             log_info(exceptions)("Found matching handler for exception of type \"%s\" in method \"%s\" at BCI: %d",
>>> 299:                                  ex_klass == NULL ? "NULL" : ex_klass->external_name(), mh()->name()->as_C_string(), handler_bci);
>>> 300:           }
>> 
>> There's so much extra code added with this logging.  It's hard to follow as it is. Could you just have the log_info level output in both this and the case above where you find the handler, here and above?  debug level logging will include info level debugging.
>
> The problem is that debug logging is step-by-step information, while the Info logging is a complete statement. So the Info message does not fit with the previous debug messages, hence the either-or approach.

Logging is supposed to be non-invasive in the source code, which is why it was defined ideally to have one line like log_info(tag)(string)
Here it seems to dominate the code in this method.  Anything to make it shorter and less invasive would be better so that you can see how the code is actually finds the exception handler.  I only made a small request for simplification.  I think the formatting of the log messages wouldn't be bad if the info line is printed rather than the debug line.

[0.208s][debug][exceptions] Looking for catch handler for exception of type "java.lang.Error" in method "main"
[0.208s][debug][exceptions] - checking exception table entry for BCI 0 to 3
[0.208s][debug][exceptions] - entry covers throw point BCI 0
[0.208s][debug][exceptions] - found catch-all handler at BCI: 14

Would be:

[0.208s][debug][exceptions] Looking for catch handler for exception of type "java.lang.Error" in method "main"
[0.208s][debug][exceptions] - checking exception table entry for BCI 0 to 3
[0.208s][debug][exceptions] - entry covers throw point BCI 0
[0.198s][info     ][exceptions] Found catch-all handler for exception of type "java.lang.Error" in method "main" at BCI: 14

I don't think this looks bad at all and worth not having special if statement for debug/info logging.  That's 6 lines saved in this 96 line function.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4266