SOLR-12699: Make contrib/ltr LTRScoringModel immutable and cache its hashCode.

(Stanislav Livotov, Edward Ribeiro, Christine Poerschke)

solr/CHANGES.txt

@@ -178,6 +178,9 @@ Improvements
 
 * SOLR-12892: MapWriter to use CharSequence instead of String (noble)
 
+* SOLR-12699: Make contrib/ltr LTRScoringModel immutable and cache its hashCode.
+  (Stanislav Livotov, Edward Ribeiro, Christine Poerschke)
+
 ==================  7.5.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

solr/contrib/ltr/src/java/org/apache/solr/ltr/model/DefaultWrapperModel.java

@@ -52,6 +52,11 @@ import org.noggit.ObjectBuilder;
  */
 public class DefaultWrapperModel extends WrapperModel {
 
+  /**
+   * resource is part of the LTRScoringModel params map
+   * and therefore here it does not individually
+   * influence the class hashCode, equals, etc.
+   */
   private String resource;
 
   public DefaultWrapperModel(String name, List<Feature> features, List<Normalizer> norms, String featureStoreName,

solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LTRScoringModel.java

@@ -16,11 +16,14 @@
  */
 package org.apache.solr.ltr.model;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Explanation;
@@ -81,6 +84,7 @@ public abstract class LTRScoringModel {
   private final List<Feature> allFeatures;
   private final Map<String,Object> params;
   protected final List<Normalizer> norms;
+  private Integer hashCode; // cached since it shouldn't actually change after construction
 
   public static LTRScoringModel getInstance(SolrResourceLoader solrResourceLoader,
       String className, String name, List<Feature> features,
@@ -111,11 +115,11 @@ public abstract class LTRScoringModel {
       String featureStoreName, List<Feature> allFeatures,
       Map<String,Object> params) {
     this.name = name;
-    this.features = features;
+    this.features = features != null ? Collections.unmodifiableList(new ArrayList<>(features)) : null;
     this.featureStoreName = featureStoreName;
-    this.allFeatures = allFeatures;
-    this.params = params;
-    this.norms = norms;
+    this.allFeatures = allFeatures != null ? Collections.unmodifiableList(new ArrayList<>(allFeatures)) : null;
+    this.params = params != null ? Collections.unmodifiableMap(new LinkedHashMap<>(params)) : null;
+    this.norms = norms != null ? Collections.unmodifiableList(new ArrayList<>(norms)) : null;
   }
 
   /**
@@ -144,7 +148,7 @@ public abstract class LTRScoringModel {
    * @return the norms
    */
   public List<Normalizer> getNorms() {
-    return Collections.unmodifiableList(norms);
+    return norms;
   }
 
   /**
@@ -158,7 +162,7 @@ public abstract class LTRScoringModel {
    * @return the features
    */
   public List<Feature> getFeatures() {
-    return Collections.unmodifiableList(features);
+    return features;
   }
 
   public Map<String,Object> getParams() {
@@ -167,13 +171,20 @@ public abstract class LTRScoringModel {
 
   @Override
   public int hashCode() {
+    if(hashCode == null) {
+      hashCode = calculateHashCode();
+    }
+    return hashCode;
+  }
+
+  final private int calculateHashCode() {
     final int prime = 31;
     int result = 1;
-    result = (prime * result) + ((features == null) ? 0 : features.hashCode());
-    result = (prime * result) + ((name == null) ? 0 : name.hashCode());
-    result = (prime * result) + ((params == null) ? 0 : params.hashCode());
-    result = (prime * result) + ((norms == null) ? 0 : norms.hashCode());
-    result = (prime * result) + ((featureStoreName == null) ? 0 : featureStoreName.hashCode());
+    result = (prime * result) + Objects.hashCode(features);
+    result = (prime * result) + Objects.hashCode(name);
+    result = (prime * result) + Objects.hashCode(params);
+    result = (prime * result) + Objects.hashCode(norms);
+    result = (prime * result) + Objects.hashCode(featureStoreName);
     return result;
   }
 

solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LinearModel.java

@@ -71,6 +71,11 @@ import org.apache.solr.ltr.norm.Normalizer;
  */
 public class LinearModel extends LTRScoringModel {
 
+  /**
+   * featureToWeight is part of the LTRScoringModel params map
+   * and therefore here it does not individually
+   * influence the class hashCode, equals, etc.
+   */
   protected Float[] featureToWeight;
 
   public void setWeights(Object weights) {

solr/contrib/ltr/src/java/org/apache/solr/ltr/model/MultipleAdditiveTreesModel.java

@@ -90,7 +90,18 @@ import org.apache.solr.util.SolrPluginUtils;
  */
 public class MultipleAdditiveTreesModel extends LTRScoringModel {
 
+  /**
+   * fname2index is filled from constructor arguments
+   * (that are already part of the base class hashCode)
+   * and therefore here it does not individually
+   * influence the class hashCode, equals, etc.
+   */
   private final HashMap<String,Integer> fname2index;
+  /**
+   * trees is part of the LTRScoringModel params map
+   * and therefore here it does not individually
+   * influence the class hashCode, equals, etc.
+   */
   private List<RegressionTree> trees;
 
   private RegressionTree createRegressionTree(Map<String,Object> map) {

solr/contrib/ltr/src/java/org/apache/solr/ltr/model/NeuralNetworkModel.java

@@ -95,6 +95,11 @@ import org.apache.solr.util.SolrPluginUtils;
  */
 public class NeuralNetworkModel extends LTRScoringModel {
 
+  /**
+   * layers is part of the LTRScoringModel params map
+   * and therefore here it does not individually
+   * influence the class hashCode, equals, etc.
+   */
   private List<Layer> layers;
 
   protected interface Activation {

solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestAdapterModel.java

@@ -93,7 +93,17 @@ public class TestAdapterModel extends TestRerankBase {
 
   public static class CustomModel extends AdapterModel {
 
+    /**
+     * answerFileName is part of the LTRScoringModel params map
+     * and therefore here it does not individually
+     * influence the class hashCode, equals, etc.
+     */
     private String answerFileName;
+    /**
+     * answerValue is obtained from answerFileName
+     * and therefore here it does not individually
+     * influence the class hashCode, equals, etc.
+     */
     private float answerValue;
 
     public CustomModel(String name, List<Feature> features, List<Normalizer> norms, String featureStoreName,