Skip to content

Improve performance 20%-110% #14

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 13 commits into from
Dec 2, 2019
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
/target
/.idea
/*.iml
*.jfr
30 changes: 28 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
<logback-classic.version>1.1.3</logback-classic.version>
<hamcrest-all.version>1.3</hamcrest-all.version>
<junit.version>4.12</junit.version>
<jmh.version>1.21</jmh.version>
</properties>

<developers>
Expand Down Expand Up @@ -105,8 +106,11 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>3.2</version>
<configuration>
<source>1.6</source>
<target>1.6</target>
<source>1.7</source>
<target>1.7</target>
<compilerArgs>
<arg>-Werror</arg>
</compilerArgs>
<compilerArgument></compilerArgument>
</configuration>
</plugin>
Expand Down Expand Up @@ -214,6 +218,16 @@
<artifactId>hamcrest-all</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-generator-annprocess</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<dependencyManagement>
Expand Down Expand Up @@ -243,6 +257,18 @@
<artifactId>hamcrest-all</artifactId>
<version>${hamcrest-all.version}</version>
</dependency>
<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-core</artifactId>
<version>${jmh.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-generator-annprocess</artifactId>
<version>${jmh.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
</dependencyManagement>

Expand Down
51 changes: 25 additions & 26 deletions src/main/java/com/arangodb/velocypack/VPackBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.UnsupportedEncodingException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.nio.charset.StandardCharsets;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -46,6 +47,7 @@
import com.arangodb.velocypack.internal.DefaultVPackBuilderOptions;
import com.arangodb.velocypack.internal.Value;
import com.arangodb.velocypack.internal.util.NumberUtil;
import com.fasterxml.jackson.core.io.CharTypes;

/**
* @author Mark Vollmary
Expand Down Expand Up @@ -216,7 +218,7 @@ public void append(final VPackBuilder builder, final VPackSlice value) throws VP
private int size;
private final List<Integer> stack; // Start positions of open
// objects/arrays
private final Map<Integer, List<Integer>> index; // Indices for starts
private List<List<Integer>> index; // Indices for starts
// of
// subindex
private boolean keyWritten; // indicates that in the current object the key
Expand All @@ -233,7 +235,7 @@ public VPackBuilder(final BuilderOptions options) {
size = 0;
buffer = new byte[10];
stack = new ArrayList<Integer>();
index = new HashMap<Integer, List<Integer>>();
index = new ArrayList<List<Integer>>(4);
}

public BuilderOptions getOptions() {
Expand Down Expand Up @@ -546,7 +548,7 @@ private void set(final Value item) throws VPackBuilderException {
throw new VPackBuilderUnexpectedValueException(ValueType.UINT, Long.class, Integer.class,
BigInteger.class);
}
if (-1 == vUInt.compareTo(BigInteger.ZERO)) {
if (vUInt.compareTo(BigInteger.ZERO) < 0) {
throw new VPackBuilderUnexpectedValueException(ValueType.UINT, "non-negative", Long.class,
Integer.class, BigInteger.class);
}
Expand Down Expand Up @@ -620,22 +622,18 @@ private void appendSQLTimestamp(final Timestamp value) {
append(value.getTime(), LONG_BYTES);
}

private void appendString(final String value) throws VPackBuilderException {
try {
final byte[] bytes = value.getBytes("UTF-8");
final int length = bytes.length;
if (length <= 126) {
// short string
add((byte) (0x40 + length));
} else {
// long string
add((byte) 0xbf);
appendLength(length);
}
appendString(bytes);
} catch (final UnsupportedEncodingException e) {
throw new VPackBuilderException(e);
private void appendString(final String value) {
final byte[] bytes = value.getBytes(StandardCharsets.UTF_8);
final int length = bytes.length;
if (length <= 126) {
// short string
add((byte) (0x40 + length));
} else {
// long string
add((byte) 0xbf);
appendLength(length);
}
appendString(bytes);
}

private void appendString(final byte[] bytes) {
Expand Down Expand Up @@ -670,7 +668,7 @@ private void addObject(final boolean unindexed) {
private void addCompoundValue(final byte head) {
// an Array or Object is started:
stack.add(size);
index.put(stack.size() - 1, new ArrayList<Integer>());
index.add(stack.size() - 1, new ArrayList<Integer>());
add(head);
// Will be filled later with bytelength and nr subs
size += 8;
Expand Down Expand Up @@ -990,10 +988,10 @@ private VPackBuilder closeArray(final int tos, final List<Integer> in) {
}

private static class SortEntry {
private final VPackSlice slice;
private final VPackStringSlice slice;
private final int offset;

public SortEntry(final VPackSlice slice, final int offset) {
public SortEntry(final VPackStringSlice slice, final int offset) {
super();
this.slice = slice;
this.offset = offset;
Expand All @@ -1002,17 +1000,18 @@ public SortEntry(final VPackSlice slice, final int offset) {

private void sortObjectIndex(final int start, final List<Integer> offsets)
throws VPackKeyTypeException, VPackNeedAttributeTranslatorException {
final List<VPackBuilder.SortEntry> attributes = new ArrayList<VPackBuilder.SortEntry>();
for (final Integer offset : offsets) {
attributes.add(new SortEntry(new VPackSlice(buffer, start + offset).makeKey(), offset));
VPackBuilder.SortEntry[] attributes = new VPackBuilder.SortEntry[offsets.size()];
for (int i = 0; i < offsets.size(); i++) {
Integer offset = offsets.get(i);
attributes[i] = new SortEntry(new VPackSlice(buffer, start + offset).makeKey().getAsStringSlice(), offset);
}
final Comparator<SortEntry> comparator = new Comparator<SortEntry>() {
@Override
public int compare(final SortEntry o1, final SortEntry o2) {
return o1.slice.getAsString().compareTo(o2.slice.getAsString());
return o1.slice.compareTo(o2.slice);
}
};
Collections.sort(attributes, comparator);
Arrays.sort(attributes, comparator);
offsets.clear();
for (final SortEntry sortEntry : attributes) {
offsets.add(sortEntry.offset);
Expand Down
20 changes: 14 additions & 6 deletions src/main/java/com/arangodb/velocypack/VPackSlice.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@
import java.math.BigDecimal;
import java.math.BigInteger;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Date;
import java.util.Iterator;
import java.util.Map.Entry;
import java.lang.Comparable;

import com.arangodb.velocypack.exception.VPackException;
import com.arangodb.velocypack.exception.VPackKeyTypeException;
Expand Down Expand Up @@ -294,6 +296,10 @@ public java.sql.Timestamp getAsSQLTimestamp() {
}

public String getAsString() {
return getAsStringSlice().toString();
}

public VPackStringSlice getAsStringSlice() {
if (!isString()) {
throw new VPackValueTypeException(ValueType.STRING);
}
Expand All @@ -308,12 +314,12 @@ private boolean isLongString() {
return head() == (byte) 0xbf;
}

private String getShortString() {
return new String(vpack, start + 1, length(), Charset.forName("UTF-8"));
private VPackStringSlice getShortString() {
return new VPackStringSlice(vpack, start + 1, length());
}

private String getLongString() {
return new String(vpack, start + 9, getLongStringLength(), Charset.forName("UTF-8"));
private VPackStringSlice getLongString() {
return new VPackStringSlice(vpack, start + 9, getLongStringLength());
}

private int getLongStringLength() {
Expand Down Expand Up @@ -574,6 +580,7 @@ private VPackSlice searchObjectKeyBinary(
long l = 0;
long r = n - 1;

byte[] attributeBytes = attribute.getBytes(StandardCharsets.UTF_8);
for (;;) {
// midpoint
final long index = l + ((r - l) / 2);
Expand All @@ -582,14 +589,14 @@ private VPackSlice searchObjectKeyBinary(
final VPackSlice key = new VPackSlice(vpack, (int) (start + keyIndex));
int res = 0;
if (key.isString()) {
res = key.compareString(attribute);
res = key.getAsStringSlice().compareToBytes(attributeBytes);
} else if (key.isInteger()) {
// translate key
if (!useTranslator) {
// no attribute translator
throw new VPackNeedAttributeTranslatorException();
}
res = key.translateUnchecked().compareString(attribute);
res = key.translateUnchecked().getAsStringSlice().compareToBytes(attributeBytes);
} else {
// invalid key
result = new VPackSlice();
Expand Down Expand Up @@ -816,4 +823,5 @@ public boolean equals(final Object obj) {
return true;
}


}
46 changes: 46 additions & 0 deletions src/main/java/com/arangodb/velocypack/VPackStringSlice.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.arangodb.velocypack;

import java.nio.charset.StandardCharsets;

/**
* Wrapper around a {@link ValueType#STRING} supporting fast bytewise comparison.
*
* @see <a href="https://github.com/arangodb/velocypack/blob/master/VelocyPack.md#objects">VelocyPack Objects</a>
*/
public class VPackStringSlice implements Comparable<VPackStringSlice> {
private byte[] vpack;
/**
* Index of the string bytes within {@link vpack},
* i.e. tag byte and length are somewhere before this index.
*/
private int start;
Copy link
Contributor Author

@htmldoug htmldoug Sep 25, 2019

Choose a reason for hiding this comment

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

I wasn't sure about this decision. Starting at the tag byte would make it more similar to VPackSlice, but it would add a bit more complexity to the compareToBytes operations. Those comparisons are on the hot path for the O(ln n) path queries and for sorting the offset table while constructing the object.

I couldn't think of a good way to make this class private, so it's probably worth giving it some thought before merging so we don't break compatibility later.

@mpv1989 @hkernbach What do you think?

private int length;

public VPackStringSlice(byte[] vpack, int start, int length) {
this.vpack = vpack;
this.start = start;
this.length = length;
}

@Override
public int compareTo(VPackStringSlice o) {
return compareToBytes(o.vpack, o.start, o.length);
}

public int compareToBytes(byte[] other) {
return compareToBytes(other, 0, other.length);
}

public int compareToBytes(byte[] other, int off, int oLen) {
for (int i = 0; i < length && i < oLen; i++) {
int c = (vpack[start + i] & 0xff) - (other[off + i] & 0xff);
if (c != 0) return c;
}
return length - oLen;
}

@Override
public String toString() {
return new String(vpack, start, length, StandardCharsets.UTF_8);
}
}
Loading