From 611a89c4b63f262f7c81614006b9fbd2d5c6a35f Mon Sep 17 00:00:00 2001 From: Yangqing Jia Date: Wed, 21 Mar 2018 10:29:45 -0700 Subject: [PATCH] Remove more protobuf APIs. (#2348) * Wrap ShutdownProtobufLibrary * Remove text_format.h header and only put the function in proto_utils.h * ParseFromString returns bool --- binaries/predictor_verifier.cc | 2 +- binaries/run_plan.cc | 2 +- binaries/run_plan_mpi.cc | 2 +- caffe2/contrib/aten/aten_op_template.h | 1 - caffe2/core/graph_test.cc | 1 - caffe2/core/memonger.cc | 3 +-- caffe2/core/net_gpu_test.cc | 3 +-- caffe2/core/net_test.cc | 7 +++---- caffe2/core/observer_test.cc | 1 - caffe2/core/parallel_net_test.cc | 3 +-- caffe2/core/predictor_test.cc | 5 ++--- caffe2/core/transform_test.cc | 1 - caffe2/mpi/mpi_gpu_test.cc | 11 +++++------ caffe2/mpi/mpi_test.cc | 11 +++++------ caffe2/observers/time_observer_test.cc | 1 - caffe2/onnx/backend.h | 1 - .../recurrent_network_blob_fetcher_op.h | 1 - caffe2/operators/recurrent_network_op.cc | 3 +-- .../common_subexpression_elimination_test.cc | 1 - .../transforms/conv_to_nnpack_transform_test.cc | 1 - caffe2/transforms/pattern_net_transform_test.cc | 1 - caffe2/utils/proto_utils.cc | 12 +++++++++++- caffe2/utils/proto_utils.h | 16 ++++++++++++++++ cmake/public/protobuf.cmake | 13 +++++++++++-- 24 files changed, 60 insertions(+), 43 deletions(-) diff --git a/binaries/predictor_verifier.cc b/binaries/predictor_verifier.cc index 8273400b889dd..e82a8e9d2cec8 100644 --- a/binaries/predictor_verifier.cc +++ b/binaries/predictor_verifier.cc @@ -52,6 +52,6 @@ int main(int argc, char** argv) { caffe2::GlobalInit(&argc, &argv); caffe2::run(); // This is to allow us to use memory leak checks. - google::protobuf::ShutdownProtobufLibrary(); + caffe2::ShutdownProtobufLibrary(); return 0; } diff --git a/binaries/run_plan.cc b/binaries/run_plan.cc index 65f4325fcb62d..5ad2c3a9b37f2 100644 --- a/binaries/run_plan.cc +++ b/binaries/run_plan.cc @@ -35,6 +35,6 @@ int main(int argc, char** argv) { workspace->RunPlan(plan_def); // This is to allow us to use memory leak checks. - google::protobuf::ShutdownProtobufLibrary(); + caffe2::ShutdownProtobufLibrary(); return 0; } diff --git a/binaries/run_plan_mpi.cc b/binaries/run_plan_mpi.cc index 0a04cdd574b12..ee720fac2965c 100644 --- a/binaries/run_plan_mpi.cc +++ b/binaries/run_plan_mpi.cc @@ -42,7 +42,7 @@ int main(int argc, char** argv) { workspace->RunPlan(plan_def); // This is to allow us to use memory leak checks. - google::protobuf::ShutdownProtobufLibrary(); + caffe2::ShutdownProtobufLibrary(); MPI_Finalize(); return 0; } diff --git a/caffe2/contrib/aten/aten_op_template.h b/caffe2/contrib/aten/aten_op_template.h index 359f4fb6e496e..e85965997e2b5 100644 --- a/caffe2/contrib/aten/aten_op_template.h +++ b/caffe2/contrib/aten/aten_op_template.h @@ -21,7 +21,6 @@ #include #include #include -#include #include // a map from descriptor strings (see [DESCRIPTORS]) diff --git a/caffe2/core/graph_test.cc b/caffe2/core/graph_test.cc index 7ec81bc80d97d..e2b911716913b 100644 --- a/caffe2/core/graph_test.cc +++ b/caffe2/core/graph_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/graph.h" #include "caffe2/core/net.h" diff --git a/caffe2/core/memonger.cc b/caffe2/core/memonger.cc index 3edc237784ca0..24d72f4e817d1 100644 --- a/caffe2/core/memonger.cc +++ b/caffe2/core/memonger.cc @@ -20,7 +20,6 @@ #include #include "caffe2/utils/proto_utils.h" -#include "google/protobuf/text_format.h" namespace caffe2 { namespace memonger { @@ -305,7 +304,7 @@ class ComputeBlobRecyclingForDag { } else { NetDef step_net; CAFFE_ENFORCE( - google::protobuf::TextFormat::ParseFromString( + TextFormat::ParseFromString( arg->s(), &step_net), "Could not parse step net:", name); diff --git a/caffe2/core/net_gpu_test.cc b/caffe2/core/net_gpu_test.cc index 5c3c701d79340..ddffc5e2bdf95 100644 --- a/caffe2/core/net_gpu_test.cc +++ b/caffe2/core/net_gpu_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/common_gpu.h" #include "caffe2/core/net.h" @@ -93,7 +92,7 @@ void checkChainingAndRun( Workspace ws; ws.CreateBlob("in"); NetDef net_def; - CAFFE_ENFORCE(google::protobuf::TextFormat::ParseFromString(spec, &net_def)); + CAFFE_ENFORCE(TextFormat::ParseFromString(spec, &net_def)); { net_def.set_num_workers(4); auto old = FLAGS_caffe2_disable_chaining; diff --git a/caffe2/core/net_test.cc b/caffe2/core/net_test.cc index 864e2bbc27e46..1202f2da02e1f 100644 --- a/caffe2/core/net_test.cc +++ b/caffe2/core/net_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/net.h" #include "caffe2/core/net_dag.h" @@ -161,7 +160,7 @@ void checkChainingAndRun( Workspace ws; ws.CreateBlob("in"); NetDef net_def; - CAFFE_ENFORCE(google::protobuf::TextFormat::ParseFromString(spec, &net_def)); + CAFFE_ENFORCE(TextFormat::ParseFromString(spec, &net_def)); { net_def.set_num_workers(4); auto old = FLAGS_caffe2_disable_chaining; @@ -181,7 +180,7 @@ void checkNumChainsAndRun(const char* spec, const int expected_num_chains) { Workspace ws; NetDef net_def; - CAFFE_ENFORCE(google::protobuf::TextFormat::ParseFromString(spec, &net_def)); + CAFFE_ENFORCE(TextFormat::ParseFromString(spec, &net_def)); net_def.set_num_workers(4); // Create all external inputs @@ -580,7 +579,7 @@ TEST(NetTest, FailingOperator) { ws.CreateBlob("in"); NetDef net_def; - CAFFE_ENFORCE(google::protobuf::TextFormat::ParseFromString(spec, &net_def)); + CAFFE_ENFORCE(TextFormat::ParseFromString(spec, &net_def)); { net_def.set_num_workers(4); diff --git a/caffe2/core/observer_test.cc b/caffe2/core/observer_test.cc index d9ad5b20a7122..fac7afed23dbe 100644 --- a/caffe2/core/observer_test.cc +++ b/caffe2/core/observer_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/common.h" #include "caffe2/core/net.h" diff --git a/caffe2/core/parallel_net_test.cc b/caffe2/core/parallel_net_test.cc index fa2ffbed3a00b..fd6504e6b366b 100644 --- a/caffe2/core/parallel_net_test.cc +++ b/caffe2/core/parallel_net_test.cc @@ -20,7 +20,6 @@ #include "caffe2/core/net.h" #include "caffe2/core/operator.h" -#include "google/protobuf/text_format.h" #include namespace caffe2 { @@ -104,7 +103,7 @@ namespace { int RunNetAndGetDuration(const string& net_def_str, const string& type) { NetDef net_def; CAFFE_ENFORCE( - google::protobuf::TextFormat::ParseFromString(net_def_str, &net_def)); + TextFormat::ParseFromString(net_def_str, &net_def)); net_def.set_type(type); Workspace ws; unique_ptr net(CreateNet(net_def, &ws)); diff --git a/caffe2/core/predictor_test.cc b/caffe2/core/predictor_test.cc index 5a51bc84ca052..dc6978e751a40 100644 --- a/caffe2/core/predictor_test.cc +++ b/caffe2/core/predictor_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include "caffe2/core/context.h" #include "caffe2/core/operator.h" #include "caffe2/core/predictor.h" @@ -158,7 +157,7 @@ std::unique_ptr randomTensor( NetDef parseNetDef(const std::string& value) { NetDef def; CAFFE_ENFORCE( - google::protobuf::TextFormat::ParseFromString(value, &def), + TextFormat::ParseFromString(value, &def), "Failed to parse NetDef with value: ", value); return def; @@ -167,7 +166,7 @@ NetDef parseNetDef(const std::string& value) { MetaNetDef parseMetaNetDef(const std::string& value) { MetaNetDef def; CAFFE_ENFORCE( - google::protobuf::TextFormat::ParseFromString(value, &def), + TextFormat::ParseFromString(value, &def), "Failed to parse NetDef with value: ", value); return def; diff --git a/caffe2/core/transform_test.cc b/caffe2/core/transform_test.cc index 1af2d31f9acd6..3b1625973b149 100644 --- a/caffe2/core/transform_test.cc +++ b/caffe2/core/transform_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/net.h" #include "caffe2/core/operator.h" diff --git a/caffe2/mpi/mpi_gpu_test.cc b/caffe2/mpi/mpi_gpu_test.cc index 0decba122386a..31047bded1559 100644 --- a/caffe2/mpi/mpi_gpu_test.cc +++ b/caffe2/mpi/mpi_gpu_test.cc @@ -19,7 +19,6 @@ #include "caffe2/core/net.h" #include "caffe2/core/operator.h" #include "caffe2/mpi/mpi_common.h" -#include "google/protobuf/text_format.h" #include CAFFE2_DEFINE_string( @@ -62,7 +61,7 @@ const char kBcastNet[] = R"NET( TEST(MPITest, TestMPIBroadcast) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kBcastNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -124,7 +123,7 @@ const char kReduceNet[] = R"NET( TEST(MPITest, TestMPIReduce) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kReduceNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -190,7 +189,7 @@ const char kMPIAllgatherNet[] = R"NET( TEST(MPITest, TestMPIAllgather) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kMPIAllgatherNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -253,7 +252,7 @@ const char kMPIAllreduceNet[] = R"NET( TEST(MPITest, TestMPIAllreduce) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kMPIAllreduceNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -315,7 +314,7 @@ const char kInPlaceMPIAllreduceNet[] = R"NET( TEST(MPITest, TestInPlaceMPIAllreduce) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kInPlaceMPIAllreduceNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); diff --git a/caffe2/mpi/mpi_test.cc b/caffe2/mpi/mpi_test.cc index ecf0ab59137ec..0787c81425aa5 100644 --- a/caffe2/mpi/mpi_test.cc +++ b/caffe2/mpi/mpi_test.cc @@ -18,7 +18,6 @@ #include "caffe2/core/net.h" #include "caffe2/core/operator.h" #include "caffe2/mpi/mpi_common.h" -#include "google/protobuf/text_format.h" #include CAFFE2_DEFINE_string( @@ -58,7 +57,7 @@ const char kBcastNet[] = R"NET( TEST(MPITest, TestMPIBroadcast) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kBcastNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -116,7 +115,7 @@ const char kReduceNet[] = R"NET( TEST(MPITest, TestMPIReduce) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kReduceNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -178,7 +177,7 @@ const char kMPIAllgatherNet[] = R"NET( TEST(MPITest, TestMPIAllgather) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kMPIAllgatherNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -236,7 +235,7 @@ const char kMPIAllreduceNet[] = R"NET( TEST(MPITest, TestMPIAllreduce) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kMPIAllreduceNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); @@ -293,7 +292,7 @@ const char kInPlaceMPIAllreduceNet[] = R"NET( TEST(MPITest, TestInPlaceMPIAllreduce) { NetDef net_def; - CHECK(google::protobuf::TextFormat::ParseFromString( + CHECK(TextFormat::ParseFromString( string(kInPlaceMPIAllreduceNet), &net_def)); // Let's set the network's constant fill value to be the mpi rank. auto* arg = net_def.mutable_op(1)->mutable_arg(1); diff --git a/caffe2/observers/time_observer_test.cc b/caffe2/observers/time_observer_test.cc index 6aa1abe72189b..af133854a010b 100644 --- a/caffe2/observers/time_observer_test.cc +++ b/caffe2/observers/time_observer_test.cc @@ -20,7 +20,6 @@ #include "caffe2/core/operator.h" #include "time_observer.h" -#include #include #include #include diff --git a/caffe2/onnx/backend.h b/caffe2/onnx/backend.h index eb9bfea4cec64..796fab20d9366 100644 --- a/caffe2/onnx/backend.h +++ b/caffe2/onnx/backend.h @@ -5,7 +5,6 @@ #include "device.h" #include "onnx/onnx_pb.h" -#include #include #include #include diff --git a/caffe2/operators/recurrent_network_blob_fetcher_op.h b/caffe2/operators/recurrent_network_blob_fetcher_op.h index e8f48d5222601..ac208e619007c 100644 --- a/caffe2/operators/recurrent_network_blob_fetcher_op.h +++ b/caffe2/operators/recurrent_network_blob_fetcher_op.h @@ -22,7 +22,6 @@ #include "caffe2/core/operator.h" #include "caffe2/core/tensor.h" #include "caffe2/operators/recurrent_network_op.h" -#include "google/protobuf/text_format.h" #include diff --git a/caffe2/operators/recurrent_network_op.cc b/caffe2/operators/recurrent_network_op.cc index d40812a9761fa..ee0af864c1b05 100644 --- a/caffe2/operators/recurrent_network_op.cc +++ b/caffe2/operators/recurrent_network_op.cc @@ -19,7 +19,6 @@ #include "caffe2/utils/proto_utils.h" #ifndef CAFFE2_RNN_NO_TEXT_FORMAT -#include #endif CAFFE2_DEFINE_bool( @@ -265,7 +264,7 @@ NetDef extractNetDef(const OperatorDef& op, const std::string& argName) { const auto netString = ArgumentHelper::GetSingleArgument(op, argName, ""); CAFFE_ENFORCE( - google::protobuf::TextFormat::ParseFromString(netString, &result), + TextFormat::ParseFromString(netString, &result), "Invalid NetDef"); return result; #else diff --git a/caffe2/transforms/common_subexpression_elimination_test.cc b/caffe2/transforms/common_subexpression_elimination_test.cc index 1480a7a5a27d7..7043a83875d76 100644 --- a/caffe2/transforms/common_subexpression_elimination_test.cc +++ b/caffe2/transforms/common_subexpression_elimination_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/net.h" #include "caffe2/core/operator.h" diff --git a/caffe2/transforms/conv_to_nnpack_transform_test.cc b/caffe2/transforms/conv_to_nnpack_transform_test.cc index 9a9bd5f941cc5..e371fa62feaa5 100644 --- a/caffe2/transforms/conv_to_nnpack_transform_test.cc +++ b/caffe2/transforms/conv_to_nnpack_transform_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/net.h" #include "caffe2/core/operator.h" diff --git a/caffe2/transforms/pattern_net_transform_test.cc b/caffe2/transforms/pattern_net_transform_test.cc index f8ee2f4c9d0b8..13ade6c1faf5d 100644 --- a/caffe2/transforms/pattern_net_transform_test.cc +++ b/caffe2/transforms/pattern_net_transform_test.cc @@ -14,7 +14,6 @@ * limitations under the License. */ -#include #include #include "caffe2/core/net.h" #include "caffe2/core/operator.h" diff --git a/caffe2/utils/proto_utils.cc b/caffe2/utils/proto_utils.cc index 2f46c86155dfa..065df103e0be6 100644 --- a/caffe2/utils/proto_utils.cc +++ b/caffe2/utils/proto_utils.cc @@ -29,11 +29,14 @@ #include "caffe2/core/logging.h" -using ::google::protobuf::Message; using ::google::protobuf::MessageLite; namespace caffe2 { +void ShutdownProtobufLibrary() { + ::google::protobuf::ShutdownProtobufLibrary(); +} + std::string DeviceTypeName(const int32_t& d) { switch (d) { case CPU: @@ -145,6 +148,13 @@ using ::google::protobuf::io::ZeroCopyInputStream; using ::google::protobuf::io::CodedInputStream; using ::google::protobuf::io::ZeroCopyOutputStream; using ::google::protobuf::io::CodedOutputStream; +using ::google::protobuf::Message; + +namespace TextFormat { +bool ParseFromString(const string& spec, Message* proto) { + return ::google::protobuf::TextFormat::ParseFromString(spec, proto); +} +} // namespace TextFormat bool ReadProtoFromTextFile(const char* filename, Message* proto) { int fd = open(filename, O_RDONLY); diff --git a/caffe2/utils/proto_utils.h b/caffe2/utils/proto_utils.h index 816281b806c03..f1ad672de21f2 100644 --- a/caffe2/utils/proto_utils.h +++ b/caffe2/utils/proto_utils.h @@ -31,6 +31,10 @@ namespace caffe2 { using std::string; using ::google::protobuf::MessageLite; +// A wrapper function to shut down protobuf library (this is needed in ASAN +// testing and valgrind cases to avoid protobuf appearing to "leak" memory). +void ShutdownProtobufLibrary(); + // A wrapper function to return device name string for use in blob serialization // / deserialization. This should have one to one correspondence with // caffe2/proto/caffe2.proto: enum DeviceType. @@ -61,6 +65,14 @@ inline void WriteProtoToBinaryFile(const MessageLite& proto, #ifdef CAFFE2_USE_LITE_PROTO +namespace TextFormat { +inline bool ParseFromString(const string& spec, MessageLite* proto) { + LOG(FATAL) << "If you are running lite version, you should not be " + << "calling any text-format protobuffers."; +} +} // namespace TextFormat + + inline string ProtoDebugString(const MessageLite& proto) { return proto.SerializeAsString(); } @@ -103,6 +115,10 @@ inline bool ReadProtoFromFile(const string& filename, MessageLite* proto) { using ::google::protobuf::Message; +namespace TextFormat { +bool ParseFromString(const string& spec, Message* proto); +} // namespace TextFormat + inline string ProtoDebugString(const Message& proto) { return proto.ShortDebugString(); } diff --git a/cmake/public/protobuf.cmake b/cmake/public/protobuf.cmake index fb3db6bcb87df..094d222b189d0 100644 --- a/cmake/public/protobuf.cmake +++ b/cmake/public/protobuf.cmake @@ -16,12 +16,21 @@ elseif(Protobuf_FOUND OR PROTOBUF_FOUND) # If the modern targets are not present, we will generate them for you for # backward compatibility. This is backported from CMake's new FindProtobuf.cmake # content. + if ((NOT PROTOBUF_LIBRARY) AND (NOT PROTOBUF_LITE_LIBRARY)) + message(FATAL_ERROR + "Caffe2: Found protobuf with old style targets, but could not find targets." + " PROTOBUF_LIBRARY: " ${PROTOBUF_LIBRARY} + " PROTOBUF_LITE_LIBRARY: " ${PROTOBUF_LITE_LIBRARY} + " Protobuf_LIBRARY: " ${Protobuf_LIBRARY} + " Protobuf_LITE_LIBRARY: " ${Protobuf_LITE_LIBRARY}) + endif() message(STATUS "Caffe2: Found protobuf with old-style protobuf targets.") + if(PROTOBUF_LIBRARY) if (NOT TARGET protobuf::libprotobuf) add_library(protobuf::libprotobuf UNKNOWN IMPORTED) set_target_properties(protobuf::libprotobuf PROPERTIES - INTERFACE_INCLUDE_DIRECTORIES "${Protobuf_INCLUDE_DIR}") + INTERFACE_INCLUDE_DIRECTORIES "${PROTOBUF_INCLUDE_DIRS}") endif() if(EXISTS "${PROTOBUF_LIBRARY}") set_target_properties(protobuf::libprotobuf PROPERTIES @@ -45,7 +54,7 @@ elseif(Protobuf_FOUND OR PROTOBUF_FOUND) if (NOT TARGET protobuf::libprotobuf-lite) add_library(protobuf::libprotobuf-lite UNKNOWN IMPORTED) set_target_properties(protobuf::libprotobuf-lite PROPERTIES - INTERFACE_INCLUDE_DIRECTORIES "${Protobuf_INCLUDE_DIR}") + INTERFACE_INCLUDE_DIRECTORIES "${PROTOBUF_INCLUDE_DIRS}") endif() if(EXISTS "${PROTOBUF_LITE_LIBRARY}") set_target_properties(protobuf::libprotobuf-lite PROPERTIES