diff options
22 files changed, 553 insertions, 67 deletions
diff --git a/Makefile.am b/Makefile.am index a7a1f413..3e988816 100644 --- a/Makefile.am +++ b/Makefile.am @@ -9,7 +9,7 @@ AUTOMAKE_OPTIONS = foreign SUBDIRS = . src # Always include gmock in distributions. -DIST_SUBDIRS = $(subdirs) src conformance +DIST_SUBDIRS = $(subdirs) src conformance benchmarks # Build gmock before we build protobuf tests. We don't add gmock to SUBDIRS # because then "make check" would also build and run all of gmock's own tests, @@ -36,6 +36,10 @@ clean-local: echo "Making clean in conformance"; \ cd conformance && $(MAKE) $(AM_MAKEFLAGS) clean; \ fi; \ + if test -e benchmarks/Makefile; then \ + echo "Making clean in benchmarks"; \ + cd benchmarks && $(MAKE) $(AM_MAKEFLAGS) clean; \ + fi; \ if test -e objectivec/DevTools; then \ echo "Cleaning any ObjC pyc files"; \ rm -f objectivec/DevTools/*.pyc; \ diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am new file mode 100644 index 00000000..1e162eb1 --- /dev/null +++ b/benchmarks/Makefile.am @@ -0,0 +1,66 @@ + +benchmarks_protoc_inputs = \ + benchmarks.proto \ + benchmark_messages_proto3.proto + +benchmarks_protoc_inputs_proto2 = \ + benchmark_messages_proto2.proto + +benchmarks_protoc_outputs = \ + benchmarks.pb.cc \ + benchmarks.pb.h \ + benchmark_messages_proto3.pb.cc \ + benchmark_messages_proto3.pb.h + +benchmarks_protoc_outputs_proto2 = \ + benchmark_messages_proto2.pb.cc \ + benchmark_messages_proto2.pb.h + +bin_PROGRAMS = generate-datasets + +generate_datasets_LDADD = $(top_srcdir)/src/libprotobuf.la +generate_datasets_SOURCES = generate_datasets.cc +generate_datasets_CPPFLAGS = -I$(top_srcdir)/src -I$(srcdir) +nodist_generate_datasets_SOURCES = \ + $(benchmarks_protoc_outputs) \ + $(benchmarks_protoc_outputs_proto2) + +# Explicit deps because BUILT_SOURCES are only done before a "make all/check" +# so a direct "make test_cpp" could fail if parallel enough. +# See: https://www.gnu.org/software/automake/manual/html_node/Built-Sources-Example.html#Recording-Dependencies-manually +generate_datasets-generate_datasets.$(OBJEXT): benchmarks.pb.h + +$(benchmarks_protoc_outputs): protoc_middleman +$(benchmarks_protoc_outputs_proto2): protoc_middleman2 + +CLEANFILES = \ + $(benchmarks_protoc_outputs) \ + $(benchmarks_protoc_outputs_proto2) \ + protoc_middleman \ + protoc_middleman2 \ + dataset.* + +if USE_EXTERNAL_PROTOC + +protoc_middleman: $(benchmarks_protoc_inputs) + $(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. $(benchmarks_protoc_inputs) + touch protoc_middleman + +protoc_middleman2: $(benchmarks_protoc_inputs_proto2) + $(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. $(benchmarks_protoc_inputs_proto2) + touch protoc_middleman2 + +else + +# We have to cd to $(srcdir) before executing protoc because $(protoc_inputs) is +# relative to srcdir, which may not be the same as the current directory when +# building out-of-tree. +protoc_middleman: $(top_srcdir)/src/protoc$(EXEEXT) $(benchmarks_protoc_inputs) $(well_known_type_protoc_inputs) + oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd $(benchmarks_protoc_inputs) ) + touch protoc_middleman + +protoc_middleman2: $(top_srcdir)/src/protoc$(EXEEXT) $(benchmarks_protoc_inputs_proto2) $(well_known_type_protoc_inputs) + oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd $(benchmarks_protoc_inputs_proto2) ) + touch protoc_middleman + +endif diff --git a/benchmarks/README.md b/benchmarks/README.md new file mode 100644 index 00000000..c9027805 --- /dev/null +++ b/benchmarks/README.md @@ -0,0 +1,28 @@ + +# Protocol Buffers Benchmarks + +This directory contains benchmarking schemas and data sets that you +can use to test a variety of performance scenarios against your +protobuf language runtime. + +The schema for the datasets is described in `benchmarks.proto`. + +Generate the data sets like so: + +``` +$ make +$ ./generate-datasets +Wrote dataset: dataset.google_message1_proto3.pb +Wrote dataset: dataset.google_message1_proto2.pb +Wrote dataset: dataset.google_message2.pb +$ +``` + +Each data set will be written to its own file. Benchmarks will +likely want to run several benchmarks against each data set (parse, +serialize, possibly JSON, possibly using different APIs, etc). + +We would like to add more data sets. In general we will favor data sets +that make the overall suite diverse without being too large or having +too many similar tests. Ideally everyone can run through the entire +suite without the test run getting too long. diff --git a/benchmarks/google_speed.proto b/benchmarks/benchmark_messages_proto2.proto index 16f6d678..01f67a1a 100644 --- a/benchmarks/google_speed.proto +++ b/benchmarks/benchmark_messages_proto2.proto @@ -1,11 +1,14 @@ +// Benchmark messages for proto2. + syntax = "proto2"; -package benchmarks; +package benchmarks.proto2; +option java_package = "com.google.protobuf.benchmarks"; -option java_outer_classname = "GoogleSpeed"; +// This is the default, but we specify it here explicitly. option optimize_for = SPEED; -message SpeedMessage1 { +message GoogleMessage1 { required string field1 = 1; optional string field9 = 9; optional string field18 = 18; @@ -40,7 +43,7 @@ message SpeedMessage1 { optional int32 field23 = 23 [default=0]; optional bool field24 = 24 [default=false]; optional int32 field25 = 25 [default=0]; - optional SpeedMessage1SubMessage field15 = 15; + optional GoogleMessage1SubMessage field15 = 15; optional bool field78 = 78; optional int32 field67 = 67 [default=0]; optional int32 field68 = 68; @@ -49,7 +52,7 @@ message SpeedMessage1 { optional int32 field131 = 131 [default=0]; } -message SpeedMessage1SubMessage { +message GoogleMessage1SubMessage { optional int32 field1 = 1 [default=0]; optional int32 field2 = 2 [default=0]; optional int32 field3 = 3 [default=0]; @@ -72,7 +75,7 @@ message SpeedMessage1SubMessage { optional uint64 field300 = 300; } -message SpeedMessage2 { +message GoogleMessage2 { optional string field1 = 1; optional int64 field3 = 3; optional int64 field4 = 4; @@ -112,7 +115,7 @@ message SpeedMessage2 { repeated int32 field73 = 73; optional int32 field20 = 20 [default=0]; optional string field24 = 24; - optional SpeedMessage2GroupedMessage field31 = 31; + optional GoogleMessage2GroupedMessage field31 = 31; } repeated string field128 = 128; optional int64 field131 = 131; @@ -123,7 +126,7 @@ message SpeedMessage2 { optional bool field206 = 206 [default=false]; } -message SpeedMessage2GroupedMessage { +message GoogleMessage2GroupedMessage { optional float field1 = 1; optional float field2 = 2; optional float field3 = 3 [default=0.0]; diff --git a/benchmarks/benchmark_messages_proto3.proto b/benchmarks/benchmark_messages_proto3.proto new file mode 100644 index 00000000..32f58698 --- /dev/null +++ b/benchmarks/benchmark_messages_proto3.proto @@ -0,0 +1,76 @@ +// Benchmark messages for proto3. + +syntax = "proto3"; + +package benchmarks.proto3; +option java_package = "com.google.protobuf.benchmarks"; + +// This is the default, but we specify it here explicitly. +option optimize_for = SPEED; + +message GoogleMessage1 { + string field1 = 1; + string field9 = 9; + string field18 = 18; + bool field80 = 80; + bool field81 = 81; + int32 field2 = 2; + int32 field3 = 3; + int32 field280 = 280; + int32 field6 = 6; + int64 field22 = 22; + string field4 = 4; + repeated fixed64 field5 = 5; + bool field59 = 59; + string field7 = 7; + int32 field16 = 16; + int32 field130 = 130; + bool field12 = 12; + bool field17 = 17; + bool field13 = 13; + bool field14 = 14; + int32 field104 = 104; + int32 field100 = 100; + int32 field101 = 101; + string field102 = 102; + string field103 = 103; + int32 field29 = 29; + bool field30 = 30; + int32 field60 = 60; + int32 field271 = 271; + int32 field272 = 272; + int32 field150 = 150; + int32 field23 = 23; + bool field24 = 24; + int32 field25 = 25; + GoogleMessage1SubMessage field15 = 15; + bool field78 = 78; + int32 field67 = 67; + int32 field68 = 68; + int32 field128 = 128; + string field129 = 129; + int32 field131 = 131; +} + +message GoogleMessage1SubMessage { + int32 field1 = 1; + int32 field2 = 2; + int32 field3 = 3; + string field15 = 15; + bool field12 = 12; + int64 field13 = 13; + int64 field14 = 14; + int32 field16 = 16; + int32 field19 = 19; + bool field20 = 20; + bool field28 = 28; + fixed64 field21 = 21; + int32 field22 = 22; + bool field23 = 23; + bool field206 = 206; + fixed32 field203 = 203; + int32 field204 = 204; + string field205 = 205; + uint64 field207 = 207; + uint64 field300 = 300; +} diff --git a/benchmarks/benchmarks.proto b/benchmarks/benchmarks.proto new file mode 100644 index 00000000..51c0b548 --- /dev/null +++ b/benchmarks/benchmarks.proto @@ -0,0 +1,63 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto3"; +package benchmarks; +option java_package = "com.google.protobuf.benchmarks"; + +message BenchmarkDataset { + // Name of the benchmark dataset. This should be unique across all datasets. + // Should only contain word characters: [a-zA-Z0-9_] + string name = 1; + + // Fully-qualified name of the protobuf message for this dataset. + // It will be one of the messages defined benchmark_messages_proto2.proto + // or benchmark_messages_proto3.proto. + // + // Implementations that do not support reflection can implement this with + // an explicit "if/else" chain that lists every known message defined + // in those files. + string message_name = 2; + + // The payload(s) for this dataset. They should be parsed or serialized + // in sequence, in a loop, ie. + // + // while (!benchmarkDone) { // Benchmark runner decides when to exit. + // for (i = 0; i < benchmark.payload.length; i++) { + // parse(benchmark.payload[i]) + // } + // } + // + // This is intended to let datasets include a variety of data to provide + // potentially more realistic results than just parsing the same message + // over and over. A single message parsed repeatedly could yield unusually + // good branch prediction performance. + repeated bytes payload = 3; +} diff --git a/benchmarks/generate_datasets.cc b/benchmarks/generate_datasets.cc new file mode 100644 index 00000000..61e7adf1 --- /dev/null +++ b/benchmarks/generate_datasets.cc @@ -0,0 +1,117 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include <fstream> +#include <iostream> +#include "benchmarks.pb.h" + +using benchmarks::BenchmarkDataset; +using google::protobuf::Descriptor; +using google::protobuf::DescriptorPool; +using google::protobuf::Message; +using google::protobuf::MessageFactory; + +std::set<std::string> names; + +const char *file_prefix = "dataset."; +const char *file_suffix = ".pb"; + +void WriteFileWithPayloads(const std::string& name, + const std::string& message_name, + const std::vector<std::string>& payload) { + if (!names.insert(name).second) { + std::cerr << "Duplicate test name: " << name << "\n"; + abort(); + } + + // First verify that this message name exists in our set of benchmark messages + // and that these payloads are valid for the given message. + const Descriptor* d = + DescriptorPool::generated_pool()->FindMessageTypeByName(message_name); + + if (!d) { + std::cerr << "For dataset " << name << ", no such message: " + << message_name << "\n"; + abort(); + } + + Message* m = MessageFactory::generated_factory()->GetPrototype(d)->New(); + + for (size_t i = 0; i < payload.size(); i++) { + if (!m->ParseFromString(payload[i])) { + std::cerr << "For dataset " << name << ", payload[" << i << "] fails " + << "to parse\n"; + abort(); + } + } + + BenchmarkDataset dataset; + dataset.set_name(name); + dataset.set_message_name(message_name); + for (size_t i = 0; i < payload.size(); i++) { + dataset.add_payload()->assign(payload[i]); + } + + std::ofstream writer; + std::string fname = file_prefix + name + file_suffix; + writer.open(fname.c_str()); + dataset.SerializeToOstream(&writer); + writer.close(); + + std::cerr << "Wrote dataset: " << fname << "\n"; +} + +void WriteFile(const std::string& name, const std::string& message_name, + const std::string& payload) { + std::vector<std::string> payloads; + payloads.push_back(payload); + WriteFileWithPayloads(name, message_name, payloads); +} + +std::string ReadFile(const std::string& name) { + std::ifstream file(name.c_str()); + GOOGLE_CHECK(file.is_open()) << "Couldn't find file '" << name << + "', please make sure you are running " + "this command from the benchmarks/ " + "directory.\n"; + return std::string((std::istreambuf_iterator<char>(file)), + std::istreambuf_iterator<char>()); +} + +int main() { + WriteFile("google_message1_proto3", "benchmarks.proto3.GoogleMessage1", + ReadFile("google_message1.dat")); + WriteFile("google_message1_proto2", "benchmarks.proto2.GoogleMessage1", + ReadFile("google_message1.dat")); + + // Not in proto3 because it has a group, which is not supported. + WriteFile("google_message2", "benchmarks.proto2.GoogleMessage2", + ReadFile("google_message2.dat")); +} diff --git a/configure.ac b/configure.ac index 33a6c64d..d56a7047 100644 --- a/configure.ac +++ b/configure.ac @@ -180,5 +180,5 @@ export CFLAGS export CXXFLAGS AC_CONFIG_SUBDIRS([gmock]) -AC_CONFIG_FILES([Makefile src/Makefile conformance/Makefile protobuf.pc protobuf-lite.pc]) +AC_CONFIG_FILES([Makefile src/Makefile benchmarks/Makefile conformance/Makefile protobuf.pc protobuf-lite.pc]) AC_OUTPUT diff --git a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs index 0b6cd7e6..827a7595 100644 --- a/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonFormatterTest.cs @@ -481,6 +481,15 @@ namespace Google.Protobuf } [Test] + public void AnyMessageType_CustomPrefix() + { + var formatter = new JsonFormatter(new JsonFormatter.Settings(false, TypeRegistry.FromMessages(TestAllTypes.Descriptor))); + var message = new TestAllTypes { SingleInt32 = 10 }; + var any = Any.Pack(message, "foo.bar/baz"); + AssertJson("{ '@type': 'foo.bar/baz/protobuf_unittest.TestAllTypes', 'singleInt32': 10 }", formatter.Format(any)); + } + + [Test] public void AnyNested() { var registry = TypeRegistry.FromMessages(TestWellKnownTypes.Descriptor, TestAllTypes.Descriptor); diff --git a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs index 684f52b4..c3ad851b 100644 --- a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs @@ -811,6 +811,17 @@ namespace Google.Protobuf } [Test] + public void Any_CustomPrefix() + { + var registry = TypeRegistry.FromMessages(TestAllTypes.Descriptor); + var message = new TestAllTypes { SingleInt32 = 10 }; + var original = Any.Pack(message, "custom.prefix/middle-part"); + var parser = new JsonParser(new JsonParser.Settings(10, registry)); + string json = "{ \"@type\": \"custom.prefix/middle-part/protobuf_unittest.TestAllTypes\", \"singleInt32\": 10 }"; + Assert.AreEqual(original, parser.Parse<Any>(json)); + } + + [Test] public void Any_UnknownType() { string json = "{ \"@type\": \"type.googleapis.com/bogus\" }"; diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/AnyTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/AnyTest.cs index f3593e5f..f21be7d9 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/AnyTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/AnyTest.cs @@ -47,6 +47,24 @@ namespace Google.Protobuf.WellKnownTypes } [Test] + public void Pack_WithCustomPrefix() + { + var message = SampleMessages.CreateFullTestAllTypes(); + var any = Any.Pack(message, "foo.bar/baz"); + Assert.AreEqual("foo.bar/baz/protobuf_unittest.TestAllTypes", any.TypeUrl); + Assert.AreEqual(message.CalculateSize(), any.Value.Length); + } + + [Test] + public void Pack_WithCustomPrefixTrailingSlash() + { + var message = SampleMessages.CreateFullTestAllTypes(); + var any = Any.Pack(message, "foo.bar/baz/"); + Assert.AreEqual("foo.bar/baz/protobuf_unittest.TestAllTypes", any.TypeUrl); + Assert.AreEqual(message.CalculateSize(), any.Value.Length); + } + + [Test] public void Unpack_WrongType() { var message = SampleMessages.CreateFullTestAllTypes(); @@ -64,6 +82,15 @@ namespace Google.Protobuf.WellKnownTypes } [Test] + public void Unpack_CustomPrefix_Success() + { + var message = SampleMessages.CreateFullTestAllTypes(); + var any = Any.Pack(message, "foo.bar/baz"); + var unpacked = any.Unpack<TestAllTypes>(); + Assert.AreEqual(message, unpacked); + } + + [Test] public void ToString_WithValues() { var message = SampleMessages.CreateFullTestAllTypes(); diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index 73a4f64b..83772473 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -567,7 +567,7 @@ namespace Google.Protobuf string typeUrl = (string) value.Descriptor.Fields[Any.TypeUrlFieldNumber].Accessor.GetValue(value); ByteString data = (ByteString) value.Descriptor.Fields[Any.ValueFieldNumber].Accessor.GetValue(value); - string typeName = GetTypeName(typeUrl); + string typeName = Any.GetTypeName(typeUrl); MessageDescriptor descriptor = settings.TypeRegistry.Find(typeName); if (descriptor == null) { @@ -608,17 +608,7 @@ namespace Google.Protobuf writer.Write(data.ToBase64()); writer.Write('"'); writer.Write(" }"); - } - - internal static string GetTypeName(String typeUrl) - { - string[] parts = typeUrl.Split('/'); - if (parts.Length != 2 || parts[0] != TypeUrlPrefix) - { - throw new InvalidProtocolBufferException($"Invalid type url: {typeUrl}"); - } - return parts[1]; - } + } private void WriteStruct(TextWriter writer, IMessage message) { diff --git a/csharp/src/Google.Protobuf/JsonParser.cs b/csharp/src/Google.Protobuf/JsonParser.cs index 80d3013d..d738ebb0 100644 --- a/csharp/src/Google.Protobuf/JsonParser.cs +++ b/csharp/src/Google.Protobuf/JsonParser.cs @@ -513,7 +513,7 @@ namespace Google.Protobuf throw new InvalidProtocolBufferException("Expected string value for Any.@type"); } string typeUrl = token.StringValue; - string typeName = JsonFormatter.GetTypeName(typeUrl); + string typeName = Any.GetTypeName(typeUrl); MessageDescriptor descriptor = settings.TypeRegistry.Find(typeName); if (descriptor == null) diff --git a/csharp/src/Google.Protobuf/WellKnownTypes/AnyPartial.cs b/csharp/src/Google.Protobuf/WellKnownTypes/AnyPartial.cs index 9d43856e..f4fac738 100644 --- a/csharp/src/Google.Protobuf/WellKnownTypes/AnyPartial.cs +++ b/csharp/src/Google.Protobuf/WellKnownTypes/AnyPartial.cs @@ -36,11 +36,27 @@ namespace Google.Protobuf.WellKnownTypes { public partial class Any { + private const string DefaultPrefix = "type.googleapis.com"; + // This could be moved to MessageDescriptor if we wanted to, but keeping it here means // all the Any-specific code is in the same place. - private static string GetTypeUrl(MessageDescriptor descriptor) + private static string GetTypeUrl(MessageDescriptor descriptor, string prefix) => + prefix.EndsWith("/") ? prefix + descriptor.FullName : prefix + "/" + descriptor.FullName; + + /// <summary> + /// Retrieves the type name for a type URL. This is always just the last part of the URL, + /// after the trailing slash. No validation of anything before the trailing slash is performed. + /// If the type URL does not include a slash, an empty string is returned rather than an exception + /// being thrown; this won't match any types, and the calling code is probably in a better position + /// to give a meaningful error. + /// There is no handling of fragments or queries at the moment. + /// </summary> + /// <param name="typeUrl">The URL to extract the type name from</param> + /// <returns>The type name</returns> + internal static string GetTypeName(string typeUrl) { - return "type.googleapis.com/" + descriptor.FullName; + int lastSlash = typeUrl.LastIndexOf('/'); + return lastSlash == -1 ? "" : typeUrl.Substring(lastSlash + 1); } /// <summary> @@ -55,25 +71,37 @@ namespace Google.Protobuf.WellKnownTypes // Note: this doesn't perform as well is it might. We could take a MessageParser<T> in an alternative overload, // which would be expected to perform slightly better... although the difference is likely to be negligible. T target = new T(); - string targetTypeUrl = GetTypeUrl(target.Descriptor); - if (TypeUrl != targetTypeUrl) + if (GetTypeName(TypeUrl) != target.Descriptor.FullName) { - throw new InvalidProtocolBufferException(string.Format("Type url for {0} is {1}; Any message's type url is {2}", - target.Descriptor.Name, targetTypeUrl, TypeUrl)); + throw new InvalidProtocolBufferException( + $"Full type name for {target.Descriptor.Name} is {target.Descriptor.FullName}; Any message's type url is {TypeUrl}"); } target.MergeFrom(Value); return target; } /// <summary> - /// Packs the specified message into an Any message. + /// Packs the specified message into an Any message using a type URL prefix of "type.googleapis.com". + /// </summary> + /// <param name="message">The message to pack.</param> + /// <returns>An Any message with the content and type URL of <paramref name="message"/>.</returns> + public static Any Pack(IMessage message) => Pack(message, DefaultPrefix); + + /// <summary> + /// Packs the specified message into an Any message using the specified type URL prefix. /// </summary> /// <param name="message">The message to pack.</param> + /// <param name="typeUrlPrefix">The prefix for the type URL.</param> /// <returns>An Any message with the content and type URL of <paramref name="message"/>.</returns> - public static Any Pack(IMessage message) + public static Any Pack(IMessage message, string typeUrlPrefix) { - ProtoPreconditions.CheckNotNull(message, "message"); - return new Any { TypeUrl = GetTypeUrl(message.Descriptor), Value = message.ToByteString() }; + ProtoPreconditions.CheckNotNull(message, nameof(message)); + ProtoPreconditions.CheckNotNull(typeUrlPrefix, nameof(typeUrlPrefix)); + return new Any + { + TypeUrl = GetTypeUrl(message.Descriptor, typeUrlPrefix), + Value = message.ToByteString() + }; } } } diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index 27e57506..d0eb9cc4 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - google-protobuf (3.0.0.alpha.5.0) + google-protobuf (3.0.0.alpha.5.0.5) GEM remote: https://rubygems.org/ diff --git a/ruby/Rakefile b/ruby/Rakefile index 8eb7a2df..fa29c315 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -5,6 +5,40 @@ require "rake/testtask" spec = Gem::Specification.load("google-protobuf.gemspec") +well_known_protos = %w[ + google/protobuf/any.proto + google/protobuf/api.proto + google/protobuf/duration.proto + google/protobuf/empty.proto + google/protobuf/field_mask.proto + google/protobuf/source_context.proto + google/protobuf/struct.proto + google/protobuf/timestamp.proto + google/protobuf/type.proto + google/protobuf/wrappers.proto +] + +# These are omitted for now because we don't support proto2. +proto2_protos = %w[ + google/protobuf/descriptor.proto + google/protobuf/compiler/plugin.proto +] + +genproto_output = [] + +# We won't have access to .. from within docker, but the proto files +# will be there, thanks to the :genproto rule dependency for gem:native. +unless ENV['IN_DOCKER'] == 'true' + well_known_protos.each do |proto_file| + input_file = "../src/" + proto_file + output_file = "lib/" + proto_file.sub(/\.proto$/, ".rb") + genproto_output << output_file + file output_file => input_file do |file_task| + sh "../src/protoc -I../src --ruby_out=lib #{input_file}" + end + end +end + if RUBY_PLATFORM == "java" if `which mvn` == '' raise ArgumentError, "maven needs to be installed" @@ -30,37 +64,16 @@ else task 'gem:windows' do require 'rake_compiler_dock' - RakeCompilerDock.sh "bundle && rake cross native gem RUBY_CC_VERSION=2.3.0:2.2.2:2.1.6" + RakeCompilerDock.sh "bundle && IN_DOCKER=true rake cross native gem RUBY_CC_VERSION=2.3.0:2.2.2:2.1.5:2.0.0" end -end - -well_known_protos = %w[ - google/protobuf/any.proto - google/protobuf/api.proto - google/protobuf/duration.proto - google/protobuf/empty.proto - google/protobuf/field_mask.proto - google/protobuf/source_context.proto - google/protobuf/struct.proto - google/protobuf/timestamp.proto - google/protobuf/type.proto - google/protobuf/wrappers.proto -] - -# These are omitted for now because we don't support proto2. -proto2_protos = %w[ - google/protobuf/descriptor.proto - google/protobuf/compiler/plugin.proto -] - -genproto_output = [] -well_known_protos.each do |proto_file| - input_file = "../src/" + proto_file - output_file = "lib/" + proto_file.sub(/\.proto$/, ".rb") - genproto_output << output_file - file output_file => input_file do |file_task| - sh "../src/protoc -I../src --ruby_out=lib #{input_file}" + if RUBY_PLATFORM =~ /darwin/ + task 'gem:native' do + system "rake genproto" + system "rake cross native gem RUBY_CC_VERSION=2.3.0:2.2.2:2.1.5:2.0.0" + end + else + task 'gem:native' => [:genproto, 'gem:windows'] end end diff --git a/ruby/google-protobuf.gemspec b/ruby/google-protobuf.gemspec index 7b64ee77..1fa626fe 100644 --- a/ruby/google-protobuf.gemspec +++ b/ruby/google-protobuf.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |s| s.name = "google-protobuf" - s.version = "3.0.0.alpha.5.0" + s.version = "3.0.0.alpha.5.0.5" s.licenses = ["BSD"] s.summary = "Protocol Buffers" s.description = "Protocol Buffers are Google's data interchange format." diff --git a/src/README.md b/src/README.md index d623a49a..63d6e24c 100644 --- a/src/README.md +++ b/src/README.md @@ -184,7 +184,7 @@ In the downloads section, download the zip file protoc-$VERSION-win32.zip. It contains the protoc binary as well as public proto files of protobuf library. -To build from source using Microsoft Visual C++, see cmake/README.md. +To build from source using Microsoft Visual C++, see [cmake/README.md](../cmake/README.md). To build from source using Cygwin or MinGW, follow the Unix installation instructions, above. diff --git a/src/google/protobuf/compiler/cpp/cpp_enum_field.cc b/src/google/protobuf/compiler/cpp/cpp_enum_field.cc index 78a4c402..10252b39 100644 --- a/src/google/protobuf/compiler/cpp/cpp_enum_field.cc +++ b/src/google/protobuf/compiler/cpp/cpp_enum_field.cc @@ -36,6 +36,7 @@ #include <google/protobuf/compiler/cpp/cpp_helpers.h> #include <google/protobuf/io/printer.h> #include <google/protobuf/stubs/strutil.h> +#include <google/protobuf/wire_format.h> namespace google { namespace protobuf { @@ -141,8 +142,9 @@ GenerateMergeFromCodedStream(io::Printer* printer) const { } else { printer->Print( "} else {\n" - " unknown_fields_stream.WriteVarint32(tag);\n" - " unknown_fields_stream.WriteVarint32(value);\n"); + " unknown_fields_stream.WriteVarint32($tag$);\n" + " unknown_fields_stream.WriteVarint32(value);\n", + "tag", SimpleItoa(internal::WireFormat::MakeTag(descriptor_))); } printer->Print(variables_, "}\n"); diff --git a/src/google/protobuf/lite_unittest.cc b/src/google/protobuf/lite_unittest.cc index d1948ab5..3ca3fbaf 100644 --- a/src/google/protobuf/lite_unittest.cc +++ b/src/google/protobuf/lite_unittest.cc @@ -686,6 +686,33 @@ int main(int argc, char* argv[]) { EXPECT_TRUE(map_message.IsInitialized()); } + { + // Check that adding more values to enum does not corrupt message + // when passed through an old client. + protobuf_unittest::V2MessageLite v2_message; + v2_message.set_int_field(800); + // Set enum field to the value not understood by the old client. + v2_message.set_enum_field(protobuf_unittest::V2_SECOND); + string v2_bytes = v2_message.SerializeAsString(); + + protobuf_unittest::V1MessageLite v1_message; + v1_message.ParseFromString(v2_bytes); + EXPECT_TRUE(v1_message.IsInitialized()); + EXPECT_EQ(v1_message.int_field(), v2_message.int_field()); + // V1 client does not understand V2_SECOND value, so it discards it and + // uses default value instead. + EXPECT_EQ(v1_message.enum_field(), protobuf_unittest::V1_FIRST); + + // However, when re-serialized, it should preserve enum value. + string v1_bytes = v1_message.SerializeAsString(); + + protobuf_unittest::V2MessageLite same_v2_message; + same_v2_message.ParseFromString(v1_bytes); + + EXPECT_EQ(v2_message.int_field(), same_v2_message.int_field()); + EXPECT_EQ(v2_message.enum_field(), same_v2_message.enum_field()); + } + std::cout << "PASS" << std::endl; return 0; } diff --git a/src/google/protobuf/unittest_lite.proto b/src/google/protobuf/unittest_lite.proto index 41ed845f..878ec7c1 100644 --- a/src/google/protobuf/unittest_lite.proto +++ b/src/google/protobuf/unittest_lite.proto @@ -386,3 +386,22 @@ message TestEmptyMessageLite{ message TestEmptyMessageWithExtensionsLite { extensions 1 to max; } + +enum V1EnumLite { + V1_FIRST = 1; +} + +enum V2EnumLite { + V2_FIRST = 1; + V2_SECOND = 2; +} + +message V1MessageLite { + required int32 int_field = 1; + optional V1EnumLite enum_field = 2 [ default = V1_FIRST ]; +} + +message V2MessageLite { + required int32 int_field = 1; + optional V2EnumLite enum_field = 2 [ default = V2_FIRST ]; +} @@ -36,6 +36,9 @@ build_cpp() { internal_build_cpp make check -j2 cd conformance && make test_cpp && cd .. + + # Verify benchmarking code can build successfully. + cd benchmarks && make && ./generate-datasets && cd .. } build_cpp_distcheck() { |