bruno-roustant commented on code in PR #1482:
URL: https://github.com/apache/solr/pull/1482#discussion_r1145997060
##########
gradle/validation/forbidden-apis/java.solr.txt:
##########
@@ -18,3 +18,12 @@ java.lang.Thread#<init>()
java.lang.Thread#<init>(java.lang.Runnable)
java.lang.Thread#<init>(java.lang.ThreadGroup,java.lang.Runnable)
+@defaultMessage Use CollectionUtil.newHashMap(int)
Review Comment:
+1
##########
solr/core/src/java/org/apache/solr/search/join/ScoreModeParser.java:
##########
@@ -29,7 +29,7 @@ class ScoreModeParser {
private ScoreModeParser() {}
private static Map<String, ScoreMode> getLowerAndCapitalCaseMap() {
- Map<String, ScoreMode> map = new HashMap<>(ScoreMode.values().length * 2);
+ Map<String, ScoreMode> map =
CollectionUtil.newHashMap(ScoreMode.values().length * 2);
Review Comment:
I suspect here it should be newHashSet(ScoreMode.values().length) (the
author doubled, knowing he/she was setting a map capacity)
##########
solr/core/src/java/org/apache/solr/search/facet/UniqueAgg.java:
##########
@@ -85,7 +85,7 @@ public void merge(Object facetResult, Context mcontext) {
List<?> vals = (List<?>) map.get(VALS);
if (vals != null) {
if (values == null) {
- values = new HashSet<>(vals.size() * 4);
+ values = CollectionUtil.newHashSet(vals.size() * 4);
Review Comment:
When I see * 4 or * 2, I wonder if the author knew it was a capacity and
decided to double the expected size.
I mean, maybe here it should be newHashSet(vals.size() * 2)?
##########
solr/solrj/src/java/org/apache/solr/common/util/Utils.java:
##########
@@ -110,8 +110,8 @@ public static Map getDeepCopy(Map<?, ?> map, int maxDepth,
boolean mutable, bool
} else {
copy =
map instanceof LinkedHashMap
- ? new LinkedHashMap<>(map.size())
- : new HashMap<>(map.size());
+ ? CollectionUtil.newLinkedHashMap(map.size())
Review Comment:
These methods in Utils seem very related to the new CollectionUtil, and I
prefer the name CollectionUtil.
Are they often used? deepCopy does not seem to be a common use case. If not,
I suggest you could move them to CollectionUtil (without too much refactoring
impact on other classes).
##########
solr/solrj/src/java/org/apache/solr/common/util/CommandOperation.java:
##########
@@ -223,7 +223,7 @@ public static List<CommandOperation> parse(InputStream in,
Set<String> singleton
List<CommandOperation> operations = new ArrayList<>();
final HashMap<Object, Object> map =
- new HashMap<>(0) {
+ new HashMap<>() {
Review Comment:
+1 to not keep 0. Anyway it is replaced internally by a power of 2.
##########
solr/solrj/src/java/org/apache/solr/common/util/CollectionUtil.java:
##########
@@ -39,6 +41,17 @@ public static <K, V> HashMap<K, V> newHashMap(int size) {
return new HashMap<>((int) (size / 0.75f) + 1);
}
+ /**
+ * Returns a new {@link LinkedHashMap} sized to contain {@code size} items
without resizing the
+ * internal array.
+ */
+ public static <K, V> LinkedHashMap<K, V> newLinkedHashMap(int size) {
+ // With Lucene 9.5 - we should replace this with
Review Comment:
Comment to finish?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
