[GitHub] [kafka] ijuma commented on a diff in pull request #13024: MINOR: Avoid unnecessary allocations in index binary search

[GitBox]

ijuma commented on code in PR #13024:
URL: https://github.com/apache/kafka/pull/13024#discussion_r1053450391


##########
storage/src/main/java/org/apache/kafka/server/log/internals/AbstractIndex.java:
##########
@@ -484,27 +478,35 @@ private static MappedByteBuffer 
createMappedBuffer(RandomAccessFile raf, boolean
     }
 
     /**
-     * Lookup lower and upper bounds for the given target.
+     * Lookup lower or upper bounds for the given target.
      */
-    private BinarySearchResult indexSlotRangeFor(ByteBuffer idx, long target, 
IndexSearchType searchEntity) {
+    private int indexSlotRangeFor(ByteBuffer idx, long target, IndexSearchType 
searchEntity,
+                                  SearchType searchType) {
         // check if the index is empty
         if (entries == 0)
-            return new BinarySearchResult(-1, -1);
+            return -1;
 
         int firstHotEntry = Math.max(0, entries - 1 - warmEntries());
         // check if the target offset is in the warm section of the index
         if (compareIndexEntry(parseEntry(idx, firstHotEntry), target, 
searchEntity) < 0) {
-            return binarySearch(idx, target, searchEntity, firstHotEntry, 
entries - 1);
+            return binarySearch(idx, target, searchEntity, searchType, 
firstHotEntry, entries - 1);
         }
 
         // check if the target offset is smaller than the least offset
-        if (compareIndexEntry(parseEntry(idx, 0), target, searchEntity) > 0)
-            return new BinarySearchResult(-1, 0);
+        if (compareIndexEntry(parseEntry(idx, 0), target, searchEntity) > 0) {
+            switch (searchType) {
+                case SMALLEST_UPPER_BOUND:
+                    return -1;
+                case LARGEST_LOWER_BOUND:
+                    return 0;
+            }

Review Comment:
   It's better to have exhaustive matches than to add a `default`. 
Unfortunately, the Java compiler doesn't always infer that the method always 
returns due to an exhaustive match (at least not with old school switches, the 
new switch probably handles it better) and hence we have to add the explicit 
`default`. So, I'd prefer to leave as is.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org