FindBugs rules - Performance

Performance - rules provided by FindBugs

Primitive value is boxed and then immediately unboxed

A primitive is boxed, and then immediately unboxed.

This probably is due to a manual boxing in a place where an unboxed value is required, thus forcing the compiler to immediately undo the work of the boxing.

Primitive value is boxed then unboxed to perform primitive coercion

A primitive boxed value constructed and then immediately converted into a different primitive type (e.g., new Double(d).intValue()).

Just perform direct primitive coercion (e.g., (int) d).

Boxed value is unboxed and then immediately reboxed

Method allocates a boxed primitive just to call toString

A boxed primitive is allocated just to call toString().

It is more effective to just use the static form of toString which takes the primitive value.

So,

Replace... new Integer(1).toString()    With... Integer.toString(1)

Replace... new Long(1).toString()    With... Long.toString(1)

Replace... new Float(1.0).toString()    With... Float.toString(1.0)

Replace... new Double(1.0).toString()    With... Double.toString(1.0)

Replace... new Byte(1).toString()    With... Byte.toString(1)

Replace... new Short(1).toString()    With... Short.toString(1)

Replace... new Boolean(true).toString()    With... Boolean.toString(true)

Method invokes inefficient floating-point Number constructor; use static valueOf instead

Using new Double(double) is guaranteed to always result in a new object whereas Double.valueOf(double) allows caching of values to be done by the compiler, class library, or JVM.

Using of cached values avoids object allocation and the code will be faster.

Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Double and Float.

Method invokes inefficient Number constructor; use static valueOf instead

Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM.

Using of cached values avoids object allocation and the code will be faster.

Values between -128 and 127 are guaranteed to have corresponding cached instances and using valueOf is approximately 3.5 times faster than using constructor.

For values outside the constant range the performance of both styles is the same.

Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Long, Integer, Short, Character, and Byte.

The equals and hashCode methods of URL are blocking

The equals and hashCode method of URL perform domain name resolution, this can result in a big performance hit.

Consider using java.net.URI instead.

Maps and sets of URLs can be performance hogs

This method or field is or uses a Map or Set of URLs.

Since both the equals and hashCode method of URL perform domain name resolution, this can result in a big performance hit.

Consider using java.net.URI instead.

Method invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead

Creating new instances of java.lang.Boolean wastes memory, since Boolean objects are immutable and there are only two useful values of this type.

Use the Boolean.valueOf() method (or Java 1.5 autoboxing) to create Boolean objects instead.

Explicit garbage collection; extremely dubious except in benchmarking code

Code explicitly invokes garbage collection. Except for specific use in benchmarking, this is very dubious.

In the past, situations where people have explicitly invoked the garbage collector in routines such as close or finalize methods has led to huge performance black holes.

Garbage collection can be expensive. Any situation that forces hundreds or thousands of garbage collections will bring the machine to a crawl.

Method allocates an object, only to get the class object

This method allocates an object just to call getClass() on it, in order to retrieve the Class object for it.

It is simpler to just access the .class property of the class.

Use the nextInt method of Random rather than nextDouble to generate a random integer

If r is a java.util.Random, you can generate a random number from 0 to n-1 using r.nextInt(n), rather than using (int)(r.nextDouble() * n).

The argument to nextInt must be positive. If, for example, you want to generate a random value from -99 to 0, use -r.nextInt(100).

Method invokes inefficient new String(String) constructor

Using the java.lang.String(String) constructor wastes memory because the object so constructed will be functionally indistinguishable from the String passed as a parameter.

Just use the argument String directly.

Method invokes toString() method on a String

Calling String.toString() is just a redundant operation. Just use the String.

Method invokes inefficient new String() constructor

Creating a new java.lang.String object using the no-argument constructor wastes memory because the object so created will be functionally indistinguishable from the empty string constant "".

Java guarantees that identical string constants will be represented by the same String object.

Therefore, you should just use the empty string constant directly.

Huge string constants is duplicated across multiple class files

A large String constant is duplicated across multiple class files.

This is likely because a final field is initialized to a String constant, and the Java language mandates that all references to a final field from other classes be inlined into that classfile.

Method uses toArray() with zero-length array argument

This method uses the toArray() method of a collection derived class, and passes in a zero-length prototype array argument.

It is more efficient to use myCollection.toArray(new Foo[myCollection.size()])

If the array passed in is big enough to store all of the elements of the collection, then it is populated and returned directly.

This avoids the need to create a second array (by reflection) to return as the result.

Method uses toArray() with zero-length array argument

This method uses the toArray() method of a collection derived class, and passes in a zero-length prototype array argument.

It is more efficient to use myCollection.toArray(new Foo[myCollection.size()])

If the array passed in is big enough to store all of the elements of the collection, then it is populated and returned directly.

This avoids the need to create a second array (by reflection) to return as the result.

Method concatenates strings using + in a loop

The method seems to be building a String using concatenation in a loop.

In each iteration, the String is converted to a StringBuffer / StringBuilder, appended to, and converted back to a String.

This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.

Better performance can be obtained by using a StringBuffer (or StringBuilder in Java 1.5) explicitly.

For example

  // This is bad

  String s = "";

  for (int i = 0; i < field.length; ++i) {

    s = s + field[i];

  }

  // This is better

  StringBuffer buf = new StringBuffer();

  for (int i = 0; i < field.length; ++i) {

    buf.append(field[i]);

  }

  String s = buf.toString();

Should be a static inner class

This class is an inner class, but does not use its embedded reference to the object which created it.

This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.

If possible, the class should be made static.

Could be refactored into a named static inner class

This class is an inner class, but does not use its embedded reference to the object which created it.

This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.

If possible, the class should be made into a static inner class.

Since anonymous inner classes cannot be marked as static, doing this will require refactoring the inner class so that it is a named inner class.

Could be refactored into a static inner class

This class is an inner class, but does not use its embedded reference to the object which created it except during construction of the inner object.

This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.

If possible, the class should be made into a static inner class.

Since the reference to the outer object is required during construction of the inner instance, the inner class will need to be refactored so as to pass a reference to the outer instance to the constructor for the inner class.

Unread field: should this field be static ?

This class contains an instance final field that is initialized to a compile-time static value.

Consider making the field static.

Method calls static Math class method on a constant value

This method uses a static method from java.lang.Math on a constant value.

This method's result in this case, can be determined statically, and is faster and sometimes more accurate to just use the constant.

Methods detected are:

Method     Parameter

abs     -any-

acos    0.0 or 1.0

asin    0.0 or 1.0

atan    0.0 or 1.0

atan2   0.0

cbrt    0.0 or 1.0

ceil    -any-

cos     0.0

cosh    0.0

exp     0.0 or 1.0

expm1   0.0

floor   -any-

log     0.0 or 1.0

log10   0.0 or 1.0

rint    -any-

round   -any-

sin     0.0

sinh    0.0

sqrt    0.0 or 1.0

tan     0.0

tanh    0.0

toDegrees   0.0 or 1.0

toRadians   0.0

Private method is never called

This private method is never called.

Although it is possible that the method will be invoked through reflection, it is more likely that the method is never used, and should be removed.

Unread field

This field is never read.

Consider removing it from the class.

Unused field

This field is never used.

Consider removing it from the class.

Inefficient use of keySet iterator instead of entrySet iterator

This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator.

It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.