Skip to content

Commit 6366bad

Browse files
loganlinnmikeproeng37
loganlinn
authored and
mikeproeng37
committed
refactor: Performance improvements for JacksonConfigParser (#218)
1 parent d60a443 commit 6366bad

13 files changed

Lines changed: 242 additions & 177 deletions

‎.editorconfig‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# EditorConfig is awesome: http://EditorConfig.org
2+
3+
# top-most EditorConfig file
4+
root = true
5+
6+
# 4 space indentation
7+
[*.{py,java}]
8+
indent_style = space
9+
indent_size = 4

‎build.gradle‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ buildscript {
1414
plugins {
1515
id 'com.github.kt3k.coveralls' version '2.8.2'
1616
id 'jacoco'
17-
id 'me.champeau.gradle.jmh' version '0.3.1'
17+
id 'me.champeau.gradle.jmh' version '0.4.5'
1818
id 'nebula.optional-base' version '3.2.0'
1919
}
2020

@@ -79,6 +79,10 @@ subprojects {
7979
}
8080
}
8181

82+
findbugs {
83+
findbugsJmh.enabled = false
84+
}
85+
8286
test {
8387
useJUnit {
8488
excludeCategories 'com.optimizely.ab.categories.ExhaustiveTest'
@@ -104,9 +108,8 @@ subprojects {
104108
}
105109

106110
dependencies {
107-
afterEvaluate {
108-
jmh configurations.testCompile.allDependencies
109-
}
111+
jmh 'org.openjdk.jmh:jmh-core:1.12'
112+
jmh 'org.openjdk.jmh:jmh-generator-annprocess:1.12'
110113
}
111114

112115
dependencies {

‎core-api/src/jmh/java/com/optimizely/ab/config/parser/JacksonConfigParserBenchmark.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
*
3+
* Copyright 2018, Optimizely and contributors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package com.optimizely.ab.config.parser;
18+
19+
import com.optimizely.ab.config.ProjectConfig;
20+
import com.optimizely.ab.config.ProjectConfigTestUtils;
21+
import org.openjdk.jmh.annotations.*;
22+
23+
import java.io.IOException;
24+
import java.util.concurrent.TimeUnit;
25+
26+
@BenchmarkMode(Mode.AverageTime)
27+
@OutputTimeUnit(TimeUnit.MICROSECONDS)
28+
@Fork(2)
29+
@Warmup(iterations = 10)
30+
@Measurement(iterations = 20)
31+
@State(Scope.Benchmark)
32+
public class JacksonConfigParserBenchmark {
33+
JacksonConfigParser parser;
34+
String jsonV2;
35+
String jsonV3;
36+
String jsonV4;
37+
38+
@Setup
39+
public void setUp() throws IOException {
40+
parser = new JacksonConfigParser();
41+
jsonV2 = ProjectConfigTestUtils.validConfigJsonV2();
42+
jsonV3 = ProjectConfigTestUtils.validConfigJsonV3();
43+
jsonV4 = ProjectConfigTestUtils.validConfigJsonV4();
44+
}
45+
46+
@Benchmark
47+
public ProjectConfig parseV2() throws ConfigParseException {
48+
return parser.parseProjectConfig(jsonV2);
49+
}
50+
51+
@Benchmark
52+
public ProjectConfig parseV3() throws ConfigParseException {
53+
return parser.parseProjectConfig(jsonV3);
54+
}
55+
56+
@Benchmark
57+
public ProjectConfig parseV4() throws ConfigParseException {
58+
return parser.parseProjectConfig(jsonV4);
59+
}
60+
}

‎core-api/src/main/java/com/optimizely/ab/config/Experiment.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2016-2017, Optimizely and contributors
3+
* Copyright 2016-2018, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -16,17 +16,14 @@
1616
*/
1717
package com.optimizely.ab.config;
1818

19-
import com.fasterxml.jackson.annotation.JsonCreator;
20-
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
21-
import com.fasterxml.jackson.annotation.JsonProperty;
22-
23-
import java.util.Collections;
24-
import java.util.List;
25-
import java.util.Map;
19+
import com.fasterxml.jackson.annotation.*;
2620

2721
import javax.annotation.Nonnull;
2822
import javax.annotation.Nullable;
2923
import javax.annotation.concurrent.Immutable;
24+
import java.util.Collections;
25+
import java.util.List;
26+
import java.util.Map;
3027

3128
/**
3229
* Represents the Optimizely Experiment configuration.

‎core-api/src/main/java/com/optimizely/ab/config/Group.java‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2016-2017, Optimizely and contributors
3+
* Copyright 2016-2018, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -20,9 +20,9 @@
2020
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2121
import com.fasterxml.jackson.annotation.JsonProperty;
2222

23-
import java.util.List;
24-
2523
import javax.annotation.concurrent.Immutable;
24+
import java.util.ArrayList;
25+
import java.util.List;
2626

2727
/**
2828
* Represents a Optimizely Group configuration
@@ -48,7 +48,24 @@ public Group(@JsonProperty("id") String id,
4848
this.id = id;
4949
this.policy = policy;
5050
this.trafficAllocation = trafficAllocation;
51-
this.experiments = experiments;
51+
// populate experiment's groupId
52+
this.experiments = new ArrayList<>(experiments.size());
53+
for (Experiment experiment : experiments) {
54+
if (id != null && !id.equals(experiment.getGroupId())) {
55+
experiment = new Experiment(
56+
experiment.getId(),
57+
experiment.getKey(),
58+
experiment.getStatus(),
59+
experiment.getLayerId(),
60+
experiment.getAudienceIds(),
61+
experiment.getVariations(),
62+
experiment.getUserIdToVariationKeyMap(),
63+
experiment.getTrafficAllocation(),
64+
id
65+
);
66+
}
67+
this.experiments.add(experiment);
68+
}
5269
}
5370

5471
public String getId() {

‎core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
*/
1717
package com.optimizely.ab.config.audience;
1818

19+
import com.fasterxml.jackson.annotation.JsonCreator;
20+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
21+
import com.fasterxml.jackson.annotation.JsonProperty;
1922
import com.optimizely.ab.config.audience.match.MatchType;
2023

2124
import javax.annotation.Nonnull;
@@ -27,14 +30,19 @@
2730
* Represents a user attribute instance within an audience's conditions.
2831
*/
2932
@Immutable
33+
@JsonIgnoreProperties(ignoreUnknown = true)
3034
public class UserAttribute implements Condition {
3135

3236
private final String name;
3337
private final String type;
3438
private final String match;
3539
private final Object value;
3640

37-
public UserAttribute(@Nonnull String name, @Nonnull String type, @Nullable String match, @Nullable Object value) {
41+
@JsonCreator
42+
public UserAttribute(@JsonProperty("name") @Nonnull String name,
43+
@JsonProperty("type") @Nonnull String type,
44+
@JsonProperty("match") @Nullable String match,
45+
@JsonProperty("value") @Nullable Object value) {
3846
this.name = name;
3947
this.type = type;
4048
this.match = match;
@@ -58,11 +66,11 @@ public Object getValue() {
5866
}
5967

6068
public @Nullable Boolean evaluate(Map<String, ?> attributes) {
61-
// Valid for primative types, but needs to change when a value is an object or an array
69+
// Valid for primitive types, but needs to change when a value is an object or an array
6270
Object userAttributeValue = attributes.get(name);
6371

6472
if (!"custom_attribute".equals(type)) {
65-
MatchType.logger.error(String.format("condition type not equal to `custom_attribute` %s", type != null ? type : ""));
73+
MatchType.logger.error(String.format("condition type not equal to `custom_attribute` %s", type));
6674
return null; // unknown type
6775
}
6876
// check user attribute value is equal
@@ -73,14 +81,22 @@ public Object getValue() {
7381
MatchType.logger.error(String.format("attribute or value null for match %s", match != null ? match : "legacy condition"),np);
7482
return null;
7583
}
76-
7784
}
7885

7986
@Override
8087
public String toString() {
88+
final String valueStr;
89+
if (value == null) {
90+
valueStr = "null";
91+
} else if (value instanceof String) {
92+
valueStr = String.format("'%s'", value);
93+
} else {
94+
valueStr = value.toString();
95+
}
8196
return "{name='" + name + "\'" +
8297
", type='" + type + "\'" +
83-
", value='" + value.toString() + "\'" +
98+
", match='" + match + "\'" +
99+
", value=" + valueStr +
84100
"}";
85101
}
86102

@@ -93,13 +109,15 @@ public boolean equals(Object o) {
93109

94110
if (!name.equals(that.name)) return false;
95111
if (!type.equals(that.type)) return false;
112+
if (match != null ? !match.equals(that.match) : that.match != null) return false;
96113
return value != null ? value.equals(that.value) : that.value == null;
97114
}
98115

99116
@Override
100117
public int hashCode() {
101118
int result = name.hashCode();
102119
result = 31 * result + type.hashCode();
120+
result = 31 * result + (match != null ? match.hashCode() : 0);
103121
result = 31 * result + (value != null ? value.hashCode() : 0);
104122
return result;
105123
}

‎core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2016-2017, Optimizely and contributors
3+
* Copyright 2016-2018, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.

‎core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,51 +17,55 @@
1717
package com.optimizely.ab.config.parser;
1818

1919
import com.fasterxml.jackson.core.JsonParser;
20+
import com.fasterxml.jackson.core.JsonProcessingException;
21+
import com.fasterxml.jackson.core.ObjectCodec;
2022
import com.fasterxml.jackson.databind.DeserializationContext;
2123
import com.fasterxml.jackson.databind.JsonDeserializer;
2224
import com.fasterxml.jackson.databind.JsonNode;
2325
import com.fasterxml.jackson.databind.ObjectMapper;
24-
25-
import com.optimizely.ab.config.audience.Audience;
26-
import com.optimizely.ab.config.audience.AndCondition;
27-
import com.optimizely.ab.config.audience.Condition;
28-
import com.optimizely.ab.config.audience.UserAttribute;
29-
import com.optimizely.ab.config.audience.NotCondition;
30-
import com.optimizely.ab.config.audience.OrCondition;
26+
import com.optimizely.ab.config.audience.*;
3127

3228
import java.io.IOException;
3329
import java.util.ArrayList;
34-
import java.util.HashMap;
3530
import java.util.List;
3631

3732
public class AudienceJacksonDeserializer extends JsonDeserializer<Audience> {
33+
private ObjectMapper objectMapper;
34+
35+
public AudienceJacksonDeserializer() {
36+
this(new ObjectMapper());
37+
}
38+
39+
AudienceJacksonDeserializer(ObjectMapper objectMapper) {
40+
this.objectMapper = objectMapper;
41+
}
3842

3943
@Override
4044
public Audience deserialize(JsonParser parser, DeserializationContext context) throws IOException {
41-
ObjectMapper mapper = new ObjectMapper();
42-
JsonNode node = parser.getCodec().readTree(parser);
45+
ObjectCodec codec = parser.getCodec();
46+
JsonNode node = codec.readTree(parser);
4347

4448
String id = node.get("id").textValue();
4549
String name = node.get("name").textValue();
46-
List<Object> rawObjectList = (List<Object>)mapper.readValue(node.get("conditions").textValue(), List.class);
47-
Condition conditions = parseConditions(rawObjectList);
50+
51+
String conditionsJson = node.get("conditions").textValue();
52+
JsonNode conditionsTree = objectMapper.readTree(conditionsJson);
53+
Condition conditions = parseConditions(conditionsTree);
4854

4955
return new Audience(id, name, conditions);
5056
}
5157

52-
private Condition parseConditions(List<Object> rawObjectList) {
58+
private Condition parseConditions(JsonNode conditionNode) throws JsonProcessingException {
5359
List<Condition> conditions = new ArrayList<Condition>();
54-
String operand = (String)rawObjectList.get(0);
60+
JsonNode opNode = conditionNode.get(0);
61+
String operand = opNode.asText();
5562

56-
for (int i = 1; i < rawObjectList.size(); i++) {
57-
Object obj = rawObjectList.get(i);
58-
if (obj instanceof List) {
59-
List<Object> objectList = (List<Object>)rawObjectList.get(i);
60-
conditions.add(parseConditions(objectList));
61-
} else {
62-
HashMap<String, ?> conditionMap = (HashMap<String, ?>)rawObjectList.get(i);
63-
conditions.add(new UserAttribute((String)conditionMap.get("name"), (String)conditionMap.get("type"),
64-
(String)conditionMap.get("match"), conditionMap.get("value")));
63+
for (int i = 1; i < conditionNode.size(); i++) {
64+
JsonNode subNode = conditionNode.get(i);
65+
if (subNode.isArray()) {
66+
conditions.add(parseConditions(subNode));
67+
} else if (subNode.isObject()) {
68+
conditions.add(objectMapper.treeToValue(subNode, UserAttribute.class));
6569
}
6670
}
6771

@@ -73,7 +77,7 @@ private Condition parseConditions(List<Object> rawObjectList) {
7377
case "or":
7478
condition = new OrCondition(conditions);
7579
break;
76-
default:
80+
default: // this makes two assumptions: operator is "not" and conditions is non-empty...
7781
condition = new NotCondition(conditions.get(0));
7882
break;
7983
}

0 commit comments

Comments
 (0)