[java] Update OptimizableToArrayCall rule for current JVMs

See https://sourceforge.net/p/pmd/bugs/1454/
and https://shipilev.net/blog/2016/arrays-wisdom-ancients/
This commit is contained in:
Andreas Dangel
2017-12-01 11:41:23 +01:00
parent 345cc50f78
commit 07cb937d6e
3 changed files with 53 additions and 21 deletions

View File

@ -252,6 +252,11 @@ The rule reference documentation has been updated to reflect these changes.
* The Java rule `EmptyCatchBlock` (category `errorprone`, former ruleset `java-empty`) has been changed to ignore
exceptions named `ignore` or `expected` by default. You can still override this behaviour by setting the `allowExceptionNameRegex` property.
* The Java rule `OptimizableToArrayCall` (category `performance`, former ruleset `design`) has been
modified to fit for the current JVM implementations: It basically detects now the opposite and suggests to
use `Collection.toArray(new E[0])` with a zero-sized array.
See [Arrays of Wisdom of the Ancients](https://shipilev.net/blog/2016/arrays-wisdom-ancients/).
#### Deprecated Rules
* The Java rules `NcssConstructorCount`, `NcssMethodCount`, and `NcssTypeCount` (ruleset `java-codesize`) have been
@ -339,6 +344,7 @@ a warning will now be produced suggesting users to adopt it for better performan
* cpp
* [#448](https://github.com/pmd/pmd/issues/448): \[cpp] Write custom CharStream to handle continuation characters
* java
* [#1454](https://sourceforge.net/p/pmd/bugs/1454/): \[java] OptimizableToArrayCall is outdated and invalid in current JVMs
* [#1513](https://sourceforge.net/p/pmd/bugs/1513/): \[java] Remove deprecated rule UseSingleton
* [#328](https://github.com/pmd/pmd/issues/328): \[java] java.lang.ClassFormatError: Absent Code attribute in method that is not native or abstract in class file javax/servlet/jsp/PageContext
* [#487](https://github.com/pmd/pmd/pull/487): \[java] Fix typeresolution for anonymous extending object

View File

@ -428,9 +428,17 @@ public class Foo {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#optimizabletoarraycall">
<description>
Calls to a collection's toArray() method should specify target arrays sized to match the size of the
collection. Initial arrays that are too small are discarded in favour of new ones that have to be created
that are the proper size.
Calls to a collection's `toArray(E[])` method should specify a target array of zero size. This allows the JVM
to optimize the memory allocation and copying as much as possible.
Previous versions of this rule (pre PMD 6.0.0) suggested the opposite, but current JVM implementations
perform always better, when they have full control over the target array. And allocation an array via
reflection is nowadays as fast as the direct allocation.
See also [Arrays of Wisdom of the Ancients](https://shipilev.net/blog/2016/arrays-wisdom-ancients/)
Note: If you don't need an array of the correct type, then the simple `toArray()` method without an array
is faster, but returns only an array of type `Object[]`.
</description>
<priority>3</priority>
<properties>
@ -442,7 +450,7 @@ that are the proper size.
[
PrimarySuffix/Arguments/ArgumentList/Expression
/PrimaryExpression/PrimaryPrefix/AllocationExpression
/ArrayDimsAndInits/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image='0']
/ArrayDimsAndInits/Expression/PrimaryExpression/PrimaryPrefix[not(Literal[@Image='0'])]
]
]]>
</value>
@ -450,13 +458,13 @@ PrimarySuffix/Arguments/ArgumentList/Expression
</properties>
<example>
<![CDATA[
List foos = getFoos();
List<Foo> foos = getFoos();
// inefficient, the array will be discarded
// much better; this one allows the jvm to allocate an array of the correct size and effectively skip
// the zeroing, since each array element will be overridden anyways
Foo[] fooArray = foos.toArray(new Foo[0]);
// much better; this one sizes the destination array,
// avoiding of a new one via reflection
// inefficient, the array needs to be zeroed out by the jvm before it is handed over to the toArray method
Foo[] fooArray = foos.toArray(new Foo[foos.size()]);
]]>
</example>

View File

@ -4,10 +4,8 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description><![CDATA[
failure case
]]></description>
<expected-problems>1</expected-problems>
<description>Preferred usage (sf #1454)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
{x.toArray(new Foo[0]);}
@ -15,10 +13,8 @@ public class Foo {
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Array dimensioner uses method call, ok
]]></description>
<expected-problems>0</expected-problems>
<description>Array dimensioner uses method call, performance issue</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
{x.toArray(new Foo[x.size()]);}
@ -26,10 +22,8 @@ public class Foo {
]]></code>
</test-code>
<test-code>
<description><![CDATA[
Array dimensioner uses variable, ok
]]></description>
<expected-problems>0</expected-problems>
<description>Array dimensioner uses variable, performance issue</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
{x.toArray(new Foo[y]);}
@ -39,12 +33,36 @@ public class Foo {
<test-code>
<description>#937 OptimizableToArrayCall does not catch multilevel method chains</description>
<expected-problems>1</expected-problems>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Test {
public void foo() {
x.y.toArray(new Foo[0]);
}
}
]]></code>
</test-code>
<test-code>
<description>Array with a literal dimension, zero still better</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public void foo() {
x.toArray(new Foo[10]);
}
}
]]></code>
</test-code>
<test-code>
<description>toArray call without an array should not be flagged</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void foo() {
x.toArray();
}
}
]]></code>
</test-code>