From 004ef6b04bbc0ce0ebfc1cad42656559b043bcb2 Mon Sep 17 00:00:00 2001 From: Eugene Sevastyanov Date: Fri, 16 Feb 2018 01:11:00 +0300 Subject: [PATCH] MINOR: Cache Node's hashCode to improve the producer's performance (#4350) `Node` is immutable so this is safe. With 100 brokers, 150 topics and 350 partitions, `HashSet.contains` in `RecordAccumulator.ready` took about 40% of the application time. It is caused by re-calculating a hash code of a leader (Node instance) for every batch entry. Caching the hashCode reduced the time of `HashSet.contains` in `RecordAccumulator.ready` to ~2%. The measurements were taken with Flight Recorder. Reviewers: Rajini Sivaram , Ted Yu , Ismael Juma --- .../java/org/apache/kafka/common/Node.java | 45 ++++++++----------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/clients/src/main/java/org/apache/kafka/common/Node.java b/clients/src/main/java/org/apache/kafka/common/Node.java index 8187369ca3c..d51fa13946b 100644 --- a/clients/src/main/java/org/apache/kafka/common/Node.java +++ b/clients/src/main/java/org/apache/kafka/common/Node.java @@ -29,12 +29,14 @@ public class Node { private final int port; private final String rack; + // Cache hashCode as it is called in performance sensitive parts of the code (e.g. RecordAccumulator.ready) + private Integer hash; + public Node(int id, String host, int port) { this(id, host, port, null); } public Node(int id, String host, int port, String rack) { - super(); this.id = id; this.idString = Integer.toString(id); this.host = host; @@ -100,39 +102,30 @@ public class Node { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((host == null) ? 0 : host.hashCode()); - result = prime * result + id; - result = prime * result + port; - result = prime * result + ((rack == null) ? 0 : rack.hashCode()); - return result; + Integer h = this.hash; + if (h == null) { + int result = 31 + ((host == null) ? 0 : host.hashCode()); + result = 31 * result + id; + result = 31 * result + port; + result = 31 * result + ((rack == null) ? 0 : rack.hashCode()); + this.hash = result; + return result; + } else { + return h; + } } @Override public boolean equals(Object obj) { if (this == obj) return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) + if (obj == null || getClass() != obj.getClass()) return false; Node other = (Node) obj; - if (host == null) { - if (other.host != null) - return false; - } else if (!host.equals(other.host)) - return false; - if (id != other.id) - return false; - if (port != other.port) - return false; - if (rack == null) { - if (other.rack != null) - return false; - } else if (!rack.equals(other.rack)) - return false; - return true; + return (host == null ? other.host == null : host.equals(other.host)) && + id == other.id && + port == other.port && + (rack == null ? other.rack == null : rack.equals(other.rack)); } @Override