From 81711d551f75975c5caa5f4746333d0ac318908e Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Tue, 27 Mar 2018 19:30:18 +0200 Subject: [PATCH] fix performance regression The last improvement of memory usage introduced a performance regression. The ingestion performance dropped by 50%-80%, because for every inserted entry the Tags were created inefficient. --- .../main/java/org/lucares/pdb/api/Entry.java | 2 +- .../lucares/pdb/api/TagByKeyComparator.java | 15 ++++++ .../main/java/org/lucares/pdb/api/Tags.java | 54 +++++++------------ .../java/org/lucares/pdb/api/TagsBuilder.java | 22 ++++++++ .../java/org/lucares/memory/MemoryScale.java | 17 +++--- .../java/org/lucares/pdbui/TcpIngestor.java | 11 ++-- .../lucares/performance/db/PerformanceDb.java | 2 +- 7 files changed, 76 insertions(+), 47 deletions(-) create mode 100644 pdb-api/src/main/java/org/lucares/pdb/api/TagByKeyComparator.java create mode 100644 pdb-api/src/main/java/org/lucares/pdb/api/TagsBuilder.java diff --git a/pdb-api/src/main/java/org/lucares/pdb/api/Entry.java b/pdb-api/src/main/java/org/lucares/pdb/api/Entry.java index 4b27981..eb32425 100644 --- a/pdb-api/src/main/java/org/lucares/pdb/api/Entry.java +++ b/pdb-api/src/main/java/org/lucares/pdb/api/Entry.java @@ -65,7 +65,7 @@ public class Entry { } final OffsetDateTime date = getDate(); - return date.format(DateTimeFormatter.ISO_ZONED_DATE_TIME) + " = " + value + " (" + tags + ")"; + return date.format(DateTimeFormatter.ISO_ZONED_DATE_TIME) + " = " + value + " (" + tags.asString() + ")"; } @Override diff --git a/pdb-api/src/main/java/org/lucares/pdb/api/TagByKeyComparator.java b/pdb-api/src/main/java/org/lucares/pdb/api/TagByKeyComparator.java new file mode 100644 index 0000000..6bf2e9b --- /dev/null +++ b/pdb-api/src/main/java/org/lucares/pdb/api/TagByKeyComparator.java @@ -0,0 +1,15 @@ +package org.lucares.pdb.api; + +import java.io.Serializable; +import java.util.Comparator; + +public class TagByKeyComparator implements Comparator, Serializable { + + private static final long serialVersionUID = -6683582291996307323L; + public static final TagByKeyComparator INSTANCE = new TagByKeyComparator(); + + @Override + public int compare(final Tag a, final Tag b) { + return a.getKey().compareToIgnoreCase(b.getKey()); + } +} diff --git a/pdb-api/src/main/java/org/lucares/pdb/api/Tags.java b/pdb-api/src/main/java/org/lucares/pdb/api/Tags.java index 2911c47..6fb8174 100644 --- a/pdb-api/src/main/java/org/lucares/pdb/api/Tags.java +++ b/pdb-api/src/main/java/org/lucares/pdb/api/Tags.java @@ -1,5 +1,7 @@ package org.lucares.pdb.api; +import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.Set; @@ -47,49 +49,30 @@ public class Tags { } } + public static Tags create(final Collection tags) { + final String newFilename = toFilename(tags); + + return new Tags(newFilename); + } + public static Tags create() { return EMPTY; } public static Tags create(final String key, final String value) { - return EMPTY.copyAdd(key, value); + return TagsBuilder.create().add(key, value).build(); } public static Tags create(final String key1, final String value1, final String key2, final String value2) { - final Tags result = EMPTY.copyAdd(key1, value1).copyAdd(key2, value2); + final Tags result = TagsBuilder.create().add(key1, value1).add(key2, value2).build(); return result; } public static Tags create(final String key1, final String value1, final String key2, final String value2, final String key3, final String value3) { - final Tags result = EMPTY.copyAdd(key1, value1).copyAdd(key2, value2).copyAdd(key3, value3); - return result; - } - - public Tags copyAdd(final String key, final String value) { - Objects.requireNonNull(key, "key must not be null"); - Objects.requireNonNull(value, "value must not be null"); - - final Tag tag = new Tag(key, value); - - final SortedSet tags = toTags(); - tags.add(tag); - - final String newFilename = toFilename(tags); - - return new Tags(newFilename); - } - - public Tags copyAddIfNotNull(final String key, final String value) { - - final Tags result; - if (value != null) { - result = copyAdd(key, value); - } else { - result = this; - } + final Tags result = TagsBuilder.create().add(key1, value1).add(key2, value2).add(key3, value3).build(); return result; } @@ -109,7 +92,7 @@ public class Tags { } private SortedSet toTags() { - final SortedSet result = new TreeSet<>((a, b) -> a.getKey().compareToIgnoreCase(b.getKey())); + final SortedSet result = new TreeSet<>(TagByKeyComparator.INSTANCE); final Matcher matcher = EXTRACT_TAGS_PATTERN.matcher(filename); if (matcher.find()) { @@ -132,9 +115,12 @@ public class Tags { return result; } - public String toFilename(final SortedSet tags) { + private static String toFilename(final Collection tags) { final StringBuilder path = new StringBuilder(); + final Tag[] tagsAsArray = tags.toArray(new Tag[tags.size()]); + Arrays.sort(tagsAsArray, TagByKeyComparator.INSTANCE); + for (final Tag tag : tags) { final String key = tag.getKey(); final String value = tag.getValue(); @@ -204,17 +190,17 @@ public class Tags { public Tags subset(final List groupByFields) { - Tags result = new Tags(); + final TagsBuilder result = TagsBuilder.create(); for (final String field : groupByFields) { final String value = getValue(field); if (value != null) { - result = result.copyAdd(field, value); + result.add(field, value); } } - return result; + return result.build(); } public boolean isEmpty() { @@ -236,7 +222,7 @@ public class Tags { } result.append(tag.getKey()); - result.append(":"); + result.append("="); result.append(tag.getValue()); } diff --git a/pdb-api/src/main/java/org/lucares/pdb/api/TagsBuilder.java b/pdb-api/src/main/java/org/lucares/pdb/api/TagsBuilder.java new file mode 100644 index 0000000..e1e5862 --- /dev/null +++ b/pdb-api/src/main/java/org/lucares/pdb/api/TagsBuilder.java @@ -0,0 +1,22 @@ +package org.lucares.pdb.api; + +import java.util.ArrayList; +import java.util.List; + +public class TagsBuilder { + + final List tags = new ArrayList<>(); + + public static TagsBuilder create() { + return new TagsBuilder(); + } + + public TagsBuilder add(final String key, final String value) { + tags.add(new Tag(key, value)); + return this; + } + + public Tags build() { + return Tags.create(tags); + } +} diff --git a/pdb-api/src/test/java/org/lucares/memory/MemoryScale.java b/pdb-api/src/test/java/org/lucares/memory/MemoryScale.java index 7b8c8ee..c7f68bd 100644 --- a/pdb-api/src/test/java/org/lucares/memory/MemoryScale.java +++ b/pdb-api/src/test/java/org/lucares/memory/MemoryScale.java @@ -9,6 +9,7 @@ import java.util.Map; import org.lucares.pdb.api.StringCompressor; import org.lucares.pdb.api.Tag; import org.lucares.pdb.api.Tags; +import org.lucares.pdb.api.TagsBuilder; import org.lucares.pdb.api.UniqueStringIntegerPairs; public class MemoryScale { @@ -70,7 +71,6 @@ public class MemoryScale { default: return null; } - } private static Object createTag() { @@ -90,13 +90,14 @@ public class MemoryScale { } private static Object createTags6() { - Tags result = Tags.create("k1", "v1"); - result = result.copyAdd("k2", "v2"); - result = result.copyAdd("k3", "v3"); - result = result.copyAdd("k4", "v4"); - result = result.copyAdd("k5", "v5"); - result = result.copyAdd("k6", "v6"); - return result; + TagsBuilder result = TagsBuilder.create(); + result = result.add("k1", "v1"); + result = result.add("k2", "v2"); + result = result.add("k3", "v3"); + result = result.add("k4", "v4"); + result = result.add("k5", "v5"); + result = result.add("k6", "v6"); + return result.build(); } private static Object createPathAsUtf8(final String string) { diff --git a/pdb-ui/src/main/java/org/lucares/pdbui/TcpIngestor.java b/pdb-ui/src/main/java/org/lucares/pdbui/TcpIngestor.java index d75d296..5a0d9c7 100644 --- a/pdb-ui/src/main/java/org/lucares/pdbui/TcpIngestor.java +++ b/pdb-ui/src/main/java/org/lucares/pdbui/TcpIngestor.java @@ -24,6 +24,7 @@ import javax.annotation.PreDestroy; import org.lucares.pdb.api.Entry; import org.lucares.pdb.api.Tags; +import org.lucares.pdb.api.TagsBuilder; import org.lucares.performance.db.BlockingQueueIterator; import org.lucares.performance.db.PerformanceDb; import org.lucares.recommind.logs.Config; @@ -124,7 +125,7 @@ public class TcpIngestor implements Ingestor, AutoCloseable, DisposableBean { } private Tags createTags(final Map map) { - Tags tags = Tags.create(); + final TagsBuilder tags = TagsBuilder.create(); for (final java.util.Map.Entry e : map.entrySet()) { final String key = e.getKey(); @@ -139,11 +140,15 @@ public class TcpIngestor implements Ingestor, AutoCloseable, DisposableBean { // ignore: we only support key/value tags break; default: - tags = tags.copyAddIfNotNull(key, String.valueOf(value)); + if (value instanceof String) { + tags.add(key, (String) value); + } else { + tags.add(key, String.valueOf(value)); + } break; } } - return tags; + return tags.build(); } private OffsetDateTime getDate(final Map map) { diff --git a/performanceDb/src/main/java/org/lucares/performance/db/PerformanceDb.java b/performanceDb/src/main/java/org/lucares/performance/db/PerformanceDb.java index fa53c11..064f100 100644 --- a/performanceDb/src/main/java/org/lucares/performance/db/PerformanceDb.java +++ b/performanceDb/src/main/java/org/lucares/performance/db/PerformanceDb.java @@ -63,7 +63,7 @@ public class PerformanceDb implements AutoCloseable { public void put(final BlockingIterator entries) throws WriteException { - final Duration timeBetweenSyncs = Duration.ofSeconds(10); + final Duration timeBetweenSyncs = Duration.ofSeconds(1); long count = 0; long insertionsSinceLastSync = 0;