Skip to content

Commit

Permalink
Merge pull request #1924 from alecgrieser/continuation-to-byte-string
Browse files Browse the repository at this point in the history 
Resolves #1923: Avoid copying continuation byte arrays by using ByteString internally
  • Loading branch information
@MMcM
MMcM committed Nov 21, 2022
2 parents b0cbab3 + d7d0fe3 commit b7d7f6d
Show file tree
Hide file tree
Showing 18 changed files with 201 additions and 75 deletions.
2 changes: 1 addition & 1 deletion docs/ReleaseNotes.md
View file
Expand Up @@ -23,7 +23,7 @@ The Guava dependency version has been updated to 31.1. Projects may need to chec
* **Performance** Improvement 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Performance** Improvement 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Performance** Improvement 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Performance** Improvement 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Performance** The number of array copies when constructing continuations should be decreased by relying more on `ByteString` internally [(Issue #1923)](https://github.com/FoundationDB/fdb-record-layer/issues/1923)
* **Performance** Improvement 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
Expand Down
3 changes: 2 additions & 1 deletion fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/ByteArrayContinuation.java
View file
Expand Up @@ -23,6 +23,7 @@
import com.apple.foundationdb.annotation.SpotBugsSuppressWarnings;
import com.apple.foundationdb.annotation.API;
import com.google.protobuf.ByteString;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -54,7 +55,7 @@ public byte[] toBytes() {
@Nonnull
public ByteString toByteString() {
if (byteString == null) {
byteString = ByteString.copyFrom(bytes);
byteString = ZeroCopyByteString.wrap(bytes);
}
return byteString;
}
Expand Down
8 changes: 8 additions & 0 deletions ...d-layer-core/src/main/java/com/apple/foundationdb/record/RecordCursorEndContinuation.java
View file
Expand Up @@ -21,7 +21,9 @@
package com.apple.foundationdb.record;

import com.apple.foundationdb.annotation.API;
import com.google.protobuf.ByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
Expand All @@ -35,6 +37,12 @@ public class RecordCursorEndContinuation implements RecordCursorContinuation {
private RecordCursorEndContinuation() {
}

@Nonnull
@Override
public ByteString toByteString() {
return ByteString.EMPTY;
}

@Nullable
@Override
public byte[] toBytes() {
Expand Down
8 changes: 8 additions & 0 deletions ...layer-core/src/main/java/com/apple/foundationdb/record/RecordCursorStartContinuation.java
View file
Expand Up @@ -21,7 +21,9 @@
package com.apple.foundationdb.record;

import com.apple.foundationdb.annotation.API;
import com.google.protobuf.ByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
Expand All @@ -45,6 +47,12 @@ public boolean isEnd() {
return false;
}

@Nonnull
@Override
public ByteString toByteString() {
return ByteString.EMPTY;
}

@Nullable
@Override
public byte[] toBytes() {
Expand Down
9 changes: 9 additions & 0 deletions fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/ChainedCursor.java
View file
Expand Up @@ -28,6 +28,8 @@
import com.apple.foundationdb.record.ScanProperties;
import com.apple.foundationdb.record.logging.LogMessageKeys;
import com.apple.foundationdb.record.provider.foundationdb.FDBRecordContext;
import com.google.protobuf.ByteString;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -260,6 +262,13 @@ public Continuation(@Nonnull Optional<T> lastValue, @Nonnull Function<T, byte[]>
this.continuationEncoder = continuationEncoder;
}

@Nonnull
@Override
public ByteString toByteString() {
byte[] bytes = toBytes();
return bytes == null ? ByteString.EMPTY : ZeroCopyByteString.wrap(bytes);
}

@Nullable
@Override
public byte[] toBytes() {
Expand Down
55 changes: 37 additions & 18 deletions ...ayer-core/src/main/java/com/apple/foundationdb/record/cursors/FlatMapPipelinedCursor.java
View file
Expand Up @@ -30,6 +30,7 @@
import com.apple.foundationdb.record.RecordCursorResult;
import com.apple.foundationdb.record.RecordCursorStartContinuation;
import com.apple.foundationdb.record.RecordCursorVisitor;
import com.google.protobuf.ByteString;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -298,6 +299,10 @@ private static class Continuation<T, V> implements RecordCursorContinuation {
private final byte[] outerCheckValue;
@Nonnull
private final RecordCursorResult<V> innerResult;
@Nullable
private ByteString cachedByteString;
@Nullable
private byte[] cachedBytes;

public Continuation(@Nonnull RecordCursorContinuation priorOuterContinuation,
@Nonnull RecordCursorResult<T> outerResult,
Expand All @@ -314,31 +319,45 @@ public boolean isEnd() {
return outerResult.getContinuation().isEnd() && innerResult.getContinuation().isEnd();
}

@Nonnull
@Override
public ByteString toByteString() {
if (isEnd()) {
return ByteString.EMPTY;
}
if (cachedByteString == null) {
final RecordCursorProto.FlatMapContinuation.Builder builder = RecordCursorProto.FlatMapContinuation.newBuilder();
final RecordCursorContinuation innerContinuation = innerResult.getContinuation();

if (innerContinuation.isEnd()) {
// This was the last of the inner cursor. Take continuation from outer after it.
builder.setOuterContinuation(outerResult.getContinuation().toByteString());
} else {
// This was in the middle of the inner cursor. Take continuation from outer before it and arrange to skip to it.
final ByteString priorOuterContinuationBytes = priorOuterContinuation.toByteString();
if (!priorOuterContinuationBytes.isEmpty()) { // isn't start or end continuation
builder.setOuterContinuation(priorOuterContinuationBytes);
}
if (outerCheckValue != null) {
builder.setCheckValue(ZeroCopyByteString.wrap(outerCheckValue));
}
builder.setInnerContinuation(innerContinuation.toByteString());
}
cachedByteString = builder.build().toByteString();
}
return cachedByteString;
}

@Nullable
@Override
public byte[] toBytes() {
if (isEnd()) {
return null;
}

final RecordCursorProto.FlatMapContinuation.Builder builder = RecordCursorProto.FlatMapContinuation.newBuilder();
final RecordCursorContinuation innerContinuation = innerResult.getContinuation();

if (innerContinuation.isEnd()) {
// This was the last of the inner cursor. Take continuation from outer after it.
builder.setOuterContinuation(ZeroCopyByteString.wrap(outerResult.getContinuation().toBytes()));
} else {
// This was in the middle of the inner cursor. Take continuation from outer before it and arrange to skip to it.
final byte[] priorOuterContinuationBytes = priorOuterContinuation.toBytes();
if (priorOuterContinuationBytes != null) { // isn't start or end continuation
builder.setOuterContinuation(ZeroCopyByteString.wrap(priorOuterContinuation.toBytes()));
}
if (outerCheckValue != null) {
builder.setCheckValue(ZeroCopyByteString.wrap(outerCheckValue));
}
builder.setInnerContinuation(ZeroCopyByteString.wrap(innerContinuation.toBytes()));
if (cachedBytes == null) {
cachedBytes = toByteString().toByteArray();
}
return builder.build().toByteArray();
return cachedBytes;
}
}
}
12 changes: 12 additions & 0 deletions fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/ListCursor.java
View file
Expand Up @@ -25,11 +25,14 @@
import com.apple.foundationdb.record.RecordCursorContinuation;
import com.apple.foundationdb.record.RecordCursorResult;
import com.apple.foundationdb.record.RecordCursorVisitor;
import com.google.protobuf.ByteString;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.nio.ByteBuffer;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.concurrent.ForkJoinPool;
Expand Down Expand Up @@ -109,6 +112,15 @@ public boolean isEnd() {
return nextPosition > listSize;
}

@Nonnull
@Override
public ByteString toByteString() {
if (isEnd()) {
return ByteString.EMPTY;
}
return ZeroCopyByteString.wrap(Objects.requireNonNull(toBytes()));
}

@Nullable
@Override
public byte[] toBytes() {
Expand Down
24 changes: 15 additions & 9 deletions fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/OrElseCursor.java
View file
Expand Up @@ -28,8 +28,8 @@
import com.apple.foundationdb.record.RecordCursorResult;
import com.apple.foundationdb.record.RecordCursorVisitor;
import com.apple.foundationdb.tuple.ByteArrayUtil2;
import com.google.protobuf.ByteString;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -184,19 +184,25 @@ public boolean isEnd() {
return innerOrOtherContinuation.isEnd();
}

@Nullable
@Nonnull
@Override
public byte[] toBytes() {
byte[] bytes = innerOrOtherContinuation.toBytes();
if (isEnd() || bytes == null) {
return null;
public ByteString toByteString() {
ByteString bytes = innerOrOtherContinuation.toByteString();
if (isEnd() || bytes.isEmpty()) {
return ByteString.EMPTY;
}

return RecordCursorProto.OrElseContinuation.newBuilder()
.setState(state)
.setContinuation(ZeroCopyByteString.wrap(bytes))
.setContinuation(bytes)
.build()
.toByteArray();
.toByteString();
}

@Nullable
@Override
public byte[] toBytes() {
ByteString byteString = toByteString();
return byteString.isEmpty() ? null : byteString.toByteArray();
}
}

Expand Down
12 changes: 12 additions & 0 deletions fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/RangeCursor.java
View file
Expand Up @@ -25,10 +25,13 @@
import com.apple.foundationdb.record.RecordCursorContinuation;
import com.apple.foundationdb.record.RecordCursorResult;
import com.apple.foundationdb.record.RecordCursorVisitor;
import com.google.protobuf.ByteString;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.nio.ByteBuffer;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;

Expand Down Expand Up @@ -106,6 +109,15 @@ public boolean isEnd() {
return nextPosition > size;
}

@Nonnull
@Override
public ByteString toByteString() {
if (isEnd()) {
return ByteString.EMPTY;
}
return ZeroCopyByteString.wrap(Objects.requireNonNull(toBytes()));
}

@Nullable
@Override
public byte[] toBytes() {
Expand Down
12 changes: 12 additions & 0 deletions ...ore/src/main/java/com/apple/foundationdb/record/provider/foundationdb/KeyValueCursor.java
View file
Expand Up @@ -41,6 +41,8 @@
import com.apple.foundationdb.record.cursors.CursorLimitManager;
import com.apple.foundationdb.subspace.Subspace;
import com.apple.foundationdb.tuple.Tuple;
import com.google.protobuf.ByteString;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -150,6 +152,16 @@ public boolean isEnd() {
return lastKey == null;
}

@Nonnull
@Override
public ByteString toByteString() {
if (lastKey == null) {
return ByteString.EMPTY;
}
ByteString base = ZeroCopyByteString.wrap(lastKey);
return base.substring(prefixLength, lastKey.length);
}

@Nullable
@Override
public byte[] toBytes() {
Expand Down
7 changes: 3 additions & 4 deletions ...pple/foundationdb/record/provider/foundationdb/cursors/BloomFilterCursorContinuation.java
View file
Expand Up @@ -22,7 +22,6 @@

import com.apple.foundationdb.record.RecordCursorContinuation;
import com.google.protobuf.ByteString;
import com.google.protobuf.ZeroCopyByteString;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -53,9 +52,9 @@ ProbableIntersectionContinuation.CursorState toProto() {
if (childContinuation.isEnd()) {
builder.setExhausted(true);
} else {
byte[] childBytes = childContinuation.toBytes();
if (childBytes != null) {
builder.setContinuation(ZeroCopyByteString.wrap(childBytes));
ByteString childBytes = childContinuation.toByteString();
if (!childBytes.isEmpty()) {
builder.setContinuation(childBytes);
}
}
if (bloomBytes != null) {
Expand Down
6 changes: 3 additions & 3 deletions ...apple/foundationdb/record/provider/foundationdb/cursors/ComparatorCursorContinuation.java
View file
Expand Up @@ -78,13 +78,13 @@ protected void addOtherChild(@Nonnull RecordCursorProto.ComparatorContinuation.B
if (continuation.isEnd()) {
cursorState = EXHAUSTED_PROTO;
} else {
byte[] asBytes = continuation.toBytes();
if (asBytes == null && !continuation.isEnd()) {
ByteString asBytes = continuation.toByteString();
if (asBytes.isEmpty() && !continuation.isEnd()) {
cursorState = START_PROTO;
} else {
cursorState = RecordCursorProto.ComparatorContinuation.CursorState.newBuilder()
.setStarted(true)
.setContinuation(ByteString.copyFrom(asBytes))
.setContinuation(asBytes)
.build();
}
}
Expand Down

0 comments on commit b7d7f6d

Please sign in to comment.