8153666: Optimize Formatter.formatMessage

[Daniel Fuchs]

Hi Jason,

On 08/06/16 15:31, Jason Mehrens wrote:
> Hi Daniel,
>
> Thanks for taking this on.  Looks good.
>
> This might be a new issue altogether but, if record.getMessage returns null a NPE can be generated during 'catalog.getString' (per RB.handleGetObject contract)  or 'java.text.MessageFormat.format' (undocumented)
> It is handled by the catch clause so it is harmless in terms of correctness.  The most common form of this abuse I've seen in the wild is
> 'logger.log(SEVERE, null, (Throwable) ioe);
>
> So if it not worth explicitly checking for null maybe we should add a comment explaining the intent that NPE could be generated and is trapped.
> For sure having tests for null message with and without a resource bundle would be a good idea.

Thanks again for pointing that out.
I thought we already had tests for that but it appear we don't.
The package.html says:

<<
In general, unless otherwise noted in the javadoc, methods and
constructors will throw NullPointerException if passed a null argument.
The one broad exception to this rule is that the logging convenience
methods in the Logger class (the config, entering, exiting, fine, finer, 
finest,
log, logp, logrb, severe, throwing, and warning methods)
will accept null values
for all arguments except for the initial Level argument (if any).
 >>

I am afraid this is both not true and not tested (sigh).

The new methods that accept a Supplier<String> may throw
a NPE for instance if the supplier is null and the level is
enabled. So it looks that we need to make yet another decision
about how to bring the spec and implementation in sync here,
and we definitely need more tests.

I will log a new issue for this - as as you say it appears
to have little to do with what this patch is about.

best regards,

-- daniel


> Thanks,
>
> Jason
>
>
> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> on behalf of Daniel Fuchs <daniel.fuchs at oracle.com>
> Sent: Wednesday, June 8, 2016 8:46 AM
> To: core-libs-dev
> Subject: RFR: 8153666: Optimize Formatter.formatMessage
>
> Hi,
>
> Please find below a patch for a small optimization
> in Formatter.formatMessage.
> This patch also removed the synchronized keyword which
> was there for historical reasons - but which has
> become needless.
>
> More background available at:
> https://bugs.openjdk.java.net/browse/JDK-8153666
> (thanks Martin!)
> and
> http://stackoverflow.com/questions/36433146/why-is-the-formatmessage-method-in-java-util-logging-formatter-synchronized
> (thanks Jason!)
>
> bug:
> 8153666: Optimize Formatter.formatMessage
> https://bugs.openjdk.java.net/browse/JDK-8153666
>
>
> patch:
> http://cr.openjdk.java.net/~dfuchs/webrev_8153666/webrev.00
>
> best regards, and thanks to all who provided suggestions and
> archeological background!
>
> -- daniel
>
>