Skip to content

Commit 276b093

Browse files
committed
grpc-native: Optimize String encoding by avoiding converting Kotlin string to std::string
Signed-off-by: Johannes Zottele <[email protected]>
1 parent a57504b commit 276b093

File tree

5 files changed

+39
-8
lines changed

5 files changed

+39
-8
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
headers = protowire.h
22
headerFilter = protowire.h
33

4+
noStringConversion = pw_encoder_write_string
5+
46
staticLibraries = libprotowire_static.a

grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/WireEncoder.native.kt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import kotlin.experimental.ExperimentalNativeApi
1111
import kotlin.native.ref.createCleaner
1212

1313

14+
// TODO: Evaluate if we should implement a ZeroCopyOutputSink (similar to the ZeroCopyInputSource)
15+
// to reduce the number of copies during encoding.
1416
@OptIn(ExperimentalForeignApi::class, ExperimentalNativeApi::class)
1517
internal class WireEncoderNative(private val sink: Sink): WireEncoder {
1618
/**
@@ -94,10 +96,12 @@ internal class WireEncoderNative(private val sink: Sink): WireEncoder {
9496
}
9597

9698
override fun writeString(fieldNr: Int, value: String): Boolean {
97-
val str = pw_string_new(value) ?: error("Failed to create string")
98-
val result = pw_encoder_write_string(raw, fieldNr, str)
99-
pw_string_delete(str)
100-
return result;
99+
if (value.isEmpty()) {
100+
return pw_encoder_write_string(raw, fieldNr, null, 0)
101+
}
102+
return value.usePinned {
103+
pw_encoder_write_string(raw, fieldNr, it.addressOf(0).reinterpret(), value.length)
104+
}
101105
}
102106

103107
override fun flush() {

grpc/grpc-core/src/nativeTest/kotlin/kotlinx/rpc/grpc/internal/WireCodecTest.kt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,4 +553,24 @@ class WireCodecTest {
553553
}
554554
assertTrue(buffer.exhausted())
555555
}
556+
557+
@Test
558+
fun testEmptyString() {
559+
val buffer = Buffer()
560+
561+
val encoder = WireEncoder(buffer)
562+
assertTrue(encoder.writeString(1, ""))
563+
encoder.flush()
564+
565+
val decoder = WireDecoder(buffer)
566+
567+
val tag = decoder.readTag()
568+
assertNotNull(tag)
569+
assertEquals(1, tag.fieldNr)
570+
assertEquals(WireType.LENGTH_DELIMITED, tag.wireType)
571+
572+
val str = decoder.readString()
573+
assertNotNull(str)
574+
assertEquals("", str)
575+
}
556576
}

grpc/grpcpp-c/include/protowire.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ extern "C" {
4848
bool pw_encoder_write_float(pw_encoder_t *self, int field_no, float value);
4949
bool pw_encoder_write_double(pw_encoder_t *self, int field_no, double value);
5050
bool pw_encoder_write_enum(pw_encoder_t *self, int field_no, int value);
51-
bool pw_encoder_write_string(pw_encoder_t *self, int field_no, pw_string_t *value);
51+
bool pw_encoder_write_string(pw_encoder_t *self, int field_no, const char *data, int size);
5252
bool pw_encoder_write_bytes(pw_encoder_t *self, int field_no, pw_string_t *value);
5353

5454

@@ -70,6 +70,7 @@ extern "C" {
7070
int64_t (*byteCount)(void *ctx);
7171
} pw_zero_copy_input_t;
7272

73+
7374
/**
7475
* Create a new pw_decoder_t that wraps a CodedInputStream to decode values from a wire format stream.
7576
*

grpc/grpcpp-c/src/protowire.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ struct pw_encoder {
101101
explicit pw_encoder(protowire::SinkStream sink)
102102
: sinkStream(std::move(sink)),
103103
cosa(&sinkStream),
104-
cos(&cosa) {}
104+
cos(&cosa) {
105+
cos.EnableAliasing(true);
106+
}
105107
};
106108

107109
struct pw_decoder {
@@ -166,8 +168,10 @@ extern "C" {
166168
WRITE_FIELD_FUNC( sfixed64, SFixed64, int64_t)
167169
WRITE_FIELD_FUNC( enum, Enum, int)
168170

169-
bool pw_encoder_write_string(pw_encoder_t *self, int field_no, pw_string_t *value) {
170-
WireFormatLite::WriteString(field_no, value->str, &self->cos);
171+
bool pw_encoder_write_string(pw_encoder_t *self, int field_no, const char *data, int size) {
172+
WireFormatLite::WriteTag(field_no, WireFormatLite::WIRETYPE_LENGTH_DELIMITED, &self->cos);
173+
self->cos.WriteVarint32(size);
174+
self->cos.WriteRawMaybeAliased(data, size);
171175
return check(self);
172176
}
173177
bool pw_encoder_write_bytes(pw_encoder_t *self, int field_no, pw_string_t *value) {

0 commit comments

Comments
 (0)