spacemonkd commented on code in PR #10919:
URL: https://github.com/apache/ozone/pull/10919#discussion_r3695976516
##########
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.hashCode = hash(hash(1, first), second);
+ this.first = Objects.requireNonNull(first, "first == null");
+ this.second = Objects.requireNonNull(second, "second == null");
+ }
+
+ @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
+ && Objects.equals(this.first, that.first)
Review Comment:
Nit: Since we already know that `first` and `second` are non-null (this is
asserted in the constructor), we can avoid the overhead of Object.equals().
A simple ` this.first.equals(that.first)` would be a bit cheaper and simpler.
##########
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.hashCode = hash(hash(1, first), second);
+ this.first = Objects.requireNonNull(first, "first == null");
+ this.second = Objects.requireNonNull(second, "second == null");
+ }
+
+ @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
+ && Objects.equals(this.first, that.first)
+ && Objects.equals(this.second, that.second);
Review Comment:
Same here
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
