You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by dk...@apache.org on 2018/11/07 17:05:41 UTC
[avro] branch master updated: [AVRO-2051] Remove synchronization
for JsonProperties.getJsonProp Fixes #245
This is an automated email from the ASF dual-hosted git repository.
dkulp pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/master by this push:
new a318a34 [AVRO-2051] Remove synchronization for JsonProperties.getJsonProp Fixes #245
a318a34 is described below
commit a318a34d1f5f8146fb441c092c73b6170719e88a
Author: Daniel Kulp <dk...@apache.org>
AuthorDate: Mon Jul 17 15:08:10 2017 -0400
[AVRO-2051] Remove synchronization for JsonProperties.getJsonProp
Fixes #245
---
.../main/java/org/apache/avro/JsonProperties.java | 92 +++++++++++++++++++---
.../src/main/java/org/apache/avro/Protocol.java | 10 +--
.../avro/src/main/java/org/apache/avro/Schema.java | 28 +++----
3 files changed, 101 insertions(+), 29 deletions(-)
diff --git a/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java b/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java
index 3844073..2fca58d 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java
@@ -17,15 +17,23 @@
*/
package org.apache.avro;
+import java.util.AbstractSet;
import java.util.Collections;
+import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Queue;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ConcurrentMap;
+
import java.io.IOException;
import org.apache.avro.util.internal.Accessor;
import org.apache.avro.util.internal.Accessor.JsonPropertiesAccessor;
+import org.apache.avro.reflect.MapEntry;
import org.apache.avro.util.internal.JacksonUtils;
import com.fasterxml.jackson.core.JsonGenerator;
@@ -129,7 +137,45 @@ public abstract class JsonProperties {
/** A value representing a JSON <code>null</code>. */
public static final Null NULL_VALUE = new Null();
- Map<String,JsonNode> props = new LinkedHashMap<>(1);
+ // use a ConcurrentHashMap for speed and thread safety, but keep a Queue of the entries to maintain order
+ // the queue is always updated after the main map and is thus is potentially a subset of the map.
+ // By making props private, we can control access and only implement/override the methods
+ // we need. We don't ever remove anything so we don't need to implement the clear/remove functionality.
+ // Also, we only ever ADD to the collection, never changing a value, so putWithAbsent is the
+ // only modifier
+ private ConcurrentMap<String,JsonNode> props = new ConcurrentHashMap<String,JsonNode>() {
+ private static final long serialVersionUID = 1L;
+ private Queue<MapEntry<String, JsonNode>> propOrder = new ConcurrentLinkedQueue<MapEntry<String, JsonNode>>();
+ public JsonNode putIfAbsent(String key, JsonNode value) {
+ JsonNode r = super.putIfAbsent(key, value);
+ if (r == null) {
+ propOrder.add(new MapEntry<String, JsonNode>(key, value));
+ }
+ return r;
+ }
+ public Set<Map.Entry<String, JsonNode>> entrySet() {
+ return new AbstractSet<Map.Entry<String, JsonNode>>() {
+ @Override
+ public Iterator<Map.Entry<String, JsonNode>> iterator() {
+ return new Iterator<Map.Entry<String, JsonNode>>() {
+ Iterator<MapEntry<String, JsonNode>> it = propOrder.iterator();
+ @Override
+ public boolean hasNext() {
+ return it.hasNext();
+ }
+ @Override
+ public java.util.Map.Entry<String, JsonNode> next() {
+ return it.next();
+ }
+ };
+ }
+ @Override
+ public int size() {
+ return propOrder.size();
+ }
+ };
+ }
+ };
private Set<String> reserved;
@@ -150,7 +196,8 @@ public abstract class JsonProperties {
* Returns the value of the named property in this schema.
* Returns <tt>null</tt> if there is no property with that name.
*/
- synchronized JsonNode getJsonProp(String name) {
+ @Deprecated
+ public JsonNode getJsonProp(String name) {
return props.get(name);
}
@@ -158,7 +205,7 @@ public abstract class JsonProperties {
* Returns the value of the named property in this schema.
* Returns <tt>null</tt> if there is no property with that name.
*/
- public synchronized Object getObjectProp(String name) {
+ public Object getObjectProp(String name) {
return JacksonUtils.toObject(props.get(name));
}
@@ -175,23 +222,37 @@ public abstract class JsonProperties {
addProp(name, TextNode.valueOf(value));
}
- synchronized void addProp(String name, JsonNode value) {
+ /**
+ * Adds a property with the given name <tt>name</tt> and
+ * value <tt>value</tt>. Neither <tt>name</tt> nor <tt>value</tt> can be
+ * <tt>null</tt>. It is illegal to add a property if another with
+ * the same name but different value already exists in this schema.
+ *
+ * @param name The name of the property to add
+ * @param value The value for the property to add
+ * @deprecated use {@link #addProp(String, Object)}
+ */
+ @Deprecated
+ public void addProp(String name, JsonNode value) {
if (reserved.contains(name))
throw new AvroRuntimeException("Can't set reserved property: " + name);
if (value == null)
throw new AvroRuntimeException("Can't set a property to null: " + name);
- JsonNode old = props.get(name);
- if (old == null)
- props.put(name, value);
- else if (!old.equals(value))
+ JsonNode old = props.putIfAbsent(name, value);
+ if (old != null && !old.equals(value)) {
throw new AvroRuntimeException("Can't overwrite property: " + name);
+ }
}
-
- public synchronized void addProp(String name, Object value) {
+ public void addProp(String name, Object value) {
addProp(name, JacksonUtils.toJsonNode(value));
}
+ public void putAll(JsonProperties np) {
+ for (Map.Entry<? extends String, ? extends JsonNode> e : np.props.entrySet())
+ addProp(e.getKey(), e.getValue());
+ }
+
/**
* Adds all the props from the specified json properties.
@@ -241,4 +302,15 @@ public abstract class JsonProperties {
gen.writeObjectField(e.getKey(), e.getValue());
}
+
+ int propsHashCode() {
+ return props.hashCode();
+ }
+ boolean propsEqual(JsonProperties np) {
+ return props.equals(np.props);
+ }
+ public boolean hasProps() {
+ return !props.isEmpty();
+ }
+
}
diff --git a/lang/java/avro/src/main/java/org/apache/avro/Protocol.java b/lang/java/avro/src/main/java/org/apache/avro/Protocol.java
index c5800a7..f381dd6 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/Protocol.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/Protocol.java
@@ -148,11 +148,11 @@ public class Protocol extends JsonProperties {
Message that = (Message)o;
return this.name.equals(that.name)
&& this.request.equals(that.request)
- && props.equals(that.props);
+ && propsEqual(that);
}
public int hashCode() {
- return name.hashCode() + request.hashCode() + props.hashCode();
+ return name.hashCode() + request.hashCode() + propsHashCode();
}
public String getDoc() { return doc; }
@@ -237,7 +237,7 @@ public class Protocol extends JsonProperties {
*/
public Protocol(Protocol p) {
this(p.getName(), p.getDoc(), p.getNamespace());
- props.putAll(p.props);
+ putAll(p);
}
public Protocol(String name, String doc, String namespace) {
@@ -320,12 +320,12 @@ public class Protocol extends JsonProperties {
&& this.namespace.equals(that.namespace)
&& this.types.equals(that.types)
&& this.messages.equals(that.messages)
- && this.props.equals(that.props);
+ && this.propsEqual(that);
}
public int hashCode() {
return name.hashCode() + namespace.hashCode()
- + types.hashCode() + messages.hashCode() + props.hashCode();
+ + types.hashCode() + messages.hashCode() + propsHashCode();
}
/** Render this as <a href="http://json.org/">JSON</a>.*/
diff --git a/lang/java/avro/src/main/java/org/apache/avro/Schema.java b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
index 6c63ddc..e08331e 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/Schema.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
@@ -138,7 +138,7 @@ public abstract class Schema extends JsonProperties {
int hashCode = NO_HASHCODE;
- @Override void addProp(String name, JsonNode value) {
+ @Override public void addProp(String name, JsonNode value) {
super.addProp(name, value);
hashCode = NO_HASHCODE;
}
@@ -350,7 +350,7 @@ public abstract class Schema extends JsonProperties {
}
void toJson(Names names, JsonGenerator gen) throws IOException {
- if (props.size() == 0) { // no props defined
+ if (!hasProps()) { // no props defined
gen.writeString(getName()); // just write name
} else {
gen.writeStartObject();
@@ -369,7 +369,7 @@ public abstract class Schema extends JsonProperties {
if (!(o instanceof Schema)) return false;
Schema that = (Schema)o;
if (!(this.type == that.type)) return false;
- return equalCachedHash(that) && props.equals(that.props);
+ return equalCachedHash(that) && propsEqual(that);
}
public final int hashCode() {
if (hashCode == NO_HASHCODE)
@@ -377,7 +377,7 @@ public abstract class Schema extends JsonProperties {
return hashCode;
}
- int computeHash() { return getType().hashCode() + props.hashCode(); }
+ int computeHash() { return getType().hashCode() + propsHashCode(); }
final boolean equalCachedHash(Schema other) {
return (hashCode == other.hashCode)
@@ -458,7 +458,7 @@ public abstract class Schema extends JsonProperties {
*/
public Field(Field field, Schema schema) {
this(field.name, schema, field.doc, field.defaultValue, field.order);
- props.putAll(field.props);
+ putAll(field);
if (field.aliases != null)
aliases = new LinkedHashSet<String>(field.aliases);
}
@@ -512,7 +512,7 @@ public abstract class Schema extends JsonProperties {
(schema.equals(that.schema)) &&
defaultValueEquals(that.defaultValue) &&
(order == that.order) &&
- props.equals(that.props);
+ propsEqual(that);
}
public int hashCode() { return name.hashCode() + schema.computeHash(); }
@@ -721,7 +721,7 @@ public abstract class Schema extends JsonProperties {
RecordSchema that = (RecordSchema)o;
if (!equalCachedHash(that)) return false;
if (!equalNames(that)) return false;
- if (!props.equals(that.props)) return false;
+ if (!propsEqual(that)) return false;
Set seen = SEEN_EQUALS.get();
SeenPair here = new SeenPair(this, o);
if (seen.contains(here)) return true; // prevent stack overflow
@@ -822,7 +822,7 @@ public abstract class Schema extends JsonProperties {
return equalCachedHash(that)
&& equalNames(that)
&& symbols.equals(that.symbols)
- && props.equals(that.props);
+ && propsEqual(that);
}
@Override
public String getEnumDefault() { return enumDefault; }
@@ -859,7 +859,7 @@ public abstract class Schema extends JsonProperties {
ArraySchema that = (ArraySchema)o;
return equalCachedHash(that)
&& elementType.equals(that.elementType)
- && props.equals(that.props);
+ && propsEqual(that);
}
@Override int computeHash() {
return super.computeHash() + elementType.computeHash();
@@ -887,7 +887,7 @@ public abstract class Schema extends JsonProperties {
MapSchema that = (MapSchema)o;
return equalCachedHash(that)
&& valueType.equals(that.valueType)
- && props.equals(that.props);
+ && propsEqual(that);
}
@Override int computeHash() {
return super.computeHash() + valueType.computeHash();
@@ -928,7 +928,7 @@ public abstract class Schema extends JsonProperties {
UnionSchema that = (UnionSchema)o;
return equalCachedHash(that)
&& types.equals(that.types)
- && props.equals(that.props);
+ && propsEqual(that);
}
@Override int computeHash() {
int hash = super.computeHash();
@@ -966,7 +966,7 @@ public abstract class Schema extends JsonProperties {
return equalCachedHash(that)
&& equalNames(that)
&& size == that.size
- && props.equals(that.props);
+ && propsEqual(that);
}
@Override int computeHash() { return super.computeHash() + size; }
void toJson(Names names, JsonGenerator gen) throws IOException {
@@ -1514,7 +1514,7 @@ public abstract class Schema extends JsonProperties {
Schema fSchema = applyAliases(f.schema, seen, aliases, fieldAliases);
String fName = getFieldAlias(name, f.name, fieldAliases);
Field newF = new Field(fName, fSchema, f.doc, f.defaultValue, f.order);
- newF.props.putAll(f.props); // copy props
+ newF.putAll(f); // copy props
newFields.add(newF);
}
result.setFields(newFields);
@@ -1547,7 +1547,7 @@ public abstract class Schema extends JsonProperties {
break;
}
if (result != s)
- result.props.putAll(s.props); // copy props
+ result.putAll(s); // copy props
return result;
}