Fix container inefficiencies in FieldInfos.java #13254
Conversation
| @@ -165,8 +164,7 @@ public FieldInfos(FieldInfo[] infos) { | |||
| valuesTemp.add(byNumberTemp[i]); | |||
| } | |||
| } | |||
| values = | |||
| Collections.unmodifiableCollection(Arrays.asList(valuesTemp.toArray(new FieldInfo[0]))); | |||
| values = Collections.unmodifiableCollection(valuesTemp); | |||
There was a problem hiding this comment.
This essentially trades RAM for CPU, as we are saving a copy but the values list may now be oversized. This doesn't obviously look like a better trade-off to me?
There was a problem hiding this comment.
How about counting the number of non-null infos in the for loop at line 86 and converting valuesTemp into a fixed-size array to store them?
There was a problem hiding this comment.
How about counting the number of non-null infos in the for loop at line 86 and converting
valuesTempinto a fixed-size array to store them?
Or use that non-null infos count to set the initial capacity of valuesTemp.
There was a problem hiding this comment.
If we pre-size valuesTemp correctly, it looks like it would work indeed.
There was a problem hiding this comment.
If we pre-size
valuesTempcorrectly, it looks like it would work indeed.
We observe that the info will never be null since the if statements already visit info.number here at line 87. Therefore, we pre-size valuesTemp to infos.length.
| maxFreqForLowerNorms = maxFreq; | ||
| } | ||
| } | ||
| return Collections.unmodifiableSet(freqNormPairs); | ||
| } |
There was a problem hiding this comment.
This change is optimizing the uncommon path (when otherFreqNormPairs is not empty) at the expense of additional code duplication. This doesn't look like a good trade-off to me.
Could you share a pointer to this tool? I'm curious how it works... thanks. |
Thank you for your interest in cinst! Our tool has not been released yet. Once cinst is open-sourced, we will inform you at first. |
Description
Hi,
We find that there exist container inefficiencies in FieldInfos.java.
At line 169, there maybe a redundant operation that
Arrays.asList(valuesTemp.toArray(new FieldInfo[0]))transform List objectvaluesTempto array and then back to List.We discovered the above containers inefficiencies by our tool cinst. The patch is submitted. Could you please check and accept it? We have tested the patch on our PC. The patched program works well.