RFR: 8292269: Log more CDS failure messages in the warning channel

David Holmes dholmes at openjdk.org
Tue Feb 7 01:13:52 UTC 2023


On Mon, 6 Feb 2023 18:09:48 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> src/hotspot/share/cds/filemap.cpp line 118:
>> 
>>> 116: }
>>> 117: 
>>> 118: void FileMapInfo::fail_continue_nowarn(const char *msg, ...) {
>> 
>> It is better to name a method by what it does do rather than what it doesn't. 
>> 
>> But I am finding the naming here problematic anyway as we do not always continue - the logic is basically `possibly_fail_else_log_and_continue`. Maybe the simplest thing here is to only have the variant that takes the `LogLevel` and so every callsite is explicit about whether it logs at warning or info level rather than try to accommodate warn/info into the name somehow?
>
> I agree that the `fail_continue` is not the ideal name for what this is trying to accomplish: log a message and *possibly* fail. 
> 
> That being said, I felt it was appropriate to abstract away the need to manage `LogLevel` as only Info and Warning are used with `fail_continue`. If someone does want to use any of the other channels, like debug, error, etc, they can still use the `fail_continue` variant that excepts a `LogLevel`, but they wouldn't have to worry about it otherwise. I think the binary option of warning vs no warning can help to keep it simple.

In that case I return to "It is better to name a method by what it does do rather than what it doesn't." So `fail_continue` and `fail_continue_warn`?

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

PR: https://git.openjdk.org/jdk/pull/12419


More information about the hotspot-runtime-dev mailing list