You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@orc.apache.org by do...@apache.org on 2021/08/17 16:48:45 UTC

[orc] branch main updated: ORC-838: Simplify `compareTo/equals/putBuffer` of ByteBufferAllocatorPool (#739)

This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/orc.git


The following commit(s) were added to refs/heads/main by this push:
     new adaf946  ORC-838: Simplify `compareTo/equals/putBuffer` of ByteBufferAllocatorPool (#739)
adaf946 is described below

commit adaf946f80b2ae656df2eb32b2d63280add16c92
Author: belugabehr <12...@users.noreply.github.com>
AuthorDate: Tue Aug 17 12:48:37 2021 -0400

    ORC-838: Simplify `compareTo/equals/putBuffer` of ByteBufferAllocatorPool (#739)
    
    ### What changes were proposed in this pull request?
    Simplify ByteBufferAllocatorPool Key class. Cache hash value for Key as it's computed more than once: putBuffer to insert into Tree and getBuffer to remove it from tree.
    
    ### Why are the changes needed?
    Less code. Performance.
    
    ### How was this patch tested?
    No changes to functionality. Use existing unit tests.
---
 .../org/apache/orc/impl/RecordReaderUtils.java     | 32 ++++++++--------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java b/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java
index 027d8c1..f8f0d9d 100644
--- a/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java
+++ b/java/core/src/java/org/apache/orc/impl/RecordReaderUtils.java
@@ -511,24 +511,17 @@ public class RecordReaderUtils {
 
       @Override
       public int compareTo(Key other) {
-        if (capacity != other.capacity) {
-          return capacity - other.capacity;
-        } else {
-          return Long.compare(insertionGeneration, other.insertionGeneration);
-        }
+        final int c = Integer.compare(capacity, other.capacity);
+        return (c != 0) ? c : Long.compare(insertionGeneration, other.insertionGeneration);
       }
 
       @Override
       public boolean equals(Object rhs) {
-        if (rhs == null) {
-          return false;
-        }
-        try {
+        if (rhs instanceof Key) {
           Key o = (Key) rhs;
-          return (compareTo(o) == 0);
-        } catch (ClassCastException e) {
-          return false;
+          return 0 == compareTo(o);
         }
+        return false;
       }
 
       @Override
@@ -568,14 +561,13 @@ public class RecordReaderUtils {
     @Override
     public void putBuffer(ByteBuffer buffer) {
       TreeMap<Key, ByteBuffer> tree = getBufferTree(buffer.isDirect());
-      while (true) {
-        Key key = new Key(buffer.capacity(), currentGeneration++);
-        if (tree.putIfAbsent(key, buffer) == null) {
-          return;
-        }
-        // Buffers are indexed by (capacity, generation).
-        // If our key is not unique on the first try, we try again
-      }
+      Key key;
+
+      // Buffers are indexed by (capacity, generation).
+      // If our key is not unique on the first try, try again
+      do {
+        key = new Key(buffer.capacity(), currentGeneration++);
+      } while (tree.putIfAbsent(key, buffer) != null);
     }
   }
 }