Merge pull request #10790 from vespa-engine/bratseth/handle-nan-in-grouping

Bratseth/handle nan in grouping

17 files changed, 110 insertions, 131 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java
index ee64632d40a..93a21476f35 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java
@@ -10,6 +10,7 @@ import java.util.Comparator;
 import java.util.List;
 
 public class SortDataHitSorter {
+
     public static void sort(HitGroup hitGroup, List<Hit> hits) {
         var sorting = hitGroup.getQuery().getRanking().getSorting();
         var fallbackOrderer = hitGroup.getOrderer();
@@ -61,4 +62,5 @@ public class SortDataHitSorter {
             return fallback.compare(left, right);
         }
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/Group.java b/container-search/src/main/java/com/yahoo/search/grouping/result/Group.java
index 2a1e3199d7d..30bf0ff4bb0 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/Group.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/Group.java
@@ -14,7 +14,6 @@ import com.yahoo.search.result.Relevance;
  */
 public class Group extends HitGroup {
 
-    private static final long serialVersionUID = 2122928012157537800L;
     private final GroupId groupId;
 
     /**
@@ -80,4 +79,5 @@ public class Group extends HitGroup {
         }
         return null;
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/HitList.java b/container-search/src/main/java/com/yahoo/search/grouping/result/HitList.java
index 03e0f7e6d59..653fc4cb978 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/HitList.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/HitList.java
@@ -6,20 +6,21 @@ import com.yahoo.search.grouping.GroupingRequest;
 import com.yahoo.search.result.Hit;
 
 /**
- * <p>This class represents a labeled hit list in the grouping result model. It is contained in {@link Group}, and
+ * This class represents a labeled hit list in the grouping result model. It is contained in {@link Group}, and
  * contains one or more {@link Hit hits} itself, making this the parent of leaf nodes in the hierarchy of grouping
- * results. Use the {@link GroupingRequest#getResultGroup(Result)} to retrieve grouping results.</p>
+ * results. Use the {@link GroupingRequest#getResultGroup(Result)} to retrieve grouping results.
  *
  * @author Simon Thoresen Hult
  */
 public class HitList extends AbstractList {
 
     /**
-     * <p>Constructs a new instance of this class.</p>
+     * Constructs a new instance of this class.
      *
-     * @param label The label to assign to this.
+     * @param label the label to assign to this
      */
     public HitList(String label) {
         super("hitlist", label);
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java b/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java
index 3907f87a276..da115807fc2 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java
@@ -11,7 +11,7 @@ import java.util.Arrays;
 import java.util.Map;
 
 /**
- * This is a helper class for rendering grouping results.
+ * A helper for rendering grouping results.
  *
  * @author Simon Thoresen Hult
  */
@@ -94,4 +94,5 @@ public abstract class HitRenderer {
     private static void renderContinuation(String id, Continuation continuation, XMLWriter writer) {
         writer.openTag(TAG_CONTINUATION).attribute(TAG_CONTINUATION_ID, id).content(continuation, false).closeTag();
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/HitConverter.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/HitConverter.java
index e8f4d566028..5ea0e3a0838 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/HitConverter.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/HitConverter.java
@@ -44,7 +44,8 @@ class HitConverter implements ResultBuilder.HitConverter {
     }
 
     private Hit convertFs4Hit(String summaryClass, FS4Hit groupHit) {
-        FastHit hit = new FastHit(groupHit.getGlobalId().getRawId(), new Relevance(groupHit.getRank()),
+        FastHit hit = new FastHit(groupHit.getGlobalId().getRawId(),
+                                  new Relevance(groupHit.getRank()),
                                   groupHit.getPath(), groupHit.getDistributionKey());
         hit.setFillable();
         hit.setSearcherSpecificMetaData(searcher, summaryClass);
@@ -72,4 +73,5 @@ class HitConverter implements ResultBuilder.HitConverter {
         }
         return ret;
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java
index 2402be27b9c..f45617d1cd7 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java
@@ -285,6 +285,7 @@ class ResultBuilder {
             page.putContinuations(hitList.continuations());
             return hitList;
         }
+
     }
 
     private class GroupListBuilder {
@@ -341,6 +342,7 @@ class ResultBuilder {
             }
             return ret;
         }
+
     }
 
     private class PageInfo {
@@ -381,16 +383,17 @@ class ResultBuilder {
                 }
             }
         }
+
     }
 
     /**
-     * Defines a helper interface to convert Vespa style grouping hits into corresponding instances of {@link Hit}. It
-     * is an interface to simplify testing.
-     *
-     * @author Simon Thoresen Hult
+     * Defines a helper interface to convert Vespa style grouping hits into corresponding instances of {@link Hit}.
+     * It is an interface to simplify testing.
      */
     public interface HitConverter {
 
-        public com.yahoo.search.result.Hit toSearchHit(String summaryClass, com.yahoo.searchlib.aggregation.Hit hit);
+        com.yahoo.search.result.Hit toSearchHit(String summaryClass, com.yahoo.searchlib.aggregation.Hit hit);
+
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
index 64db1cf0062..1c58081e4f1 100644
--- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
+++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java
@@ -394,27 +394,29 @@ public class SearchHandler extends LoggingRequestHandler {
             log.log(LogLevel.DEBUG, () -> error.getDetailedMessage());
             return new Result(query, error);
         } catch (IllegalArgumentException e) {
-            ErrorMessage error = ErrorMessage.createBadRequest("Invalid search request [" + request + "]: "
-                                                               + Exceptions.toMessageString(e));
-            log.log(LogLevel.DEBUG, () -> error.getDetailedMessage());
-            return new Result(query, error);
-        } catch (LinkageError e) {
-            // Should have been an Exception in an OSGi world - typical bundle dependency issue problem
-            ErrorMessage error = ErrorMessage.createErrorInPluginSearcher(
-                            "Error executing " + searchChain + "]: " + Exceptions.toMessageString(e), e);
-            log(request, query, e);
-            return new Result(query, error);
-        } catch (StackOverflowError e) { // Also recoverable
-            ErrorMessage error = ErrorMessage.createErrorInPluginSearcher(
-                            "Error executing " + searchChain + "]: " + Exceptions.toMessageString(e), e);
+            if ("Comparison method violates its general contract!".equals(e.getMessage())) {
+                // This is an error in application components or Vespa code
+                log(request, query, e);
+                return new Result(query, ErrorMessage.createUnspecifiedError("Failed searching: " +
+                                                                             Exceptions.toMessageString(e), e));
+            }
+            else {
+                ErrorMessage error = ErrorMessage.createBadRequest("Invalid search request [" + request + "]: "
+                                                                   + Exceptions.toMessageString(e));
+                log.log(LogLevel.DEBUG, () -> error.getDetailedMessage());
+                return new Result(query, error);
+            }
+        } catch (LinkageError | StackOverflowError e) {
+            // LinkageError should have been an Exception in an OSGi world - typical bundle dependency issue problem
+            // StackOverflowError is recoverable
+            ErrorMessage error = ErrorMessage.createErrorInPluginSearcher("Error executing " + searchChain + "]: " +
+                                                                          Exceptions.toMessageString(e), e);
             log(request, query, e);
             return new Result(query, error);
         } catch (Exception e) {
-            Result result = new Result(query);
             log(request, query, e);
-            result.hits().addError(
-                    ErrorMessage.createUnspecifiedError("Failed searching: " + Exceptions.toMessageString(e), e));
-            return result;
+            return new Result(query, ErrorMessage.createUnspecifiedError("Failed searching: " +
+                                                                         Exceptions.toMessageString(e), e));
         }
     }
 
@@ -448,12 +450,10 @@ public class SearchHandler extends LoggingRequestHandler {
     private void log(String request, Query query, Throwable e) {
         // Attempted workaround for missing stack traces
         if (e.getStackTrace().length == 0) {
-            log.log(LogLevel.ERROR,
-                    "Failed executing " + query.toDetailString() + " [" + request
-                            + "], received exception with no context", e);
+            log.log(LogLevel.ERROR, "Failed executing " + query.toDetailString() +
+                                    " [" + request + "], received exception with no context", e);
         } else {
-            log.log(LogLevel.ERROR,
-                    "Failed executing " + query.toDetailString() + " [" + request + "]", e);
+            log.log(LogLevel.ERROR, "Failed executing " + query.toDetailString() + " [" + request + "]", e);
         }
     }
 
diff --git a/container-search/src/main/java/com/yahoo/search/result/HitOrderer.java b/container-search/src/main/java/com/yahoo/search/result/HitOrderer.java
index 331a9ab5852..848666d628b 100644
--- a/container-search/src/main/java/com/yahoo/search/result/HitOrderer.java
+++ b/container-search/src/main/java/com/yahoo/search/result/HitOrderer.java
@@ -9,15 +9,13 @@ import java.util.List;
  *
  * @author bratseth
  */
-
 public abstract class HitOrderer {
 
     /** Orders the given list of hits */
     public abstract void order(List<Hit> hits);
 
     /**
-     * Returns the Comparator that this HitOrderer uses internally to
-     * sort hits. Returns null if no Comparator is used.
+     * Returns the Comparator that this HitOrderer uses internally to sort hits. Returns null if no Comparator is used.
      * <p>
      * This default implementation returns null.
      *
diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java
index b660d90072b..dfc603846fd 100644
--- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java
+++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java
@@ -1039,7 +1039,7 @@ public class YqlParser implements Parser {
         assertHasOperator(ast, ExpressionOperator.CONTAINS);
         String field = getIndex(ast.getArgument(0));
         if (userQuery != null && indexFactsSession.getIndex(field).isAttribute()) {
-            userQuery.trace("Field '" + field + "' is an attribute, 'contains' will only match exactly", 1);
+            userQuery.trace("Field '" + field + "' is an attribute, 'contains' will only match exactly", 2);
         }
         return instantiateLeafItem(field, ast.<OperatorNode<ExpressionOperator>> getArgument(1));
     }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/AggregationResult.java b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/AggregationResult.java
index 264a9d4d4e9..6858ebb8f82 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/AggregationResult.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/AggregationResult.java
@@ -8,7 +8,7 @@ import com.yahoo.vespa.objects.ObjectVisitor;
 import com.yahoo.vespa.objects.Serializer;
 
 /**
- * <p>This is the aggregation super-class from which all types of aggregation inherits.</p>
+ * The result of some aggregation
  *
  * @author baldersheim
  * @author Simon Thoresen Hult
@@ -20,7 +20,7 @@ public abstract class AggregationResult extends ExpressionNode {
     private int tag = -1;
 
     /**
-     * <p>Returns the tag of this aggregation result. This is useful for uniquely identifying a result.</p>
+     * Returns the tag of this aggregation result. This is useful for uniquely identifying a result.
      *
      * @return The numerical tag.
      */
@@ -29,10 +29,10 @@ public abstract class AggregationResult extends ExpressionNode {
     }
 
     /**
-     * <p>Assigns a tag to this group.</p>
+     * Assigns a tag to this group.
      *
-     * @param tag The numerical tag to set.
-     * @return This, to allow chaining.
+     * @param tag the numerical tag to set.
+     * @return this, to allow chaining.
      */
     public AggregationResult setTag(int tag) {
         this.tag = tag;
@@ -40,55 +40,47 @@ public abstract class AggregationResult extends ExpressionNode {
     }
 
     /**
-     * <p>This method is called when merging aggregation results. This method is simply a proxy for the abstract {@link
-     * #onMerge(AggregationResult)} method.</p>
+     * Called when merging aggregation results. This method is simply a proxy for the abstract {@link
+     * #onMerge(AggregationResult)} method.
      *
-     * @param result The result to merge with.
+     * @param result the result to merge with.
      */
     public void merge(AggregationResult result) {
         onMerge(result);
     }
 
     /**
-     * <p>This method is called when all aggregation results have been merged. This method can be overloaded by
-     * subclasses that need special behaviour to occur after merge.</p>
+     * Hook called when all aggregation results have been merged. This method can be overloaded by
+     * subclasses that need special behaviour to occur after merge.
      */
     public void postMerge() {
         // empty
     }
 
-    /**
-     * <p>This method returns a value that can be used for ranking.</p>
-     *
-     * @return The rankable result.
-     */
+    /** Returns a value that can be used for ranking. */
     public abstract ResultNode getRank();
 
     /**
-     * <p>Sets the expression to aggregate on.</p>
+     * Sets the expression to aggregate on.
      *
-     * @param exp The expression.
-     * @return This, to allow chaining.
+     * @param exp the expression
+     * @return this, to allow chaining
      */
     public AggregationResult setExpression(ExpressionNode exp) {
         expression = exp;
         return this;
     }
 
-    /**
-     * <p>Returns the expression to aggregate on.</p>
-     *
-     * @return The expression.
-     */
+    /** Returns the expression to aggregate on. */
     public ExpressionNode getExpression() {
         return expression;
     }
 
     /**
-     * <p>This method must be implemented by subclasses to support merge. It is called as the {@link
-     * #merge(AggregationResult)} method is invoked.</p>
+     * Mmust be implemented by subclasses to support merge. It is called as the {@link
+     * #merge(AggregationResult)} method is invoked.
      *
-     * @param result The result to merge with.
+     * @param result the result to merge with
      */
     protected abstract void onMerge(AggregationResult result);
 
@@ -158,4 +150,5 @@ public abstract class AggregationResult extends ExpressionNode {
         visitor.visit("expression", expression);
         visitor.visit("tag", tag);
     }
+
 }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/FS4Hit.java b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/FS4Hit.java
index 399ffd3128f..07de8bbdc55 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/FS4Hit.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/FS4Hit.java
@@ -7,7 +7,7 @@ import com.yahoo.vespa.objects.ObjectVisitor;
 import com.yahoo.vespa.objects.Serializer;
 
 /**
- * This class represents a single hit from the fastserver4 backend
+ * A single hit from a Vespa content cluster
  *
  * @author havardpe
  */
@@ -103,7 +103,7 @@ public class FS4Hit extends Hit {
         return super.hashCode() + path + globalId.hashCode() + distributionKey;
     }
 
-    @SuppressWarnings({ "EqualsWhichDoesntCheckParameterClass", "RedundantIfStatement" })
+    @SuppressWarnings("RedundantIfStatement")
     @Override
     public boolean equals(Object obj) {
         if (!super.equals(obj)) {
@@ -129,4 +129,5 @@ public class FS4Hit extends Hit {
         visitor.visit("globalId", globalId.toString());
         visitor.visit("distributionKey", distributionKey);
     }
+
 }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Group.java b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Group.java
index 73171f4dd00..c508296d739 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Group.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Group.java
@@ -12,28 +12,28 @@ public class Group extends Identifiable {
 
     public static final int classId = registerClass(0x4000 + 90, Group.class);
     private static final ObjectPredicate REF_LOCATOR = new RefLocator();
-    private List<Integer> orderByIdx = new ArrayList<Integer>();
-    private List<ExpressionNode> orderByExp = new ArrayList<ExpressionNode>();
-    private List<AggregationResult> aggregationResults = new ArrayList<AggregationResult>();
-    private List<Group> children = new ArrayList<Group>();
+    private List<Integer> orderByIdx = new ArrayList<>();
+    private List<ExpressionNode> orderByExp = new ArrayList<>();
+    private List<AggregationResult> aggregationResults = new ArrayList<>();
+    private List<Group> children = new ArrayList<>();
     private ResultNode id = null;
     private double rank;
     private int tag = -1;
     private SortType sortType = SortType.UNSORTED;
 
     /**
-     * <p>This tells you if the children are ranked by the pure relevance or by a more complex expression.  That
-     * indicates if the rank score from the child can be used for ordering.</p>
+     * This tells you if the children are ranked by the pure relevance or by a more complex expression.
+     * That indicates if the rank score from the child can be used for ordering.
      *
-     * @return True if it ranked by pure relevance.
+     * @return true if it ranked by pure relevance.
      */
     public boolean isRankedByRelevance() {
         return orderByIdx.isEmpty();
     }
 
     /**
-     * <p>Merges the content of the given group <b>into</b> this. When this function returns, make sure to call {@link
-     * #postMerge(java.util.List, int, int)}.</p>
+     * Merges the content of the given group <b>into</b> this. When this function returns, make sure to call
+     * {@link #postMerge(java.util.List, int, int)}.
      *
      * @param firstLevel   The first level to merge.
      * @param currentLevel The current level.
@@ -49,7 +49,7 @@ public class Group extends Identifiable {
             }
         }
 
-        ArrayList<Group> merged = new ArrayList<Group>();
+        ArrayList<Group> merged = new ArrayList<>();
         Iterator<Group> lhsChild = children.iterator(), rhsChild = rhs.children.iterator();
         if (lhsChild.hasNext() && rhsChild.hasNext()) {
             Group lhsGroup = lhsChild.next();
@@ -93,8 +93,8 @@ public class Group extends Identifiable {
     }
 
     /**
-     * <p>After merging, this method will prune all levels so that they do not exceed the configured maximum number of
-     * groups per level.</p>
+     * After merging, this method will prune all levels so that they do not exceed the configured maximum number of
+     * groups per level.
      *
      * @param levels       The specs of all grouping levels.
      * @param firstLevel   The first level to merge.
@@ -127,9 +127,7 @@ public class Group extends Identifiable {
 
     }
 
-    /**
-     * <p>Will sort the children by their id, if they are not sorted already.</p>
-     */
+    /** Sorts the children by their id, if they are not sorted already. */
     public void sortChildrenById() {
         if (sortType == SortType.BYID) {
             return;
@@ -142,9 +140,7 @@ public class Group extends Identifiable {
         sortType = SortType.BYID;
     }
 
-    /**
-     * <p>Will sort the children by their rank, if they are not sorted already.</p>
-     */
+    /** Sorts the children by their rank, if they are not sorted already. */
     public void sortChildrenByRank() {
         if (sortType == SortType.BYRANK) {
             return;
@@ -158,18 +154,16 @@ public class Group extends Identifiable {
     }
 
     /**
-     * <p>Returns the label to use for this group. See comment on {@link #setId(com.yahoo.searchlib.expression.ResultNode)}
-     * on the rationale of this being a {@link ResultNode}.</p>
-     *
-     * @return The label.
+     * Returns the label to use for this group. See comment on {@link #setId(com.yahoo.searchlib.expression.ResultNode)}
+     * on the rationale of this being a {@link ResultNode}.
      */
     public ResultNode getId() {
         return id;
     }
 
     /**
-     * <p>Sets the label to use for this group. This is a {@link ResultNode} so that a group can be labeled with
-     * whatever value the classifier expression returns.</p>
+     * Sets the label to use for this group. This is a {@link ResultNode} so that a group can be labeled with
+     * whatever value the classifier expression returns.
      *
      * @param id The label to set.
      * @return This, to allow chaining.
@@ -180,7 +174,7 @@ public class Group extends Identifiable {
     }
 
     /**
-     * <p>Sets the relevancy to use for this group.</p>
+     * Sets the relevancy to use for this group.
      *
      * @param rank The rank to set.
      * @return This, to allow chaining.
@@ -190,17 +184,13 @@ public class Group extends Identifiable {
         return this;
     }
 
-    /**
-     * <p>Return the relevancy of this group.</p>
-     *
-     * @return Relevance.
-     */
+    /** Return the rank score of this group. */
     public double getRank() {
         return rank;
     }
 
     /**
-     * <p>Adds a child group to this.</p>
+     * Adds a child group to this.
      *
      * @param child The group to add.
      * @return This, to allow chaining.
@@ -308,13 +298,7 @@ public class Group extends Identifiable {
         if (diff > 0) {
             return 1;
         }
-        if (rank > rhs.rank) {
-            return -1;
-        }
-        if (rank < rhs.rank) {
-            return 1;
-        }
-        return 0;
+        return -Double.compare(rank, rhs.rank);
     }
 
     @Override
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Hit.java b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Hit.java
index 6b2ce5c3b72..663bc18dc14 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Hit.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Hit.java
@@ -79,14 +79,14 @@ public abstract class Hit extends Identifiable {
         return super.hashCode() + (int)rank;
     }
 
-    @SuppressWarnings({ "RedundantIfStatement", "EqualsWhichDoesntCheckParameterClass" })
+    @SuppressWarnings({ "RedundantIfStatement" })
     @Override
     public boolean equals(Object obj) {
         if (!super.equals(obj)) {
             return false;
         }
         Hit rhs = (Hit)obj;
-        if (rank != rhs.rank) {
+        if (Double.compare(rank, rhs.rank) != 0) {
             return false;
         }
         if (!equals(context, rhs.context)) {
@@ -101,4 +101,5 @@ public abstract class Hit extends Identifiable {
         visitor.visit("rank", rank);
         visitor.visit("context", context);
     }
+
 }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/HitsAggregationResult.java b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/HitsAggregationResult.java
index 275f38f7350..63b2b881e01 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/HitsAggregationResult.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/HitsAggregationResult.java
@@ -73,8 +73,8 @@ public class HitsAggregationResult extends AggregationResult {
     /**
      * Sets the summary class of hits to collect.
      *
-     * @param summaryClass The summary class to collect.
-     * @return This, to allow chaining.
+     * @param summaryClass the summary class to collect.
+     * @return this, to allow chaining.
      */
     public HitsAggregationResult setSummaryClass(String summaryClass) {
         this.summaryClass = summaryClass;
@@ -84,8 +84,8 @@ public class HitsAggregationResult extends AggregationResult {
     /**
      * Sets the maximum number of hits to collect.
      *
-     * @param maxHits The number of hits to collect.
-     * @return This, to allow chaining.
+     * @param maxHits the number of hits to collect.
+     * @return this, to allow chaining.
      */
     public HitsAggregationResult setMaxHits(int maxHits) {
         this.maxHits = maxHits;
@@ -102,7 +102,7 @@ public class HitsAggregationResult extends AggregationResult {
     }
 
     /**
-     * Add a hit to this aggregation result
+     * Adds a hit to this aggregation result
      *
      * @param h the hit
      * @return this object
@@ -159,11 +159,7 @@ public class HitsAggregationResult extends AggregationResult {
 
     @Override
     public void postMerge() {
-        Collections.sort(hits, new Comparator<Hit>() {
-            public int compare(Hit lhs, Hit rhs) {
-                return (lhs.getRank() > rhs.getRank()) ? -1 : (lhs.getRank() < rhs.getRank()) ? 1 : 0;
-            }
-        });
+        hits.sort((lhs, rhs) -> -Double.compare(lhs.getRank(), rhs.getRank()));
         if ((maxHits >= 0) && (hits.size() > maxHits)) {
             hits = hits.subList(0, maxHits);
         }
@@ -172,15 +168,9 @@ public class HitsAggregationResult extends AggregationResult {
     @Override
     protected boolean equalsAggregation(AggregationResult obj) {
         HitsAggregationResult rhs = (HitsAggregationResult)obj;
-        if (!summaryClass.equals(rhs.summaryClass)) {
-            return false;
-        }
-        if (maxHits != rhs.maxHits) {
-            return false;
-        }
-        if (!hits.equals(rhs.hits)) {
-            return false;
-        }
+        if ( ! summaryClass.equals(rhs.summaryClass)) return false;
+        if (maxHits != rhs.maxHits) return false;
+        if ( ! hits.equals(rhs.hits)) return false;
         return true;
     }
 
@@ -215,4 +205,5 @@ public class HitsAggregationResult extends AggregationResult {
             hit.select(predicate, operation);
         }
     }
+
 }
diff --git a/searchlib/src/test/java/com/yahoo/searchlib/aggregation/GroupingTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/aggregation/GroupingTestCase.java
index fe5405ecb6a..ec379e5f8af 100644
--- a/searchlib/src/test/java/com/yahoo/searchlib/aggregation/GroupingTestCase.java
+++ b/searchlib/src/test/java/com/yahoo/searchlib/aggregation/GroupingTestCase.java
@@ -223,5 +223,6 @@ public class GroupingTestCase {
         Grouping other = (Grouping)Grouping.create(buf);
         assertEquals(grouping, other);
     }
+
 }
 
diff --git a/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java b/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java
index 3e392448ce2..78fad5f5867 100644
--- a/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java
+++ b/vespajlib/src/main/java/com/yahoo/vespa/objects/Identifiable.java
@@ -9,7 +9,7 @@ import java.lang.reflect.InvocationTargetException;
 import java.util.HashMap;
 
 /**
- * This is the base class to do cross-language serialization and deserialization of complete object structures without
+ * The base class to do cross-language serialization and deserialization of complete object structures without
  * the need for a separate protocol. Each subclass needs to register itself using the {@link #registerClass(int, Class)}
  * method, and override {@link #onGetClassId()} to return the same classId as the one registered. Creating an instance
  * of an identifiable object is done through the {@link #create(Deserializer)} or {@link #createFromId(int)} factory
diff --git a/vespajlib/src/main/java/com/yahoo/vespa/objects/Selectable.java b/vespajlib/src/main/java/com/yahoo/vespa/objects/Selectable.java
index f1b7d6dc841..e60bb664d7d 100644
--- a/vespajlib/src/main/java/com/yahoo/vespa/objects/Selectable.java
+++ b/vespajlib/src/main/java/com/yahoo/vespa/objects/Selectable.java
@@ -2,17 +2,17 @@
 package com.yahoo.vespa.objects;
 
 /**
- * @author baldersheim
- *
- * This class acts as an interface for traversing a tree, or a graph.
+ * A node in a traversable tree.
  * Every non leaf Object implements {@link #selectMembers(ObjectPredicate, ObjectOperation)} implementing
  * the actual traversal. You can then implement an {@link ObjectPredicate} to select which nodes you want to look at with
  * your {@link ObjectOperation}
+ *
+ * @author baldersheim
  */
 public class Selectable {
 
     /**
-     * Apply the predicate to this object. If the predicate returns true, pass this object to the operation, otherwise
+     * Applies the predicate to this object. If the predicate returns true, pass this object to the operation, otherwise
      * invoke the {@link #selectMembers(ObjectPredicate, ObjectOperation)} method to locate sub-elements that might
      * trigger the predicate.
      *
@@ -28,7 +28,7 @@ public class Selectable {
     }
 
     /**
-     * Invoke {@link #select(ObjectPredicate, ObjectOperation)} on any member objects this object wants to expose
+     * Invokes {@link #select(ObjectPredicate, ObjectOperation)} on any member objects this object wants to expose
      * through the selection mechanism. Overriding this method is optional, and which objects to expose is determined by
      * the application logic of the object itself.
      *
@@ -44,4 +44,5 @@ public class Selectable {
             selectable.select(predicate, operation);
         }
     }
+
 }