Skip to content

Performance improvements for JacksonConfigParser #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# EditorConfig is awesome: http://EditorConfig.org
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty cool! Will start using it in our projects :)


# top-most EditorConfig file
root = true

# 4 space indentation
[*.{py,java}]
indent_style = space
indent_size = 4
11 changes: 7 additions & 4 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ buildscript {
plugins {
id 'com.github.kt3k.coveralls' version '2.8.2'
id 'jacoco'
id 'me.champeau.gradle.jmh' version '0.3.1'
id 'me.champeau.gradle.jmh' version '0.4.5'
id 'nebula.optional-base' version '3.2.0'
}

Expand Down Expand Up @@ -79,6 +79,10 @@ subprojects {
}
}

findbugs {
findbugsJmh.enabled = false
}

test {
useJUnit {
excludeCategories 'com.optimizely.ab.categories.ExhaustiveTest'
Expand All @@ -104,9 +108,8 @@ subprojects {
}

dependencies {
afterEvaluate {
jmh configurations.testCompile.allDependencies
}
jmh 'org.openjdk.jmh:jmh-core:1.12'
jmh 'org.openjdk.jmh:jmh-generator-annprocess:1.12'
}

dependencies {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
*
* Copyright 2018, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.optimizely.ab.config.parser;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the license header to this file. You only need to include the year 2018 because it was created this year.


import com.optimizely.ab.config.ProjectConfig;
import com.optimizely.ab.config.ProjectConfigTestUtils;
import org.openjdk.jmh.annotations.*;

import java.io.IOException;
import java.util.concurrent.TimeUnit;

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
@Fork(2)
@Warmup(iterations = 10)
@Measurement(iterations = 20)
@State(Scope.Benchmark)
public class JacksonConfigParserBenchmark {
JacksonConfigParser parser;
String jsonV2;
String jsonV3;
String jsonV4;

@Setup
public void setUp() throws IOException {
parser = new JacksonConfigParser();
jsonV2 = ProjectConfigTestUtils.validConfigJsonV2();
jsonV3 = ProjectConfigTestUtils.validConfigJsonV3();
jsonV4 = ProjectConfigTestUtils.validConfigJsonV4();
}

@Benchmark
public ProjectConfig parseV2() throws ConfigParseException {
return parser.parseProjectConfig(jsonV2);
}

@Benchmark
public ProjectConfig parseV3() throws ConfigParseException {
return parser.parseProjectConfig(jsonV3);
}

@Benchmark
public ProjectConfig parseV4() throws ConfigParseException {
return parser.parseProjectConfig(jsonV4);
}
}
13 changes: 5 additions & 8 deletions core-api/src/main/java/com/optimizely/ab/config/Experiment.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2016-2017, Optimizely and contributors
* Copyright 2016-2018, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,17 +16,14 @@
*/
package com.optimizely.ab.config;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import com.fasterxml.jackson.annotation.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the license year to 2016-2018 for this and all the files you touched.


import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import java.util.Collections;
import java.util.List;
import java.util.Map;

/**
* Represents the Optimizely Experiment configuration.
Expand Down
25 changes: 21 additions & 4 deletions core-api/src/main/java/com/optimizely/ab/config/Group.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2016-2017, Optimizely and contributors
* Copyright 2016-2018, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,9 +20,9 @@
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;

import java.util.List;

import javax.annotation.concurrent.Immutable;
import java.util.ArrayList;
import java.util.List;

/**
* Represents a Optimizely Group configuration
Expand All @@ -48,7 +48,24 @@ public Group(@JsonProperty("id") String id,
this.id = id;
this.policy = policy;
this.trafficAllocation = trafficAllocation;
this.experiments = experiments;
// populate experiment's groupId
this.experiments = new ArrayList<>(experiments.size());
for (Experiment experiment : experiments) {
if (id != null && !id.equals(experiment.getGroupId())) {
experiment = new Experiment(
experiment.getId(),
experiment.getKey(),
experiment.getStatus(),
experiment.getLayerId(),
experiment.getAudienceIds(),
experiment.getVariations(),
experiment.getUserIdToVariationKeyMap(),
experiment.getTrafficAllocation(),
id
);
}
this.experiments.add(experiment);
}
}

public String getId() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
*/
package com.optimizely.ab.config.audience;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.optimizely.ab.config.audience.match.MatchType;

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

private final String name;
private final String type;
private final String match;
private final Object value;

public UserAttribute(@Nonnull String name, @Nonnull String type, @Nullable String match, @Nullable Object value) {
@JsonCreator
public UserAttribute(@JsonProperty("name") @Nonnull String name,
@JsonProperty("type") @Nonnull String type,
@JsonProperty("match") @Nullable String match,
@JsonProperty("value") @Nullable Object value) {
this.name = name;
this.type = type;
this.match = match;
Expand All @@ -58,11 +66,11 @@ public Object getValue() {
}

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

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

}

@Override
public String toString() {
final String valueStr;
if (value == null) {
valueStr = "null";
} else if (value instanceof String) {
valueStr = String.format("'%s'", value);
} else {
valueStr = value.toString();
}
return "{name='" + name + "\'" +
", type='" + type + "\'" +
", value='" + value.toString() + "\'" +
", match='" + match + "\'" +
", value=" + valueStr +
"}";
}

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

if (!name.equals(that.name)) return false;
if (!type.equals(that.type)) return false;
if (match != null ? !match.equals(that.match) : that.match != null) return false;
return value != null ? value.equals(that.value) : that.value == null;
}

@Override
public int hashCode() {
int result = name.hashCode();
result = 31 * result + type.hashCode();
result = 31 * result + (match != null ? match.hashCode() : 0);
result = 31 * result + (value != null ? value.hashCode() : 0);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2016-2017, Optimizely and contributors
* Copyright 2016-2018, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,51 +17,55 @@
package com.optimizely.ab.config.parser;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.ObjectCodec;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;

import com.optimizely.ab.config.audience.Audience;
import com.optimizely.ab.config.audience.AndCondition;
import com.optimizely.ab.config.audience.Condition;
import com.optimizely.ab.config.audience.UserAttribute;
import com.optimizely.ab.config.audience.NotCondition;
import com.optimizely.ab.config.audience.OrCondition;
import com.optimizely.ab.config.audience.*;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;

public class AudienceJacksonDeserializer extends JsonDeserializer<Audience> {
private ObjectMapper objectMapper;

public AudienceJacksonDeserializer() {
this(new ObjectMapper());
}

AudienceJacksonDeserializer(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
}

@Override
public Audience deserialize(JsonParser parser, DeserializationContext context) throws IOException {
ObjectMapper mapper = new ObjectMapper();
JsonNode node = parser.getCodec().readTree(parser);
ObjectCodec codec = parser.getCodec();
JsonNode node = codec.readTree(parser);

String id = node.get("id").textValue();
String name = node.get("name").textValue();
List<Object> rawObjectList = (List<Object>)mapper.readValue(node.get("conditions").textValue(), List.class);
Condition conditions = parseConditions(rawObjectList);

String conditionsJson = node.get("conditions").textValue();
JsonNode conditionsTree = objectMapper.readTree(conditionsJson);
Condition conditions = parseConditions(conditionsTree);

return new Audience(id, name, conditions);
}

private Condition parseConditions(List<Object> rawObjectList) {
private Condition parseConditions(JsonNode conditionNode) throws JsonProcessingException {
List<Condition> conditions = new ArrayList<Condition>();
String operand = (String)rawObjectList.get(0);
JsonNode opNode = conditionNode.get(0);
String operand = opNode.asText();

for (int i = 1; i < rawObjectList.size(); i++) {
Object obj = rawObjectList.get(i);
if (obj instanceof List) {
List<Object> objectList = (List<Object>)rawObjectList.get(i);
conditions.add(parseConditions(objectList));
} else {
HashMap<String, ?> conditionMap = (HashMap<String, ?>)rawObjectList.get(i);
conditions.add(new UserAttribute((String)conditionMap.get("name"), (String)conditionMap.get("type"),
(String)conditionMap.get("match"), conditionMap.get("value")));
for (int i = 1; i < conditionNode.size(); i++) {
JsonNode subNode = conditionNode.get(i);
if (subNode.isArray()) {
conditions.add(parseConditions(subNode));
} else if (subNode.isObject()) {
conditions.add(objectMapper.treeToValue(subNode, UserAttribute.class));
}
}

Expand All @@ -73,7 +77,7 @@ private Condition parseConditions(List<Object> rawObjectList) {
case "or":
condition = new OrCondition(conditions);
break;
default:
default: // this makes two assumptions: operator is "not" and conditions is non-empty...
condition = new NotCondition(conditions.get(0));
break;
}
Expand Down
Loading