perf: optimize copy on write attributes (#74)

**Requirements**

- [x] I have added test coverage for new or changed functionality
- [x] I have followed the repository's [pull request submission
guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests)
- [x] I have validated my changes against all supported platform
versions

**Related issues**

none

**Describe the solution you've provided**

i have modified the copy on write mechanism for custom attributes to
avoid having to copy the whole attribute map. instead i chain to the
parent map. this does not change the public API or outwards behavior.
the implementation was complicated because of the need to support the
"remove" operation. additionally, i leveraged the equals and hashCode of
Map. note: hashCode from Map is not order dependent since it uses
addition across all of the entries and addition is commutative.

**Describe alternatives you've considered**

i was also playing around with a lazy LDValue provider such that we do
not need to create all of the attributes if we do not use them but that
seemed like a lot of rework and hard to do in a codebase currently
targeting jdk 7.


**Additional context**

i was doing flame graph analysis and found that there were a lot of
copies of the hash maps going on, so i thought i could try to eliminate
those copies. my use case involves the case in the new test case i
added, but it is located in a hotspot in my codebase. the parent context
has dozens of attributes and is shared across multiple evaluations.
however the child contexts add on only a handful of attributes. the
current implementation was copying over all of the attributes for each
new build since the parent context was tainted. this root/parent
chaining and reuse is a common paradigm over the codebase

```
controller(){
 LDContext rootContext = LDContext.builder(key).set(...).set(...)......build();
 ...
 if (ldClient.boolVariation(rootContext, ...)) { ... }
 ...
 for(a lot of iterations) {
  LDContext innerContext = LDContext.builderFromContext(rootContext).set(....).build()
  if(ldClient.boolVariation(innerContext, ...)) { ... }
  // LDContext.builderFromContext(innerContext) may be called as well
  ...
 }
 ...
}
```

Co-authored-by: Todd Anderson <[email protected]>

Author: yuzawa-san

lib/shared/common/src/main/java/com/launchdarkly/sdk/AttributeMap.java

@@ -0,0 +1,87 @@
+package com.launchdarkly.sdk;
+
+import java.util.HashMap;
+import java.util.Map;
+
+final class AttributeMap {
+  private final AttributeMap parent;
+  private final Map<String, LDValue> map;
+
+  AttributeMap() {
+    this(null);
+  }
+
+  AttributeMap(AttributeMap parent) {
+    this.parent = parent;
+    this.map = new HashMap<>();
+  }
+
+  LDValue get(String key) {
+    AttributeMap current = this;
+    while (current != null) {
+      LDValue value = current.map.get(key);
+      if (value != null) {
+        if (value.isNull()) {
+          break;
+        }
+        return value;
+      }
+      current = current.parent;
+    }
+    return null;
+  }
+
+  void put(String key, LDValue value) {
+    map.put(key, value);
+  }
+
+  @Override
+  public int hashCode() {
+    return flatten().hashCode();
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    if (this == other) {
+      return true;
+    }
+    if (!(other instanceof AttributeMap)) {
+      return false;
+    }
+    AttributeMap o = (AttributeMap) other;
+    return flatten().equals(o.flatten());
+  }
+
+  Map<String, LDValue> flatten() {
+    if (parent == null) {
+      return map;
+    }
+    Map<String, LDValue> out = new HashMap<>();
+    flattenRecursive(out);
+    return out;
+  }
+
+  private void flattenRecursive(Map<String, LDValue> out) {
+    if (parent != null) {
+      parent.flattenRecursive(out);
+    }
+    for (Map.Entry<String, LDValue> entry : map.entrySet()) {
+      String key = entry.getKey();
+      LDValue value = entry.getValue();
+      if (value.isNull()) {
+        out.remove(key);
+      } else {
+        out.put(key, value);
+      }
+    }
+  }
+
+  void remove(String key) {
+    if (parent == null) {
+      map.remove(key);
+      return;
+    }
+    // we need to hide the value from the parents
+    map.put(key, LDValue.ofNull());
+  }
+}

lib/shared/common/src/main/java/com/launchdarkly/sdk/ContextBuilder.java

