HDDS-16059. Optimize varargs in IOzoneManagerLock.

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/CompositeKey.java

@@ -18,53 +18,94 @@
 package org.apache.hadoop.hdds.utils;
 
 import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ratis.util.Preconditions;
 
 /**
  * This is a utility to combine multiple objects as a key that can be used in
  * hash map access. The advantage of this is that it is cheap in comparison
  * to other methods like string concatenation.
- *
- * For example, if a composition of volume, bucket and key is needed to
- * access a hash map, the natural method is:
- * <pre> {@code
- * String key = "/" + volume + "/" + bucket + "/" + key.
- * map.put(key, value);
- * }</pre>
- * This is costly because it creates (and stores) a new buffer.
- *
- * In comparison, the following achieve the same logic without creating any new
- * buffer.
- * <pre> {@code
- * Object key = combineKeys(volume, bucket, key).
- * map.put(key, value);
- * }</pre>
- *
  */
-public final class CompositeKey {
-  private final int hashCode;
-  private final Object[] components;
-
-  CompositeKey(Object[] components) {
-    this.components = components;
-    this.hashCode = Arrays.hashCode(components);
+public abstract class CompositeKey {
+  /** The same as {@link Arrays#hashCode(Object[])} for one loop step. */
+  static int hash(int result, Object next) {
+    return 31 * result + next.hashCode();
   }
 
-  @Override
-  public int hashCode() {
-    return hashCode;
+  private static final class TwoComponents extends CompositeKey {
+    private final int hashCode;
+    private final Object first;
+    private final Object second;
+
+    private TwoComponents(Object first, Object second) {
+      this.first = Objects.requireNonNull(first, "first == null");
+      this.second = Objects.requireNonNull(second, "second == null");
+      this.hashCode = hash(hash(1, first), second);
+    }
+
+    @Override
+    public int hashCode() {
+      return hashCode;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (this == obj) {
+        return true;
+      } else if (!(obj instanceof TwoComponents)) {
+        return false;
+      }
+      final TwoComponents that = (TwoComponents) obj;
+      return this.hashCode == that.hashCode
+          && this.first.equals(that.first)
+          && this.second.equals(that.second);
+    }
   }
 
-  @Override
-  public boolean equals(Object obj) {
-    if (!(obj instanceof CompositeKey)) {
-      return false;
+  private static final class MultiComponents extends CompositeKey {
+    private final int hashCode;
+    private final Object[] components;
+
+    MultiComponents(Object[] components) {
+      Preconditions.assertTrue(components.length > 2, () -> "components.length " + components.length + " <= 2");
+      for (int i = 0; i < components.length; i++) {
+        final int j = i;
+        Objects.requireNonNull(components[j], () -> "components[" + j + "] == null");
+      }
+
+      this.hashCode = Arrays.hashCode(components);
+      this.components = components;
+    }
+
+    @Override
+    public int hashCode() {
+      return hashCode;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (this == obj) {
+        return true;
+      } else if (!(obj instanceof MultiComponents)) {
+        return false;
+      }
+      final MultiComponents that = (MultiComponents) obj;
+      return this.hashCode == that.hashCode
+          && Arrays.equals(this.components, that.components);
     }
-    CompositeKey other = (CompositeKey) obj;
-    return Arrays.equals(components, other.components);
+  }
+
+  public static CompositeKey combineTwoKeys(Object first, Object second) {
+    return new TwoComponents(first, second);
+  }
+
+  public static CompositeKey combineMultiKeys(Object[] components) {
+    return new MultiComponents(components);
   }
 
   public static Object combineKeys(Object[] components) {
-    return components.length == 1 ?
-        components[0] : new CompositeKey(components);
+    return components.length == 1 ? components[0]
+        : components.length == 2 ? CompositeKey.combineTwoKeys(components[0], components[1])
+        : CompositeKey.combineMultiKeys(components);
   }
 }

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/TestCompositeKey.java

@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.utils;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.Arrays;
+import java.util.Random;
+import org.junit.jupiter.api.Test;
+
+/** Test {@link CompositeKey}. */
+public final class TestCompositeKey {
+  private static final Random RANDOM = new Random();
+
+  static String randomString(int length) {
+    final StringBuilder builder = new StringBuilder(length);
+    for (int i = 0; i < length; i++) {
+      builder.append(RANDOM.nextInt(10));
+    }
+    return builder.toString();
+  }
+
+  static Object[] randomComponents(int numComponents) {
+    final Object[] components = new Object[numComponents];
+    for (int i = 0; i < components.length; i++) {
+      components[i] = randomString(RANDOM.nextInt(10));
+    }
+    return components;
+  }
+
+  private static final class OldCompositeKey {
+    private final int hashCode;
+    private final Object[] components;
+
+    OldCompositeKey(Object[] components) {
+      this.components = components;
+      this.hashCode = Arrays.hashCode(components);
+    }
+
+    @Override
+    public int hashCode() {
+      return hashCode;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (!(obj instanceof OldCompositeKey)) {
+        return false;
+      }
+      OldCompositeKey other = (OldCompositeKey) obj;
+      return Arrays.equals(components, other.components);
+    }
+
+    static Object combineKeys(Object[] components) {
+      return components.length == 1 ?
+          components[0] : new OldCompositeKey(components);
+    }
+  }
+
+  static void assertHashCode(Object[] components, int computed) {
+    final Object expected = OldCompositeKey.combineKeys(components);
+    assertEquals(expected.hashCode(), CompositeKey.combineKeys(components).hashCode());
+    assertEquals(expected.hashCode(), computed);
+  }
+
+  @Test
+  public void testHashCodeOne() {
+    for (int i = 0; i < 100; i++) {
+      final Object[] components = {randomString(i)};
+      assertHashCode(components, components[0].hashCode());
+    }
+  }
+
+  @Test
+  public void testHashCodeTwo() {
+    for (int i = 0; i < 10; i++) {
+      for (int j = 0; j < 10; j++) {
+        final Object first = randomString(i);
+        final Object second = randomString(j);
+        final Object[] components = {first, second};
+        assertHashCode(components, CompositeKey.combineTwoKeys(first, second).hashCode());
+      }
+    }
+  }
+
+  @Test
+  public void testHashCodeMulti() {
+    for (int i = 3; i < 100; i++) {
+      final Object[] components = randomComponents(i);
+      assertHashCode(components, CompositeKey.combineMultiKeys(components).hashCode());
+    }
+  }
+}

hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/lock/IOzoneManagerLock.java

@@ -26,34 +26,52 @@
  */
 public interface IOzoneManagerLock {
 
-  OMLockDetails acquireReadLock(Resource resource,
-                                String... resources);
+  // ---------- acquireReadLock ----------
+  OMLockDetails acquireReadLock(Resource resource, String key);
+
+  OMLockDetails acquireReadLock(Resource resource, String key1, String key2);
+
+  OMLockDetails acquireReadLock(Resource resource, String... keys);
 
   OMLockDetails acquireReadLocks(Resource resource, Iterable<String[]> keys);
 
-  OMLockDetails acquireWriteLock(Resource resource,
-                                 String... resources);
+  // ---------- acquireWriteLock ----------
+  OMLockDetails acquireWriteLock(Resource resource, String key);
+
+  OMLockDetails acquireWriteLock(Resource resource, String key1, String key2);
+
+  OMLockDetails acquireWriteLock(Resource resource, String... keys);
 
   OMLockDetails acquireWriteLocks(Resource resource, Iterable<String[]> keys);
 
   OMLockDetails acquireResourceWriteLock(Resource resource);
 
+  // ---------- MultiUserLock ----------
   boolean acquireMultiUserLock(String firstUser, String secondUser);
 
   void releaseMultiUserLock(String firstUser, String secondUser);
 
-  OMLockDetails releaseWriteLock(Resource resource,
-                        String... resources);
+  // ---------- releaseWriteLock ----------
+  OMLockDetails releaseWriteLock(Resource resource, String key);
+
+  OMLockDetails releaseWriteLock(Resource resource, String key1, String key2);
+
+  OMLockDetails releaseWriteLock(Resource resource, String... keys);
 
   OMLockDetails releaseWriteLocks(Resource resource, Iterable<String[]> keys);
 
   OMLockDetails releaseResourceWriteLock(Resource resource);
 
-  OMLockDetails releaseReadLock(Resource resource,
-                                String... resources);
+  // ---------- releaseReadLock ----------
+  OMLockDetails releaseReadLock(Resource resource, String key);
+
+  OMLockDetails releaseReadLock(Resource resource, String key1, String key2);
+
+  OMLockDetails releaseReadLock(Resource resource, String... keys);
 
   OMLockDetails releaseReadLocks(Resource resource, Iterable<String[]> keys);
 
+  // ---------- other methods ----------
   @VisibleForTesting
   int getReadHoldCount(Resource resource,
       String... resources);

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/lock/OBSKeyPathLockStrategy.java

@@ -43,6 +43,7 @@ public OMLockDetails acquireWriteLock(OMMetadataManager omMetadataManager,
     Preconditions.checkArgument(omLockDetails.isLockAcquired(),
         "BUCKET_LOCK should be acquired!");
 
+    // TODO optimize three key case in similar way as HDDS-16059
     omLockDetails.merge(omMetadataManager.getLock()
         .acquireWriteLock(KEY_PATH_LOCK, volumeName, bucketName, keyName));
 

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java

@@ -28,7 +28,17 @@
 public class OmReadOnlyLock implements IOzoneManagerLock {
 
   @Override
-  public OMLockDetails acquireReadLock(Resource resource, String... resources) {
+  public OMLockDetails acquireReadLock(Resource resource, String key) {
+    return EMPTY_DETAILS_LOCK_ACQUIRED;
+  }
+
+  @Override
+  public OMLockDetails acquireReadLock(Resource resource, String key1, String key2) {
+    return EMPTY_DETAILS_LOCK_ACQUIRED;
+  }
+
+  @Override
+  public OMLockDetails acquireReadLock(Resource resource, String... keys) {
     return EMPTY_DETAILS_LOCK_ACQUIRED;
   }
 
@@ -38,8 +48,17 @@ public OMLockDetails acquireReadLocks(Resource resource, Iterable<String[]> keys
   }
 
   @Override
-  public OMLockDetails acquireWriteLock(Resource resource,
-      String... resources) {
+  public OMLockDetails acquireWriteLock(Resource resource, String key) {
+    return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
+  }
+
+  @Override
+  public OMLockDetails acquireWriteLock(Resource resource, String key1, String key2) {
+    return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
+  }
+
+  @Override
+  public OMLockDetails acquireWriteLock(Resource resource, String... keys) {
     return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
   }
 
@@ -64,8 +83,17 @@ public void releaseMultiUserLock(String firstUser, String secondUser) {
   }
 
   @Override
-  public OMLockDetails releaseWriteLock(Resource resource,
-      String... resources) {
+  public OMLockDetails releaseWriteLock(Resource resource, String key) {
+    return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
+  }
+
+  @Override
+  public OMLockDetails releaseWriteLock(Resource resource, String key1, String key2) {
+    return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
+  }
+
+  @Override
+  public OMLockDetails releaseWriteLock(Resource resource, String... keys) {
     return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
   }
 
@@ -79,6 +107,16 @@ public OMLockDetails releaseResourceWriteLock(Resource resource) {
     return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
   }
 
+  @Override
+  public OMLockDetails releaseReadLock(Resource resource, String key) {
+    return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
+  }
+
+  @Override
+  public OMLockDetails releaseReadLock(Resource resource, String key1, String key2) {
+    return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;
+  }
+
   @Override
   public OMLockDetails releaseReadLock(Resource resource, String... resources) {
     return EMPTY_DETAILS_LOCK_NOT_ACQUIRED;