github-mirrors/logstash
1
0
Fork 0
mirror of https://github.com/elastic/logstash.git synced 2025-04-24 22:57:16 -04:00
Code Issues Projects Releases Packages Wiki Activity

PERFORMANCE: Remove lut Cache in Accessors

Browse source 
Fixes #7839
This commit is contained in:
Armin 2017-07-28 16:49:51 +02:00 • committed by Armin Braun
parent f639df97cd
commit 23877c0b7c
3 changed files with 157 additions and 306 deletions
Download patch file Download diff file Expand all files Collapse all files

240
logstash-core/src/main/java/org/logstash/Accessors.java
View file

@ -1,201 +1,157 @@
package org.logstash;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
class Accessors {
public final class Accessors {
private final ConvertedMap data;
protected Map<String, Object> lut;
public Accessors(final ConvertedMap data) {
this.data = data;
this.lut = new HashMap<>(); // reference -> target LUT
private Accessors() {
//Utility Class
}
public Object get(String reference) {
FieldReference field = PathCache.cache(reference);
Object target = findTarget(field);
return (target == null) ? null : fetch(target, field.getKey());
}
public Object set(String reference, Object value) {
public static Object get(final ConvertedMap data, final String reference) {
final FieldReference field = PathCache.cache(reference);
final Object target = findCreateTarget(field);
final String key = field.getKey();
if (target instanceof ConvertedMap) {
((ConvertedMap) target).put(key, value);
} else if (target instanceof ConvertedList) {
int i;
try {
i = Integer.parseInt(key);
} catch (NumberFormatException e) {
return null;
}
int size = ((ConvertedList) target).size();
if (i >= size) {
// grow array by adding trailing null items
// this strategy reflects legacy Ruby impl behaviour and is backed by specs
// TODO: (colin) this is potentially dangerous, and could produce OOM using arbitrary big numbers
// TODO: (colin) should be guard against this?
for (int j = size; j < i; j++) {
((List<Object>) target).add(null);
}
((List<Object>) target).add(value);
} else {
int offset = listIndex(i, ((List) target).size());
((ConvertedList) target).set(offset, value);
}
} else {
throw newCollectionException(target);
}
return value;
final Object target = findParent(data, field);
return target == null ? null : fetch(target, field.getKey());
}
public Object del(String reference) {
FieldReference field = PathCache.cache(reference);
Object target = findTarget(field);
if (target != null) {
public static Object set(final ConvertedMap data, final String reference,
final Object value) {
final FieldReference field = PathCache.cache(reference);
return setChild(findCreateTarget(data, field), field.getKey(), value);
}
public static Object del(final ConvertedMap data, final String reference) {
final FieldReference field = PathCache.cache(reference);
final Object target = findParent(data, field);
if (target instanceof ConvertedMap) {
return ((ConvertedMap) target).remove(field.getKey());
} else if (target instanceof ConvertedList) {
try {
int i = Integer.parseInt(field.getKey());
int offset = listIndex(i, ((List) target).size());
return ((List)target).remove(offset);
} catch (IndexOutOfBoundsException|NumberFormatException e) {
return null;
}
} else {
throw newCollectionException(target);
return target == null ? null : delFromList((ConvertedList) target, field.getKey());
}
}
return null;
}
public boolean includes(String reference) {
public static boolean includes(final ConvertedMap data, final String reference) {
final FieldReference field = PathCache.cache(reference);
final Object target = findTarget(field);
final Object target = findParent(data, field);
final String key = field.getKey();
return target instanceof ConvertedMap && ((ConvertedMap) target).containsKey(key) ||
target instanceof ConvertedList && foundInList(key, (ConvertedList) target);
}
private static boolean foundInList(final String key, final ConvertedList target) {
private static Object delFromList(final ConvertedList list, final String key) {
try {
return foundInList(target, Integer.parseInt(key));
} catch (NumberFormatException e) {
return false;
}
}
private Object findTarget(FieldReference field) {
final Object target = this.lut.get(field.getReference());
return target != null ? target : cacheTarget(field);
}
private Object cacheTarget(final FieldReference field) {
Object target = this.data;
for (final String key : field.getPath()) {
target = fetch(target, key);
if (!isCollection(target)) {
return list.remove(listIndex(key, list.size()));
} catch (IndexOutOfBoundsException | NumberFormatException e) {
return null;
}
}
private static Object setOnList(final String key, final Object value, final ConvertedList list) {
final int index;
try {
index = Integer.parseInt(key);
} catch (NumberFormatException e) {
return null;
}
final int size = list.size();
if (index >= size) {
appendAtIndex(list, value, index, size);
} else {
list.set(listIndex(index, size), value);
}
return value;
}
private static void appendAtIndex(final ConvertedList list, final Object value, final int index,
final int size) {
// grow array by adding trailing null items
// this strategy reflects legacy Ruby impl behaviour and is backed by specs
// TODO: (colin) this is potentially dangerous, and could produce OOM using arbitrary big numbers
// TODO: (colin) should be guard against this?
for (int i = size; i < index; i++) {
list.add(null);
}
list.add(value);
}
private static Object findParent(final ConvertedMap data, final FieldReference field) {
Object target = data;
for (final String key : field.getPath()) {
target = fetch(target, key);
if (!(target instanceof ConvertedMap || target instanceof ConvertedList)) {
return null;
}
}
this.lut.put(field.getReference(), target);
return target;
}
private Object findCreateTarget(FieldReference field) {
Object target;
// flush the @lut to prevent stale cached fieldref which may point to an old target
// which was overwritten with a new value. for example, if "[a][b]" is cached and we
// set a new value for "[a]" then reading again "[a][b]" would point in a stale target.
// flushing the complete @lut is suboptimal, but a hierarchical lut would be required
// to be able to invalidate fieldrefs from a common root.
// see https://github.com/elastic/logstash/pull/5132
this.lut.clear();
target = this.data;
for (String key : field.getPath()) {
Object result = fetch(target, key);
if (result == null) {
private static Object findCreateTarget(final ConvertedMap data, final FieldReference field) {
Object target = data;
boolean create = false;
for (final String key : field.getPath()) {
Object result;
if (create) {
result = createChild((ConvertedMap) target, key);
} else {
result = fetch(target, key);
create = result == null;
if (create) {
result = new ConvertedMap(1);
if (target instanceof ConvertedMap) {
((ConvertedMap) target).put(key, result);
} else if (target instanceof ConvertedList) {
try {
int i = Integer.parseInt(key);
// TODO: what about index out of bound?
((ConvertedList) target).set(i, result);
} catch (NumberFormatException e) {
continue;
}
} else if (target != null) {
throw newCollectionException(target);
setChild(target, key, result);
}
}
target = result;
}
this.lut.put(field.getReference(), target);
return target;
}
private static boolean foundInList(ConvertedList target, int index) {
try {
int offset = listIndex(index, target.size());
return target.get(offset) != null;
} catch (IndexOutOfBoundsException e) {
return false;
private static Object setChild(final Object target, final String key, final Object value) {
if (target instanceof Map) {
((ConvertedMap) target).put(key, value);
return value;
} else {
return setOnList(key, value, (ConvertedList) target);
}
}
private static Object createChild(final ConvertedMap target, final String key) {
final Object result = new ConvertedMap(1);
target.put(key, result);
return result;
}
private static Object fetch(Object target, String key) {
if (target instanceof ConvertedMap) {
return ((ConvertedMap) target).get(key);
} else if (target instanceof ConvertedList) {
return target instanceof ConvertedMap
? ((ConvertedMap) target).get(key) : fetchFromList((ConvertedList) target, key);
}
private static Object fetchFromList(final ConvertedList list, final String key) {
try {
int offset = listIndex(Integer.parseInt(key), ((ConvertedList) target).size());
return ((ConvertedList) target).get(offset);
return list.get(listIndex(key, list.size()));
} catch (IndexOutOfBoundsException | NumberFormatException e) {
return null;
}
} else if (target == null) {
return null;
} else {
throw newCollectionException(target);
}
}
private static boolean isCollection(Object target) {
return target instanceof ConvertedList || target instanceof ConvertedMap;
private static boolean foundInList(final String key, final ConvertedList target) {
return fetchFromList(target, key) != null;
}
private static ClassCastException newCollectionException(Object target) {
return new ClassCastException("expecting ConvertedList or ConvertedMap, found " + target.getClass());
}
/*
/**
* Returns a positive integer offset for a list of known size.
*
* @param i if positive, and offset from the start of the list. If negative, the offset from the end of the list, where -1 means the last element.
* @param size the size of the list.
* @return the positive integer offset for the list given by index i.
*/
public static int listIndex(int i, int size) {
if (i >= size || i < -size) {
throw new IndexOutOfBoundsException("Index " + i + " is out of bounds for a list with size " + size);
return i < 0 ? size + i : i;
}
if (i < 0) { // Offset from the end of the array.
return size + i;
} else {
return i;
}
/**
* Returns a positive integer offset for a list of known size.
* @param size the size of the list.
* @return the positive integer offset for the list given by index i.
*/
private static int listIndex(final String key, final int size) {
return listIndex(Integer.parseInt(key), size);
}
}

35
logstash-core/src/main/java/org/logstash/Event.java
View file

@ -26,8 +26,6 @@ public final class Event implements Cloneable, Queueable {
private ConvertedMap data;
private ConvertedMap metadata;
private Timestamp timestamp;
private Accessors accessors;
private Accessors metadata_accessors;
public static final String METADATA = "@metadata";
public static final String METADATA_BRACKETS = "[" + METADATA + "]";
@ -49,8 +47,6 @@ public final class Event implements Cloneable, Queueable {
this.cancelled = false;
this.timestamp = new Timestamp();
this.data.put(TIMESTAMP, this.timestamp);
this.accessors = new Accessors(this.data);
this.metadata_accessors = new Accessors(this.metadata);
}
/**
@ -79,8 +75,6 @@ public final class Event implements Cloneable, Queueable {
} else {
this.metadata = new ConvertedMap(10);
}
this.metadata_accessors = new Accessors(this.metadata);
this.cancelled = false;
Object providedTimestamp = data.get(TIMESTAMP);
@ -89,7 +83,6 @@ public final class Event implements Cloneable, Queueable {
this.timestamp = (parsedTimestamp == null) ? Timestamp.now() : parsedTimestamp;
this.data.put(TIMESTAMP, this.timestamp);
this.accessors = new Accessors(this.data);
// the tag() method has to be called after the Accessors initialization
if (parsedTimestamp == null) {
@ -106,10 +99,6 @@ public final class Event implements Cloneable, Queueable {
return this.metadata;
}
private Accessors getAccessors() {
return this.accessors;
}
public void cancel() {
this.cancelled = true;
}
@ -144,23 +133,22 @@ public final class Event implements Cloneable, Queueable {
if (reference.equals(METADATA)) {
return this.metadata;
} else if (reference.startsWith(METADATA_BRACKETS)) {
return this.metadata_accessors.get(reference.substring(METADATA_BRACKETS.length()));
return Accessors.get(metadata, reference.substring(METADATA_BRACKETS.length()));
} else {
return this.accessors.get(reference);
return Accessors.get(data, reference);
}
}
public void setField(String reference, Object value) {
if (reference.equals(TIMESTAMP)) {
// TODO(talevy): check type of timestamp
this.accessors.set(reference, value);
Accessors.set(data, reference, value);
} else if (reference.equals(METADATA_BRACKETS) || reference.equals(METADATA)) {
this.metadata = ConvertedMap.newFromMap((Map) value);
this.metadata_accessors = new Accessors(this.metadata);
} else if (reference.startsWith(METADATA_BRACKETS)) {
this.metadata_accessors.set(reference.substring(METADATA_BRACKETS.length()), value);
Accessors.set(metadata, reference.substring(METADATA_BRACKETS.length()), value);
} else {
this.accessors.set(reference, Valuefier.convert(value));
Accessors.set(data, reference, Valuefier.convert(value));
}
}
@ -168,9 +156,9 @@ public final class Event implements Cloneable, Queueable {
if (reference.equals(METADATA_BRACKETS) || reference.equals(METADATA)) {
return true;
} else if (reference.startsWith(METADATA_BRACKETS)) {
return this.metadata_accessors.includes(reference.substring(METADATA_BRACKETS.length()));
return Accessors.includes(metadata, reference.substring(METADATA_BRACKETS.length()));
} else {
return this.accessors.includes(reference);
return Accessors.includes(data, reference);
}
}
@ -243,7 +231,6 @@ public final class Event implements Cloneable, Queueable {
public Event overwrite(Event e) {
this.data = e.getData();
this.accessors = e.getAccessors();
this.cancelled = e.isCancelled();
try {
this.timestamp = e.getTimestamp();
@ -261,7 +248,7 @@ public final class Event implements Cloneable, Queueable {
}
public Object remove(String path) {
return this.accessors.del(path);
return Accessors.del(data, path);
}
public String sprintf(String s) throws IOException {
@ -331,7 +318,7 @@ public final class Event implements Cloneable, Queueable {
}
public void tag(final String tag) {
final Object tags = accessors.get("tags");
final Object tags = Accessors.get(data,"tags");
// short circuit the null case where we know we won't need deduplication step below at the end
if (tags == null) {
initTag(tag);
@ -347,7 +334,7 @@ public final class Event implements Cloneable, Queueable {
private void initTag(final String tag) {
final ConvertedList list = new ConvertedList(1);
list.add(new StringBiValue(tag));
accessors.set("tags", list);
Accessors.set(data, "tags", list);
}
/**
@ -373,7 +360,7 @@ public final class Event implements Cloneable, Queueable {
// TODO: we should eventually look into using alternate data structures to do more efficient dedup but that will require properly defining the tagging API too
if (!tags.contains(tag)) {
tags.add(tag);
accessors.set("tags", ConvertedList.newFromList(tags));
Accessors.set(data,"tags", ConvertedList.newFromList(tags));
}
}

188
logstash-core/src/test/java/org/logstash/AccessorsTest.java
View file

@ -5,13 +5,7 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.theories.DataPoint;
import org.junit.experimental.theories.Theories;
import org.junit.experimental.theories.Theory;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.logstash.bivalues.StringBiValue;
import static org.junit.Assert.assertEquals;
@ -21,113 +15,68 @@ import static org.junit.Assert.assertTrue;
public class AccessorsTest {
private static final class TestableAccessors extends Accessors {
public TestableAccessors(ConvertedMap data) {
super(data);
}
public Object lutGet(String reference) {
return this.lut.get(reference);
}
}
@Test
public void testBareGet() throws Exception {
final ConvertedMap data = new ConvertedMap(1);
Map<Serializable, Object> data = new HashMap<>();
data.put("foo", "bar");
String reference = "foo";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals("bar", accessors.get(reference));
assertEquals(data, accessors.lutGet(reference));
assertEquals(new StringBiValue("bar"), Accessors.get(ConvertedMap.newFromMap(data), reference));
}
@Test
public void testAbsentBareGet() throws Exception {
final Map<Serializable, Object> java = new HashMap<>(1);
java.put("foo", "bar");
final ConvertedMap data = ConvertedMap.newFromMap(java);
Map<Serializable, Object> data = new HashMap<>();
data.put("foo", "bar");
String reference = "baz";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertNull(accessors.get(reference));
assertEquals(data, accessors.lutGet(reference));
assertNull(Accessors.get(ConvertedMap.newFromMap(data), reference));
}
@Test
public void testBareBracketsGet() throws Exception {
final Map<Serializable, Object> java = new HashMap<>(1);
java.put("foo", "bar");
final ConvertedMap data = ConvertedMap.newFromMap(java);
Map<Serializable, Object> data = new HashMap<>();
data.put("foo", "bar");
String reference = "[foo]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals(new StringBiValue("bar"), accessors.get(reference));
assertEquals(data, accessors.lutGet(reference));
assertEquals(new StringBiValue("bar"), Accessors.get(ConvertedMap.newFromMap(data), reference));
}
@Test
public void testDeepMapGet() throws Exception {
final ConvertedMap data = new ConvertedMap(1);
Map<String, Object> inner = new ConvertedMap(1);
Map<Serializable, Object> data = new HashMap<>();
Map<Serializable, Object> inner = new HashMap<>();
data.put("foo", inner);
inner.put("bar", "baz");
String reference = "[foo][bar]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals("baz", accessors.get(reference));
assertEquals(inner, accessors.lutGet(reference));
assertEquals(new StringBiValue("baz"), Accessors.get(ConvertedMap.newFromMap(data), reference));
}
@Test
public void testAbsentDeepMapGet() throws Exception {
final ConvertedMap data = new ConvertedMap(1);
Map<String, Object> inner = new ConvertedMap(1);
Map<Serializable, Object> data = new HashMap<>();
Map<Serializable, Object> inner = new HashMap<>();
data.put("foo", inner);
inner.put("bar", "baz");
String reference = "[foo][foo]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertNull(accessors.get(reference));
assertEquals(inner, accessors.lutGet(reference));
assertNull(Accessors.get(ConvertedMap.newFromMap(data), reference));
}
@Test
public void testDeepListGet() throws Exception {
final ConvertedMap data = new ConvertedMap(1);
List inner = new ConvertedList(1);
Map<Serializable, Object> data = new HashMap<>();
List inner = new ArrayList();
data.put("foo", inner);
inner.add("bar");
String reference = "[foo][0]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals("bar", accessors.get(reference));
assertEquals(inner, accessors.lutGet(reference));
assertEquals(new StringBiValue("bar"), Accessors.get(ConvertedMap.newFromMap(data), reference));
}
@Test
public void testAbsentDeepListGet() throws Exception {
final ConvertedMap data = new ConvertedMap(1);
List inner = new ConvertedList(1);
Map<Serializable, Object> data = new HashMap<>();
List inner = new ArrayList();
data.put("foo", inner);
inner.add("bar");
String reference = "[foo][1]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertNull(accessors.get(reference));
assertEquals(inner, accessors.lutGet(reference));
assertNull(Accessors.get(ConvertedMap.newFromMap(data), reference));
}
/*
* Check if accessors are able to recovery from
@ -138,32 +87,25 @@ public class AccessorsTest {
@Test
public void testInvalidIdList() throws Exception {
final ConvertedMap data = new ConvertedMap(1);
List inner = new ConvertedList(1);
List inner = new ConvertedList(2);
data.put("map1", inner);
inner.add("obj1");
inner.add("obj2");
String reference = "[map1][IdNonNumeric]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertNull(accessors.get(reference));
assertNull(accessors.set(reference, "obj3"));
assertEquals(inner, accessors.lutGet(reference));
assertFalse(accessors.includes(reference));
assertNull(accessors.del(reference));
assertNull(Accessors.get(data, reference));
assertNull(Accessors.set(data, reference, "obj3"));
assertFalse(Accessors.includes(data, reference));
assertNull(Accessors.del(data, reference));
}
@Test
public void testBarePut() throws Exception {
final ConvertedMap data = new ConvertedMap(1);
String reference = "foo";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals("bar", accessors.set(reference, "bar"));
assertEquals(data, accessors.lutGet(reference));
assertEquals("bar", accessors.get(reference));
assertEquals("bar", Accessors.set(data, reference, "bar"));
assertEquals("bar", Accessors.get(data, reference));
}
@Test
@ -171,11 +113,8 @@ public class AccessorsTest {
final ConvertedMap data = new ConvertedMap(1);
String reference = "[foo]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals("bar", accessors.set(reference, "bar"));
assertEquals(data, accessors.lutGet(reference));
assertEquals("bar", accessors.get(reference));
assertEquals("bar", Accessors.set(data, reference, "bar"));
assertEquals("bar", Accessors.get(data, reference));
}
@Test
@ -184,11 +123,8 @@ public class AccessorsTest {
String reference = "[foo][bar]";
TestableAccessors accessors = new TestableAccessors(data);
assertNull(accessors.lutGet(reference));
assertEquals("baz", accessors.set(reference, "baz"));
assertEquals(accessors.lutGet(reference), data.get("foo"));
assertEquals("baz", accessors.get(reference));
assertEquals("baz", Accessors.set(data, reference, "baz"));
assertEquals("baz", Accessors.get(data, reference));
}
@Test
@ -198,44 +134,40 @@ public class AccessorsTest {
data.put("foo", inner);
inner.add("bar");
data.put("bar", "baz");
TestableAccessors accessors = new TestableAccessors(data);
assertEquals("bar", accessors.del("[foo][0]"));
assertNull(accessors.del("[foo][0]"));
assertEquals(new ArrayList<>(), accessors.get("[foo]"));
assertEquals("baz", accessors.del("[bar]"));
assertNull(accessors.get("[bar]"));
assertEquals("bar", Accessors.del(data, "[foo][0]"));
assertNull(Accessors.del(data, "[foo][0]"));
assertEquals(new ConvertedList(0), Accessors.get(data,"[foo]"));
assertEquals("baz", Accessors.del(data, "[bar]"));
assertNull(Accessors.get(data, "[bar]"));
}
@Test
public void testNilInclude() throws Exception {
ConvertedMap data = new ConvertedMap(1);
final ConvertedMap data = new ConvertedMap(1);
data.put("nilfield", null);
TestableAccessors accessors = new TestableAccessors(data);
assertTrue(accessors.includes("nilfield"));
assertTrue(Accessors.includes(data, "nilfield"));
}
@Test
public void testInvalidPath() throws Exception {
ConvertedMap data = new ConvertedMap(1);
Accessors accessors = new Accessors(data);
final ConvertedMap data = new ConvertedMap(1);
assertEquals(1, accessors.set("[foo]", 1));
assertNull(accessors.get("[foo][bar]"));
assertEquals(1, Accessors.set(data, "[foo]", 1));
assertNull(Accessors.get(data, "[foo][bar]"));
}
@Test
public void testStaleTargetCache() throws Exception {
ConvertedMap data = new ConvertedMap(1);
final ConvertedMap data = new ConvertedMap(1);
Accessors accessors = new Accessors(data);
assertNull(accessors.get("[foo][bar]"));
assertEquals("baz", accessors.set("[foo][bar]", "baz"));
assertEquals("baz", accessors.get("[foo][bar]"));
assertNull(Accessors.get(data,"[foo][bar]"));
assertEquals("baz", Accessors.set(data,"[foo][bar]", "baz"));
assertEquals("baz", Accessors.get(data, "[foo][bar]"));
assertEquals("boom", accessors.set("[foo]", "boom"));
assertNull(accessors.get("[foo][bar]"));
assertEquals("boom", accessors.get("[foo]"));
assertEquals("boom", Accessors.set(data, "[foo]", "boom"));
assertNull(Accessors.get(data, "[foo][bar]"));
assertEquals("boom", Accessors.get(data,"[foo]"));
}
@Test
@ -247,28 +179,4 @@ public class AccessorsTest {
assertEquals(1, Accessors.listIndex(-9, 10));
assertEquals(0, Accessors.listIndex(-10, 10));
}
@RunWith(Theories.class)
public static class TestListIndexFailureCases {
private static final int size = 10;
@DataPoint
public static final int tooLarge = size;
@DataPoint
public static final int tooLarge1 = size+1;
@DataPoint
public static final int tooLargeNegative = -size - 1;
@Rule
public ExpectedException exception = ExpectedException.none();
@Theory
public void testListIndexOutOfBounds(int i) {
exception.expect(IndexOutOfBoundsException.class);
Accessors.listIndex(i, size);
}
}
}