Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProtocolBench Build Rules #626

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
77 changes: 47 additions & 30 deletions ThriftLibrary.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ endmacro()
# #include_prefix
# )
# add_library(somelib ...)
# target_link_libraries(somelibe ${file_name}-${language} ...)
# target_link_libraries(somelib ${file_name}-${language} ...)
#

macro(thrift_library
Expand Down Expand Up @@ -167,15 +167,21 @@ endmacro()
#
# thrift_generate
# This is used to codegen thrift files using the thrift compiler
# Supports library names that differ from the file name (to handle two libraries
# with the same filename on disk (in different folders))
# Params:
# @file_name - The name of tge thrift file
# @file_name - Input file name. Will be used for naming the CMake
# target if TARGET_NAME_BASE is not specified.
# @services - A list of services that are declared in the thrift file
# @language - The generator to use (cpp, cpp2 or py3)
# @options - Extra options to pass to the generator
# @output_path - The directory where the thrift file lives
# @include_prefix - Prefix to use for thrift includes in generated sources
# @TARGET_NAME_BASE - (optional) - name used for target instead of real filename
# @THRIFT_INCLUDE_DIRECTORIES - (optional) path to thrift include directories
#
# Output:
# file-language-target - A custom target to add a dependenct
# file-language-target - A custom target to add a dependency
# ${file-language-HEADERS} - The generated Header Files.
# ${file-language-SOURCES} - The generated Source Files.
#
Expand All @@ -186,7 +192,6 @@ endmacro()
# bypass_source_check(${file_language-SOURCES})
# This will prevent cmake from complaining about missing source files
#

macro(thrift_generate
file_name
services
Expand All @@ -198,43 +203,55 @@ macro(thrift_generate
)
cmake_parse_arguments(THRIFT_GENERATE # Prefix
"" # Options
"" # One Value args
"TARGET_NAME_BASE" # One Value args
"THRIFT_INCLUDE_DIRECTORIES" # Multi-value args
"${ARGN}")

set(source_file_name ${file_name})
set(target_file_name ${file_name})
set(thrift_include_directories)
foreach(dir ${THRIFT_GENERATE_THRIFT_INCLUDE_DIRECTORIES})
list(APPEND thrift_include_directories "-I" "${dir}")
endforeach()
if(DEFINED THRIFT_GENERATE_TARGET_NAME_BASE
AND NOT THRIFT_GENERATE_TARGET_NAME_BASE STREQUAL "")
set(target_file_name ${THRIFT_GENERATE_TARGET_NAME_BASE})
endif()

set("${file_name}-${language}-HEADERS"
${output_path}/gen-${language}/${file_name}_constants.h
${output_path}/gen-${language}/${file_name}_data.h
${output_path}/gen-${language}/${file_name}_metadata.h
${output_path}/gen-${language}/${file_name}_types.h
${output_path}/gen-${language}/${file_name}_types.tcc
set("${target_file_name}-${language}-HEADERS"
${output_path}/gen-${language}/${source_file_name}_constants.h
${output_path}/gen-${language}/${source_file_name}_data.h
${output_path}/gen-${language}/${source_file_name}_metadata.h
${output_path}/gen-${language}/${source_file_name}_types.h
${output_path}/gen-${language}/${source_file_name}_types.tcc
)
set("${file_name}-${language}-SOURCES"
${output_path}/gen-${language}/${file_name}_constants.cpp
${output_path}/gen-${language}/${file_name}_data.cpp
${output_path}/gen-${language}/${file_name}_types.cpp
set("${target_file_name}-${language}-SOURCES"
${output_path}/gen-${language}/${source_file_name}_constants.cpp
${output_path}/gen-${language}/${source_file_name}_data.cpp
${output_path}/gen-${language}/${source_file_name}_types.cpp
)
if("${options}" MATCHES "layouts")
set("${target_file_name}-${language}-SOURCES"
${${target_file_name}-${language}-SOURCES}
${output_path}/gen-${language}/${source_file_name}_layouts.cpp
)
endif()
if(NOT "${options}" MATCHES "no_metadata")
set("${file_name}-${language}-SOURCES"
${${file_name}-${language}-SOURCES}
${output_path}/gen-${language}/${file_name}_metadata.cpp
set("${target_file_name}-${language}-SOURCES"
${${target_file_name}-${language}-SOURCES}
${output_path}/gen-${language}/${source_file_name}_metadata.cpp
)
endif()
foreach(service ${services})
set("${file_name}-${language}-HEADERS"
${${file_name}-${language}-HEADERS}
set("${target_file_name}-${language}-HEADERS"
${${source_file_name}-${language}-HEADERS}
${output_path}/gen-${language}/${service}.h
${output_path}/gen-${language}/${service}.tcc
${output_path}/gen-${language}/${service}AsyncClient.h
${output_path}/gen-${language}/${service}_custom_protocol.h
)
set("${file_name}-${language}-SOURCES"
${${file_name}-${language}-SOURCES}
set("${target_file_name}-${language}-SOURCES"
${${source_file_name}-${language}-SOURCES}
${output_path}/gen-${language}/${service}.cpp
${output_path}/gen-${language}/${service}AsyncClient.cpp
)
Expand All @@ -252,25 +269,25 @@ macro(thrift_generate
set(gen_language "mstch_cpp2")
elseif("${language}" STREQUAL "py3")
set(gen_language "mstch_py3")
file(WRITE "${output_path}/gen-${language}/${file_name}/__init__.py")
file(WRITE "${output_path}/gen-${language}/${source_file_name}/__init__.py")
endif()
add_custom_command(
OUTPUT ${${file_name}-${language}-HEADERS}
${${file_name}-${language}-SOURCES}
OUTPUT ${${target_file_name}-${language}-HEADERS}
${${target_file_name}-${language}-SOURCES}
COMMAND ${THRIFT1}
--gen "${gen_language}:${options}${include_prefix_text}"
-o ${output_path}
${thrift_include_directories}
"${file_path}/${file_name}.thrift"
"${file_path}/${source_file_name}.thrift"
DEPENDS
${THRIFT1}
"${file_path}/${file_name}.thrift"
COMMENT "Generating ${file_name} files. Output: ${output_path}"
"${file_path}/${source_file_name}.thrift"
COMMENT "Generating ${target_file_name} files. Output: ${output_path}"
)
add_custom_target(
${file_name}-${language}-target ALL
${target_file_name}-${language}-target ALL
DEPENDS ${${language}-${language}-HEADERS}
${${file_name}-${language}-SOURCES}
${${target_file_name}-${language}-SOURCES}
)
install(
DIRECTORY gen-${language}
Expand Down
4 changes: 4 additions & 0 deletions thrift/lib/cpp2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
# Set the cpp2 directory
set(LIB_CPP2_HOME ${CMAKE_CURRENT_SOURCE_DIR})

if(enable_tests)
add_subdirectory(protocol/test)
endif(enable_tests)
add_subdirectory(test)

#######
# CMAKE variables only have local/subdirectory scope
Expand Down
19 changes: 13 additions & 6 deletions thrift/lib/cpp2/protocol/test/BinaryProtocolTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,25 @@ bool makeInvalidBool() {
return *reinterpret_cast<const volatile bool*>("\x42");
}

TEST_F(BinaryProtocolTest, writeInvalidBool) {
auto w = BinaryProtocolWriter();
void testWriteInvalidBool() {
auto w = BinaryProtocolWriter();
auto q = folly::IOBufQueue();
w.setOutput(&q);
// writeBool should either fail CHECK or write a valid bool.
EXPECT_DEATH(
{
w.writeBool(makeInvalidBool());

w.writeBool(makeInvalidBool());
auto s = std::string();
q.appendToString(s);
// Die on success.
CHECK(s != std::string(1, '\0')) << "invalid bool value";

CHECK(s != std::string(1, '\0')) << "invalid bool value";


}
TEST_F(BinaryProtocolTest, writeInvalidBool) {
EXPECT_DEATH(
{
testWriteInvalidBool();
},
"invalid bool value");
}
Expand Down
60 changes: 60 additions & 0 deletions thrift/lib/cpp2/protocol/test/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Copyright (c) Meta Platforms, Inc. and affiliates and Contributors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
include(GoogleTest)

# Generate required thrift dependencies
foreach(thrift_file Module CompactProtocolTestStructs)
thrift_generate(
"${thrift_file}" #file_name
"" #services
"cpp2" #language
"frozen2,layouts" #options
"${CMAKE_CURRENT_SOURCE_DIR}" #file_path
"${CMAKE_CURRENT_BINARY_DIR}" #output_path
"thrift/lib/cpp2/protocol/test" #include_prefix
THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR}
)
set_source_files_properties(${thrift_file} PROPERTIES GENERATED TRUE)
set(GENERATED_THRIFT_SOURCES ${GENERATED_THRIFT_SOURCES} ${${thrift_file}-cpp2-SOURCES})
endforeach()

# Test files to compile
set(PROTOCOL_TESTS
BinaryProtocolTest
CompactProtocolTest
CompactV1ProtocolTest
ContainerSkippingTest
DebugProtocolTest
F14RoundTripTest
InterfaceTest
JSONProtocolTest
ProtocolReaderStructReadStateTest
ProtocolTest
SimpleJSONProtocolTest
TraitsTest)

# Build each test independently
foreach(test_name ${PROTOCOL_TESTS})
add_executable(${test_name}
${test_name}.cpp
${GENERATED_THRIFT_SOURCES})
target_compile_options(${test_name} PUBLIC
-Wno-shadow-compatible-local)
target_link_libraries(${test_name}
GTest::gtest_main
thriftcpp2
Folly::folly
)
gtest_discover_tests(${test_name})
endforeach()
22 changes: 13 additions & 9 deletions thrift/lib/cpp2/protocol/test/CompactProtocolTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,24 @@ bool makeInvalidBool() {
return *reinterpret_cast<const volatile bool*>("\x42");
}

void writeInvalidBoolCheck() {
auto w = CompactProtocolWriter();
auto q = folly::IOBufQueue();
w.setOutput(&q);
w.writeBool(makeInvalidBool());
auto s = std::string();
q.appendToString(s);
// Die on success.
CHECK(s != std::string(1, '\1') && s != std::string(1, '\2'))
<< "invalid bool value";
}

TEST(CompactProtocolTest, writeInvalidBool) {
auto w = CompactProtocolWriter();
auto q = folly::IOBufQueue();
w.setOutput(&q);
// writeBool should either throw or write a valid bool. The exact value may
// depend on the build mode because the optimizer can make use of the UB.
EXPECT_DEATH(
{
w.writeBool(makeInvalidBool());
auto s = std::string();
q.appendToString(s);
// Die on success.
CHECK(s != std::string(1, '\1') && s != std::string(1, '\2'))
<< "invalid bool value";
writeInvalidBoolCheck();
},
"invalid bool value");
}
Expand Down
1 change: 1 addition & 0 deletions thrift/lib/cpp2/protocol/test/ContainerSkippingTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <thrift/lib/cpp2/protocol/CompactProtocol.h>
#include <thrift/lib/cpp2/protocol/SimpleJSONProtocol.h>
#include <thrift/lib/cpp2/protocol/test/gen-cpp2/Module_types.h>
#include <thrift/lib/cpp2/protocol/test/gen-cpp2/Module_types.tcc>

namespace apache::thrift::test {

Expand Down
90 changes: 90 additions & 0 deletions thrift/lib/cpp2/test/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Copyright (c) Meta Platforms, Inc. and affiliates and Contributors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# This CMakeLists.txt takes care of building a selection of benchmarks and tests in this folder.
# Currently, it only builds ProtocolBench with its dependencies

# obtain paths to other generated thrift files from the global property space
get_property(protocol-cpp2-SOURCES GLOBAL PROPERTY
protocol-cpp2-SOURCES)
get_property(protocol_detail-cpp2-SOURCES GLOBAL PROPERTY
protocol_detail-cpp2-SOURCES)
get_property(frozen-cpp2-SOURCES GLOBAL PROPERTY
frozen-cpp2-SOURCES)
get_property(field_mask-cpp2-SOURCES GLOBAL PROPERTY
field_mask-cpp2-SOURCES)
vitaut marked this conversation as resolved.
Show resolved Hide resolved

# generate thrift files for this benchmark
thrift_generate(
"ProtocolBenchData" #file_name
"" #services
"cpp2" #language
"frozen2,layouts" #options
"${CMAKE_CURRENT_SOURCE_DIR}" #file_path
"${CMAKE_CURRENT_BINARY_DIR}" #output_path
"thrift/lib/cpp2/test" #include_prefix
THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR}
)

set(GENERATED_THRIFT_SOURCES ${ProtocolBenchData-cpp2-SOURCES}
${protocol-cpp2-SOURCES}
${protocol_detail-cpp2-SOURCES}
${field_mask-cpp2-SOURCES}
${frozen-cpp2-SOURCES})
set(SOURCE_FILES ProtocolBench.cpp)
set(ADDITIONAL_SOURCE_FILES ../../thrift/detail/protocol.cpp ../../thrift/TypeToMaskAdapter.cpp)

# generate conformance thrift files
foreach(conf_name "protocol" "type" "any" "serialization" "patch_data" "conformance")
thrift_generate(
"${conf_name}" #file_name
"" #services
"cpp2" #language
"frozen2" #options
"${CMAKE_CURRENT_SOURCE_DIR}/../../../conformance/if" #file_path
"${CMAKE_CURRENT_BINARY_DIR}/../../../conformance/if" #output_path
"thrift/conformance/if" #include_prefix
THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR}
TARGET_NAME_BASE "${conf_name}conformance"
)
set(GENERATED_THRIFT_SOURCES
${GENERATED_THRIFT_SOURCES}
${${conf_name}conformance-cpp2-SOURCES})
endforeach()

foreach(GENERATED ${GENERATED_THRIFT_SOURCES})
set_source_files_properties(${GENERATED} PROPERTIES GENERATED TRUE)
endforeach()

# build the executable
add_executable(ProtocolBench
${SOURCE_FILES}
${ADDITIONAL_SOURCE_FILES}
${GENERATED_THRIFT_SOURCES}
)
target_link_libraries(ProtocolBench
thriftcpp2
Folly::folly
Folly::follybenchmark
xxhash
)

install(
TARGETS ProtocolBench
EXPORT fbthrift-exports
RUNTIME DESTINATION ${BIN_INSTALL_DIR}
LIBRARY DESTINATION ${LIB_INSTALL_DIR}
ARCHIVE DESTINATION ${LIB_INSTALL_DIR}
)

2 changes: 2 additions & 0 deletions thrift/lib/cpp2/test/Structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,7 @@ void populateMap(F&& getter) {
}
}

#if defined __has_include && __has_include(<thrift/lib/cpp2/test/ProtoBufBenchData.pb.h>)
#include <thrift/lib/cpp2/test/ProtoBufStructs-inl.h>
#endif
#include <thrift/lib/cpp2/test/ThriftStructs-inl.h>
Loading