Forbid easily misused HashSet and HashMap constructors #9165
Conversation
|
Edit: Not sure what triggered things to run again, but build appears to be green now 🤷♂ The travis-ci logs for the failed build steps all appear to be a 5XX error pulling external resources. Any way to get the build restarted short of pushing a dummy commit? |
|
@capistrant I restarted the failed runs earlier, was going to comment but got pulled away for a moment |
| java.util.HashSet#<init>(int) @ Use com.google.collect.Sets#newHashSetWithExpectedSize(int) instead | ||
| java.util.HashSet#<init>(int, float) @ Use com.google.collect.Sets#newHashSetWithExpectedSize(int) instead | ||
| java.util.LinkedHashSet#<init>(int) @ Use com.google.collect.Sets#newLinkedHashSatWithExpectedSize(int) instead | ||
| java.util.LinkedHashSet#<init>(int, float) @ Use com.google.collect.Sets#newLinkedHashSatWithExpectedSize(int) instead |
There was a problem hiding this comment.
LinkedHashMap should be prohibited, too, even though there is no method ready in Guava. A polyfill method could be added to CollectionUtils.
…ty method as replacement for them
| @@ -39,6 +41,8 @@ | |||
|
|
|||
| public final class CollectionUtils | |||
| { | |||
| public static final int MAX_EXPECTED_SIZE = (1 << 30); | |||
There was a problem hiding this comment.
Nit: should better be private.
There was a problem hiding this comment.
Thanks for catching that
|
Looks like a non-related TravisCI failure due to external dependency pulls |
…e proper sizing is used
…ckstyle during CI
| @@ -1341,6 +1341,9 @@ | |||
| <signaturesFile>${project.parent.basedir}/codestyle/joda-time-forbidden-apis.txt</signaturesFile> | |||
| <signaturesFile>${project.parent.basedir}/codestyle/druid-forbidden-apis.txt</signaturesFile> | |||
| </signaturesFiles> | |||
| <excludes> | |||
| <exclude>**/SomeAvroDatum.class</exclude> | |||
There was a problem hiding this comment.
I've created https://issues.apache.org/jira/browse/AVRO-2731 and #9331 to track this. FYI @Fokko.
@capistrant please try to create such issues yourself to close the loops and offload info from the heads of people.
Fixes #8423
Description
This PR adds constructors for HashMap, HashSet, and LinkedHashSet that specify an initial size parameter to the forbiddenapi list. It replaces all usage of these constructors with Guava helper methods that take the users parameter for size and create a more optimally sized object for them compared to what they get from the native constructors.
HashMap, HashSet and LinkedHashSet objects are often created in an inefficient way when the creator specifies a size. Guava has created utilities to take the size desired by the creator and come up with a proper initial size for the underlying object for them. It is explained more clearly here. The same is also true for LinkedHashMap, but unfortunately we are still using Guava 16, and the Guava implementation for LinkedHashMaps did not come about until Guava 19.
I had to add an ignores item in the pom.xml file for
SomeAvroDatum.classbecause it is generated at build time so it can't have a suppression annotation inline. If there is a better way to exclude that I'm not thinking of, please let me know.This PR has:
Key changed/added classes in this PR
Many classes were modified, but the changes are all transparent. Just changing the way Map and Set objects are created in some specific cases.