@@ -1,9 +1,7 @@
 package com.launchdarkly.sdk;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 /**
  * A mutable object that uses the builder pattern to specify properties for {@link LDContext}.
@@ -34,7 +32,7 @@ public final class ContextBuilder {
   private ContextKind kind;
   private String key;
   private String name;
-  private Map<String, LDValue> attributes;
+  private AttributeMap attributes;
   private boolean anonymous;
   private List<AttributeRef> privateAttributes;
   private boolean copyOnWriteAttributes;
@@ -303,7 +301,7 @@ public boolean trySet(String attributeName, LDValue value) {
       return false;
     default:
       if (copyOnWriteAttributes) {
-        attributes = new HashMap<>(attributes);
+        attributes = new AttributeMap(attributes);
         copyOnWriteAttributes = false;
       }
       if (value == null || value.isNull()) {
@@ -312,7 +310,7 @@ public boolean trySet(String attributeName, LDValue value) {
         }
       } else {
         if (attributes == null) {
-          attributes = new HashMap<>();
+          attributes = new AttributeMap();
         }
         attributes.put(attributeName, value);
       }

lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContext.java

@@ -58,7 +58,7 @@ public final class LDContext implements JsonSerializable {
   final String key;
   final String fullyQualifiedKey;
   final String name;
-  final Map<String, LDValue> attributes;
+  final AttributeMap attributes;
   final boolean anonymous;
   final List<AttributeRef> privateAttributes;
 
@@ -68,7 +68,7 @@ private LDContext(
       String key,
       String fullyQualifiedKey,
       String name,
-      Map<String, LDValue> attributes,
+      AttributeMap attributes,
       boolean anonymous,
       List<AttributeRef> privateAttributes
       ) {
@@ -100,7 +100,7 @@ static LDContext createSingle(
       ContextKind kind,
       String key,
       String name,
-      Map<String, LDValue> attributes,
+      AttributeMap attributes,
       boolean anonymous,
       List<AttributeRef> privateAttributes,
       boolean allowEmptyKey // allowEmptyKey is true only when deserializing old-style user JSON
@@ -300,22 +300,22 @@ public static LDContext fromUser(LDUser user) {
         return failed(Errors.CONTEXT_NO_KEY);
       }
     }
-    Map<String, LDValue> attributes = null;
+    AttributeMap attributes = null;
     for (UserAttribute a: UserAttribute.OPTIONAL_STRING_ATTRIBUTES) {
       if (a == UserAttribute.NAME) {
         continue;
       }
       LDValue value = user.getAttribute(a);
       if (!value.isNull()) {
         if (attributes == null) {
-          attributes = new HashMap<>(); 
+          attributes = new AttributeMap(); 
         }
         attributes.put(a.getName(), value);
       }
     }
     if (user.custom != null && !user.custom.isEmpty()) {
       if (attributes == null) {
-        attributes = new HashMap<>(); 
+        attributes = new AttributeMap(); 
       }
       for (Map.Entry<UserAttribute, LDValue> kv: user.custom.entrySet()) {
         attributes.put(kv.getKey().getName(), kv.getValue());
@@ -671,7 +671,7 @@ public LDValue getValue(AttributeRef attributeRef) {
    * @return an iterable of strings (may be empty, but will never be null)
    */
   public Iterable<String> getCustomAttributeNames() {
-    return attributes == null ? Collections.<String>emptyList() : attributes.keySet();
+    return attributes == null ? Collections.<String>emptyList() : attributes.flatten().keySet();
   }
 
   /**
@@ -853,17 +853,9 @@ public boolean equals(Object other) {
     if (!Objects.equals(key, o.key) || !Objects.equals(name, o.name) || anonymous != o.anonymous) {
       return false;
     }
-    if ((attributes == null ? 0 : attributes.size()) !=
-        (o.attributes == null ? 0 : o.attributes.size())) {
+    if (!Objects.equals(attributes, o.attributes)) {
       return false;
     }
-    if (attributes != null) {
-      for (Map.Entry<String, LDValue> kv: attributes.entrySet()) {
-        if (!Objects.equals(o.attributes.get(kv.getKey()), kv.getValue())) {
-          return false;
-        }
-      }
-    }
     if (getPrivateAttributeCount() != o.getPrivateAttributeCount()) {
       return false;
     }
@@ -886,22 +878,16 @@ public boolean equals(Object other) {
 
   @Override
   public int hashCode() {
-    // This implementation of hashCode() is inefficient due to the need to create a predictable ordering
-    // of attribute names. That's necessary just for the sake of aligning with the behavior of equals(),
-    // which is insensitive to ordering. However, using an LDContext as a map key is not an anticipated
-    // or recommended use case.
+    // This implementation of hashCode() is inefficient due to the need to flatten the attributes map.
+    // However, using an LDContext as a map key is not an anticipated or recommended use case.
     int h = Objects.hash(error, kind, key, name, anonymous);
     if (multiContexts != null) {
       for (LDContext c: multiContexts) {
         h = h * 17 + c.hashCode();
       }
     }
     if (attributes != null) {
-      String[] names = attributes.keySet().toArray(new String[attributes.size()]);
-      Arrays.sort(names);
-      for (String name: names) {
-        h = (h * 17 + name.hashCode()) * 17 + attributes.get(name).hashCode(); 
-      }
+      h = h * 17 + attributes.hashCode();
     }
     if (privateAttributes != null) {
       AttributeRef[] refs = privateAttributes.toArray(new AttributeRef[privateAttributes.size()]);

lib/shared/common/src/main/java/com/launchdarkly/sdk/LDContextTypeAdapter.java

@@ -51,7 +51,7 @@ private void writeSingleKind(JsonWriter out, LDContext c, boolean includeKind) t
       out.name(ATTR_ANONYMOUS).value(c.isAnonymous());
     }
     if (c.attributes != null) {
-      for (Map.Entry<String, LDValue> kv: c.attributes.entrySet()) {
+      for (Map.Entry<String, LDValue> kv: c.attributes.flatten().entrySet()) {
         out.name(kv.getKey());
         LDValueTypeAdapter.INSTANCE.write(out, kv.getValue());
       }

lib/shared/common/src/test/java/com/launchdarkly/sdk/ContextBuilderTest.java

@@ -2,7 +2,9 @@
 
 import org.junit.Test;
 
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import static com.launchdarkly.sdk.LDContextTest.kind1;
 import static com.launchdarkly.sdk.LDContextTest.kind2;
@@ -75,6 +77,35 @@ public void copyOnWriteAttributes() {
     assertThat(c1.getValue("b"), equalTo(LDValue.ofNull()));
     assertThat(c2.getValue("a"), equalTo(LDValue.of(2)));
     assertThat(c2.getValue("b"), equalTo(LDValue.of(3)));
+    Map<String, LDValue> c1Map = new HashMap<>();
+    c1Map.put("a", LDValue.of(1));
+    assertThat(c1.attributes.flatten(), equalTo(c1Map));
+    Map<String, LDValue> c2Map = new HashMap<>();
+    c2Map.put("a", LDValue.of(2));
+    c2Map.put("b", LDValue.of(3));
+    assertThat(c2.attributes.flatten(), equalTo(c2Map));
+  }
+  
+  
+  @Test
+  public void copyOnWriteAttributes2() {
+    LDContext c1 = LDContext.builder("key").set("a", 1).set("c", LDValue.ofNull()).set("fsdf", 12).build();
+    LDContext c2 = LDContext.builderFromContext(c1).set("a", 2).set("b", 3).set("fsdf", LDValue.ofNull()).build();
+    
+    assertThat(c1.getValue("a"), equalTo(LDValue.of(1)));
+    assertThat(c1.getValue("b"), equalTo(LDValue.ofNull()));
+    assertThat(c1.getValue("fsdf"), equalTo(LDValue.of(12)));
+    assertThat(c2.getValue("a"), equalTo(LDValue.of(2)));
+    assertThat(c2.getValue("b"), equalTo(LDValue.of(3)));
+    assertThat(c2.getValue("fsdf"), equalTo(LDValue.ofNull()));
+    Map<String, LDValue> c1Map = new HashMap<>();
+    c1Map.put("a", LDValue.of(1));
+    c1Map.put("fsdf", LDValue.of(12));
+    assertThat(c1.attributes.flatten(), equalTo(c1Map));
+    Map<String, LDValue> c2Map = new HashMap<>();
+    c2Map.put("a", LDValue.of(2));
+    c2Map.put("b", LDValue.of(3));
+    assertThat(c2.attributes.flatten(), equalTo(c2Map));
   }
 
   @Test