Repository: kudu Updated Branches: refs/heads/master 2bff29b59 -> bf6c2c07b
jsonwriter: small optimization for PB fields This avoids extra non-inlinable calls into the protobuf library: - extra calls to GetReflection() - calls to FieldSize() while looping through repeated fields This has a small benefit on all PBs and a big benefit on those PBs with a lot of repeated fields. The latter should be a nice boost particularly when serializing metrics. Before: TestJsonWriter.BenchmarkAllTypes Throughput: 157.609MB/sec TestJsonWriter.BenchmarkNestedMessage Throughput: 124.048MB/sec TestJsonWriter.BenchmarkRepeatedInt64 Throughput: 113.77MB/sec After: TestJsonWriter.BenchmarkAllTypes Throughput: 163.327MB/sec (+3.6%) TestJsonWriter.BenchmarkNestedMessage Throughput: 127.633MB/sec (+2.9%) TestJsonWriter.BenchmarkRepeatedInt64 Throughput: 158.229MB/sec (+39.1%) Change-Id: Iceee0096a3af9300fca3dd18508a12e140e99b31 Reviewed-on: http://gerrit.cloudera.org:8080/9184 Reviewed-by: Sailesh Mukil <sail...@cloudera.com> Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon <t...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f27f5ada Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f27f5ada Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f27f5ada Branch: refs/heads/master Commit: f27f5adaedabf5aac53583a2cd425c6a38484b78 Parents: 2bff29b Author: Todd Lipcon <t...@apache.org> Authored: Thu Feb 1 14:57:36 2018 -0800 Committer: Todd Lipcon <t...@apache.org> Committed: Sat Feb 3 01:53:02 2018 +0000 ---------------------------------------------------------------------- src/kudu/util/jsonwriter-test.cc | 27 ++++++++++++++++++++++++--- src/kudu/util/jsonwriter.cc | 15 ++++++++------- src/kudu/util/jsonwriter.h | 5 ++++- 3 files changed, 36 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/f27f5ada/src/kudu/util/jsonwriter-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/jsonwriter-test.cc b/src/kudu/util/jsonwriter-test.cc index 608d9c0..6f9b10d 100644 --- a/src/kudu/util/jsonwriter-test.cc +++ b/src/kudu/util/jsonwriter-test.cc @@ -29,12 +29,17 @@ #include "kudu/util/stopwatch.h" #include "kudu/util/test_util.h" +namespace google { namespace protobuf { class Message; } } + +using google::protobuf::Message; using jsonwriter_test::TestAllTypes; namespace kudu { class TestJsonWriter : public KuduTest { protected: + void DoBenchmark(const Message& pb); + TestAllTypes MakeAllTypesPB() { TestAllTypes pb; pb.set_optional_int32(1); @@ -170,9 +175,7 @@ TEST_F(TestJsonWriter, TestPBNestedMessage) { JsonWriter::ToJson(pb, JsonWriter::COMPACT)); } -TEST_F(TestJsonWriter, Benchmark) { - TestAllTypes pb = MakeAllTypesPB(); - +void TestJsonWriter::DoBenchmark(const Message& pb) { int64_t total_len = 0; Stopwatch sw; sw.start(); @@ -191,5 +194,23 @@ TEST_F(TestJsonWriter, Benchmark) { LOG(INFO) << "Throughput: " << mbps << "MB/sec"; } +TEST_F(TestJsonWriter, BenchmarkAllTypes) { + DoBenchmark(MakeAllTypesPB()); +} + +TEST_F(TestJsonWriter, BenchmarkNestedMessage) { + TestAllTypes pb; + pb.add_repeated_nested_message()->set_int_field(12345); + pb.mutable_optional_nested_message()->set_int_field(54321); + DoBenchmark(pb); +} + +TEST_F(TestJsonWriter, BenchmarkRepeatedInt64) { + TestAllTypes pb; + for (int i = 0; i < 10000; i++) { + pb.add_repeated_int64(i); + } + DoBenchmark(pb); +} } // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/f27f5ada/src/kudu/util/jsonwriter.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/jsonwriter.cc b/src/kudu/util/jsonwriter.cc index f194d5b..3a5580c 100644 --- a/src/kudu/util/jsonwriter.cc +++ b/src/kudu/util/jsonwriter.cc @@ -188,19 +188,20 @@ void JsonWriter::Protobuf(const Message& pb) { String(field->name()); if (field->is_repeated()) { StartArray(); - for (int i = 0; i < reflection->FieldSize(pb, field); i++) { - ProtobufRepeatedField(pb, field, i); + int size = reflection->FieldSize(pb, field); + for (int i = 0; i < size; i++) { + ProtobufRepeatedField(pb, reflection, field, i); } EndArray(); } else { - ProtobufField(pb, field); + ProtobufField(pb, reflection, field); } } EndObject(); } -void JsonWriter::ProtobufField(const Message& pb, const FieldDescriptor* field) { - const Reflection* reflection = pb.GetReflection(); +void JsonWriter::ProtobufField(const Message& pb, const Reflection* reflection, + const FieldDescriptor* field) { switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_INT32: Int(reflection->GetInt32(pb, field)); @@ -238,8 +239,8 @@ void JsonWriter::ProtobufField(const Message& pb, const FieldDescriptor* field) } } -void JsonWriter::ProtobufRepeatedField(const Message& pb, const FieldDescriptor* field, int index) { - const Reflection* reflection = pb.GetReflection(); +void JsonWriter::ProtobufRepeatedField(const Message& pb, const Reflection* reflection, + const FieldDescriptor* field, int index) { switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_INT32: Int(reflection->GetRepeatedInt32(pb, field, index)); http://git-wip-us.apache.org/repos/asf/kudu/blob/f27f5ada/src/kudu/util/jsonwriter.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/jsonwriter.h b/src/kudu/util/jsonwriter.h index 860bfaa..24b4575 100644 --- a/src/kudu/util/jsonwriter.h +++ b/src/kudu/util/jsonwriter.h @@ -27,8 +27,9 @@ namespace google { namespace protobuf { -class Message; class FieldDescriptor; +class Message; +class Reflection; } // namespace protobuf } // namespace google @@ -85,8 +86,10 @@ class JsonWriter { private: void ProtobufField(const google::protobuf::Message& pb, + const google::protobuf::Reflection* reflection, const google::protobuf::FieldDescriptor* field); void ProtobufRepeatedField(const google::protobuf::Message& pb, + const google::protobuf::Reflection* reflection, const google::protobuf::FieldDescriptor* field, int index);
