Inefficient Usages of Java Collections #1125
Conversation
The first five lists can be replaced by numeric variables further.
|
|
| @@ -59,7 +58,7 @@ | |||
| */ | |||
| public GlowHelpMap(GlowServer server) { | |||
| this.server = server; | |||
| helpTopics = new TreeMap<>(NAME_COMPARE); | |||
| helpTopics = new HashMap<>(); | |||
There was a problem hiding this comment.
This is iterated upon in Bukkit (a Maven dependency of ours, see net.glowstone.glowkit), in HelpCommand.
| @@ -157,8 +157,7 @@ public static Library fromConfigMap(Map<?, ?> configMap) { | |||
| * @return A map that is able to be serialized into a config. | |||
| */ | |||
| public Map<?, ?> toConfigMap() { | |||
| // Using LinkedHashMap to keep the props in order when written into the config file. | |||
| Map<String, Object> configMap = new LinkedHashMap<>(); | |||
| Map<String, Object> configMap = new HashMap<>(); | |||
There was a problem hiding this comment.
This isn't called within Glowstone (yet?), but the interface guarantees ordering for a config file.
There was a problem hiding this comment.
Yeah. Our tool finds that the order is actually not used because the map object is not traversed. Of course, you can maintain it as a LinkedHashMap if you want to assure the extensibility of the program. For the current version, the HashMap is a better choice.
| @@ -25,6 +27,8 @@ public boolean generate(World world, Random random, int sourceX, int sourceY, in | |||
| } | |||
| boolean succeeded = false; | |||
| int stemRadius = Math.max(0, Math.min(MAX_STEM_RADIUS, tipRadius - 1)); | |||
| Set<Material> materialSet = new HashSet<>(Arrays.asList(MATERIALS)); | |||
There was a problem hiding this comment.
Would much rather have the MATERIALS variable be directly converted to a HashSet.
| @@ -45,6 +47,7 @@ protected final void setLeavesHeight(int leavesHeight) { | |||
|
|
|||
| @Override | |||
| public boolean canPlace(int baseX, int baseY, int baseZ, World world) { | |||
| Set<Material> overridableSet = new HashSet<>(overridables); | |||
There was a problem hiding this comment.
I think we should convert the base class' overridables to a HashSet, to prevent allocations every call.
There was a problem hiding this comment.
You can redesign the class to achieve better optimization. Currently, such allocation also improves the efficiency of the single function indeed.
|
Also, can the CLA be signed by @DittoTool? |
Hi,
We find that there are several inefficient usages of Java Collections:
We discovered the above inefficient usage of containers by our tool Ditto. The patch is submitted. Could you please check and accept it? We have tested the patch on our PC. The patched program works well.
Bests
Ditto