JDK 13 RFR of JDK-8220346: Refactor java.lang.Throwable to use Objects.requireNonNull

[Remi Forax]

----- Mail original -----
> De: "Martin Buchholz" <martinrb at google.com>
> À: "Tagir Valeev" <amaembo at gmail.com>
> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Vendredi 8 Mars 2019 21:35:59
> Objet: Re: JDK 13 RFR of JDK-8220346: Refactor java.lang.Throwable to use Objects.requireNonNull

> On Fri, Mar 8, 2019 at 3:57 AM Tagir Valeev <amaembo at gmail.com> wrote:
> 
>> Hello!
>>
>> > diff -r 274361bd6915 src/java.base/share/classes/java/lang/Throwable.java
>> > --- a/src/java.base/share/classes/java/lang/Throwable.java    Thu Mar 07
>> > 10:22:19 2019 +0100
>> > +++ b/src/java.base/share/classes/java/lang/Throwable.java    Fri Mar 08
>> > 02:06:42 2019 -0800
>> > @@ -874,8 +874,7 @@
>> >           // Validate argument
>> >           StackTraceElement[] defensiveCopy = stackTrace.clone();
>> >           for (int i = 0; i < defensiveCopy.length; i++) {
>> > -            if (defensiveCopy[i] == null)
>> > -                throw new NullPointerException("stackTrace[" + i + "]");
>> > +            Objects.requireNonNull(defensiveCopy[i], "stackTrace[" + i
>> > + "]");
>>
>> Won't it produce unnecessary garbage due to string concatenation on
>> the common case when all frames are non-null?
>>
> 
> I share Tagir's doubts.  I avoid this construction in performance sensitive
> code.
> Java doesn't offer syntactic abstraction (can I haz macros?) so the Java
> Way is to write the explicit if.
> 
> Logging frameworks have the same trouble.
> https://github.com/google/flogger


No, they don't if the API use the pattern described by Peter
https://github.com/forax/beautiful_logger

> 
> Perhaps hotspot's improved automatic NPE reporting makes the message detail
> here obsolete?

anyway, i agree with what's Martin and Tagir said, requireNonNull() should not be used in this particular case.

Rémi