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

refactor: move to flatbuffers feature #4553

Merged
merged 6 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build_windows_cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
run: ${{ env.VCPKG_ROOT }}/vcpkg.exe --triplet x64-windows install zlib flatbuffers
- name: Generate project files
run: |
cmake -S "${{ env.SOURCE_DIR }}" -B "${{ env.CMAKE_BUILD_DIR }}" -A "x64" -DVCPKG_MANIFEST_MODE=OFF -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_ROOT }}/scripts/buildsystems/vcpkg.cmake" -DBUILD_FLATBUFFERS=On -DVW_FEAT_CSV=On -Dvw_BUILD_NET_FRAMEWORK=On
cmake -S "${{ env.SOURCE_DIR }}" -B "${{ env.CMAKE_BUILD_DIR }}" -A "x64" -DVCPKG_MANIFEST_MODE=OFF -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_ROOT }}/scripts/buildsystems/vcpkg.cmake" -DVW_FEAT_FLATBUFFERS=On -DVW_FEAT_CSV=On -Dvw_BUILD_NET_FRAMEWORK=On
- name: Build project
run: |
cmake --build "${{ env.CMAKE_BUILD_DIR }}" --config ${{ matrix.build_config }} --verbose
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
schedule:
- cron: '26 11 * * 5'

concurrency:
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.sha }}
cancel-in-progress: true

Expand Down Expand Up @@ -67,12 +67,12 @@ jobs:
sudo apt update
sudo apt install -y ninja-build
dotnet tool install --global dotnet-t4
cmake -S . -B build -G Ninja -Dvw_BUILD_NET_CORE=On -Dvw_DOTNET_USE_MSPROJECT=Off -DBUILD_FLATBUFFERS=Off -DRAPIDJSON_SYS_DEP=Off -DFMT_SYS_DEP=Off -DSPDLOG_SYS_DEP=Off -DVW_ZLIB_SYS_DEP=Off -DVW_BOOST_MATH_SYS_DEP=Off -DVW_BUILD_VW_C_WRAPPER=Off -DBUILD_TESTING=Off -DBUILD_SHARED_LIBS=Off
cmake -S . -B build -G Ninja -Dvw_BUILD_NET_CORE=On -Dvw_DOTNET_USE_MSPROJECT=Off -DVW_FEAT_FLATBUFFERS=Off -DRAPIDJSON_SYS_DEP=Off -DFMT_SYS_DEP=Off -DSPDLOG_SYS_DEP=Off -DVW_ZLIB_SYS_DEP=Off -DVW_BOOST_MATH_SYS_DEP=Off -DVW_BUILD_VW_C_WRAPPER=Off -DBUILD_TESTING=Off -DBUILD_SHARED_LIBS=Off
cmake --build build --config Debug
- name: Build CSharp .NET Framework
if: ${{ matrix.config.language == 'csharp' && startsWith(matrix.config.os, 'windows') }}
run: |
cmake -S . -B build -G "Visual Studio 17 2022" -A x64 -Dvw_BUILD_NET_FRAMEWORK=On -DBUILD_FLATBUFFERS=Off -DRAPIDJSON_SYS_DEP=Off -DFMT_SYS_DEP=Off -DSPDLOG_SYS_DEP=Off -DVW_ZLIB_SYS_DEP=Off -DVW_BOOST_MATH_SYS_DEP=Off -DVW_BUILD_VW_C_WRAPPER=Off -DBUILD_TESTING=Off -DBUILD_SHARED_LIBS=Off
cmake -S . -B build -G "Visual Studio 17 2022" -A x64 -Dvw_BUILD_NET_FRAMEWORK=On -DVW_FEAT_FLATBUFFERS=Off -DRAPIDJSON_SYS_DEP=Off -DFMT_SYS_DEP=Off -DSPDLOG_SYS_DEP=Off -DVW_ZLIB_SYS_DEP=Off -DVW_BOOST_MATH_SYS_DEP=Off -DVW_BUILD_VW_C_WRAPPER=Off -DBUILD_TESTING=Off -DBUILD_SHARED_LIBS=Off
cmake --build build --config Debug -- /p:UseSharedCompilation=false
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v2
6 changes: 3 additions & 3 deletions .github/workflows/dotnet_nugets.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ on:
branches:
- '*'

concurrency:
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.sha }}
cancel-in-progress: true

Expand Down Expand Up @@ -61,7 +61,7 @@ jobs:
-DVW_NUGET_PACKAGE_NAME=VowpalWabbit
-DVW_NUGET_PACKAGE_VERSION="${{ steps.get_version.outputs.version }}"
-DVW_NUGET_PACKAGE_VERSION_TRIMMED="${{ steps.get_version.outputs.version_trimmed }}"
-DBUILD_FLATBUFFERS=Off
-DVW_FEAT_FLATBUFFERS=Off
-DRAPIDJSON_SYS_DEP=Off
-DFMT_SYS_DEP=Off
-DSPDLOG_SYS_DEP=Off
Expand All @@ -87,7 +87,7 @@ jobs:
-DVW_NUGET_PACKAGE_NAME=VowpalWabbit
-DVW_NUGET_PACKAGE_VERSION="${{ steps.get_version.outputs.version }}"
-DVW_NUGET_PACKAGE_VERSION_TRIMMED="${{ steps.get_version.outputs.version_trimmed }}"
-DBUILD_FLATBUFFERS=Off
-DVW_FEAT_FLATBUFFERS=Off
-DRAPIDJSON_SYS_DEP=Off
-DFMT_SYS_DEP=Off
-DSPDLOG_SYS_DEP=Off
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/native_nugets.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
-T ${{ matrix.toolset }}
-DVW_NUGET_PACKAGE_VERSION="${{ steps.get_version.outputs.version }}"
-DNATIVE_NUGET_PLATFORM_TAG="${{ matrix.arch_config.arch }}"
-DBUILD_FLATBUFFERS=Off
-DVW_FEAT_FLATBUFFERS=Off
-DBUILD_TESTING=Off
-DRAPIDJSON_SYS_DEP=Off
-DFMT_SYS_DEP=Off
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/valgrind.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
submodules: recursive
- name: Build C++ VW binary
run: |
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_EXPERIMENTAL_BINDING=On -DBUILD_FLATBUFFERS=On -DVW_FEAT_CSV=On
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_EXPERIMENTAL_BINDING=On -DVW_FEAT_FLATBUFFERS=On -DVW_FEAT_CSV=On
cmake --build build
- name: Upload vw binary
uses: actions/upload-artifact@v2
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/vendor_build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
run: >
cmake -S . -B build -G Ninja
-DCMAKE_BUILD_TYPE=${{matrix.build_type}}
-DBUILD_FLATBUFFERS=Off
-DVW_FEAT_FLATBUFFERS=Off
-DVW_FEAT_CSV=On
-DRAPIDJSON_SYS_DEP=Off
-DFMT_SYS_DEP=Off
Expand Down Expand Up @@ -75,7 +75,7 @@ jobs:
run: >
cmake -S "${{ env.SOURCE_DIR }}" -B "${{ env.CMAKE_BUILD_DIR }}" -A "x64"
-DUSE_LATEST_STD=On
-DBUILD_FLATBUFFERS=Off
-DVW_FEAT_FLATBUFFERS=Off
-DVW_FEAT_CSV=On
-DRAPIDJSON_SYS_DEP=Off
-DFMT_SYS_DEP=Off
Expand Down Expand Up @@ -107,7 +107,7 @@ jobs:
run: >
cmake -S . -B build -G Ninja
-DCMAKE_BUILD_TYPE=${{matrix.build_type}}
-DBUILD_FLATBUFFERS=Off
-DVW_FEAT_FLATBUFFERS=Off
-DVW_FEAT_CSV=On
-DRAPIDJSON_SYS_DEP=Off
-DFMT_SYS_DEP=Off
Expand Down
2 changes: 1 addition & 1 deletion .scripts/build.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ cmake -S "%vwRoot%" -B "%vwRoot%\build" -G "Visual Studio 16 2019" -A "x64" ^
-DCMAKE_TOOLCHAIN_FILE="%VCPKG_INSTALLATION_ROOT%\scripts\buildsystems\vcpkg.cmake" ^
-DVCPKG_MANIFEST_MODE=OFF ^
-Dvw_BUILD_NET_FRAMEWORK=On ^
-DBUILD_FLATBUFFERS=On ^
-DVW_FEAT_FLATBUFFERS=On ^
-DVW_FEAT_CSV=On ^
-Dvw_BUILD_NET_FRAMEWORK=On ^
-DRAPIDJSON_SYS_DEP=Off ^
Expand Down
2 changes: 1 addition & 1 deletion .scripts/linux/build-slim.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ cd $REPO_DIR
rm -rf build
cmake -S . -B build -G Ninja \
-DBUILD_TESTING=On \
-DBUILD_FLATBUFFERS=Off \
-DVW_FEAT_FLATBUFFERS=Off \
-DRAPIDJSON_SYS_DEP=Off \
-DFMT_SYS_DEP=Off \
-DSPDLOG_SYS_DEP=Off \
Expand Down
2 changes: 1 addition & 1 deletion .scripts/linux/build-static-java.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mkdir -p build
cd build
# Boost unit tests don't like the static linking
# /usr/local/bin/gcc + g++ is 9.2.0 version
cmake -E env LDFLAGS="-Wl,--exclude-libs,ALL -static-libgcc -static-libstdc++" cmake .. -DCMAKE_BUILD_TYPE=Release -DWARNINGS=Off -DBUILD_JAVA=On -DBUILD_DOCS=Off -DBUILD_FLATBUFFERS=On -DVW_FEAT_CSV=On\
cmake -E env LDFLAGS="-Wl,--exclude-libs,ALL -static-libgcc -static-libstdc++" cmake .. -DCMAKE_BUILD_TYPE=Release -DWARNINGS=Off -DBUILD_JAVA=On -DBUILD_DOCS=Off -DVW_FEAT_FLATBUFFERS=On -DVW_FEAT_CSV=On\
-DBUILD_PYTHON=Off -DSTATIC_LINK_VW_JAVA=On -DCMAKE_C_COMPILER=/usr/local/bin/gcc -DCMAKE_CXX_COMPILER=/usr/local/bin/g++ \
-DBUILD_TESTING=Off -DVW_ZLIB_SYS_DEP=Off -DBUILD_SHARED_LIBS=Off -DVW_BUILD_LAS_WITH_SIMD=Off
NUM_PROCESSORS=$(nproc)
Expand Down
2 changes: 1 addition & 1 deletion .scripts/linux/build-with-coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
REPO_DIR=$SCRIPT_DIR/../../
cd $REPO_DIR

cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DVW_GCOV=ON -DWARNINGS=OFF -DBUILD_JAVA=Off -DBUILD_PYTHON=Off -DBUILD_TESTING=On -DBUILD_FLATBUFFERS=On -DVW_FEAT_CSV=On
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DVW_GCOV=ON -DWARNINGS=OFF -DBUILD_JAVA=Off -DBUILD_PYTHON=Off -DBUILD_TESTING=On -DVW_FEAT_FLATBUFFERS=On -DVW_FEAT_CSV=On
cmake --build build
2 changes: 1 addition & 1 deletion .scripts/linux/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ cd $REPO_DIR
# If parameter 1 is not supplied, it defaults to Release
BUILD_CONFIGURATION=${1:-Release}

cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=${BUILD_CONFIGURATION} -DWARNINGS=Off -DWARNING_AS_ERROR=On -DVW_BUILD_VW_C_WRAPPER=Off -DBUILD_JAVA=On -DBUILD_PYTHON=Off -DBUILD_TESTING=On -DBUILD_EXPERIMENTAL_BINDING=On -DBUILD_FLATBUFFERS=On -DVW_FEAT_CSV=On
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=${BUILD_CONFIGURATION} -DWARNINGS=Off -DWARNING_AS_ERROR=On -DVW_BUILD_VW_C_WRAPPER=Off -DBUILD_JAVA=On -DBUILD_PYTHON=Off -DBUILD_TESTING=On -DBUILD_EXPERIMENTAL_BINDING=On -DVW_FEAT_FLATBUFFERS=On -DVW_FEAT_CSV=On
cmake --build build --target all
6 changes: 3 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ if(WIN32)
endif()
endif()

option(BUILD_FLATBUFFERS "Build flatbuffers" OFF)
if(BUILD_FLATBUFFERS)
option(VW_FEAT_FLATBUFFERS "Build flatbuffers" OFF)
if(VW_FEAT_FLATBUFFERS)
list(APPEND VCPKG_MANIFEST_FEATURES "flatbuffers")
endif()

Expand Down Expand Up @@ -232,7 +232,7 @@ include(ext_libs/ext_libs.cmake)
add_subdirectory(vowpalwabbit)
add_subdirectory(utl/dump_options)

if (BUILD_FLATBUFFERS)
if (VW_FEAT_FLATBUFFERS)
add_subdirectory(utl/flatbuffer)
endif()

Expand Down
2 changes: 1 addition & 1 deletion CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
"type": "BOOL",
"value": "On"
},
"BUILD_FLATBUFFERS": {
"VW_FEAT_FLATBUFFERS": {
"type": "BOOL",
"value": "On"
},
Expand Down
12 changes: 6 additions & 6 deletions cmake/VowpalWabbitFeatures.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
# LAS_SIMD: Enable large action space with explicit simd (only work with linux for now)

# set(VW_ALL_FEATURES "LAS_SIMD;FLATBUFFERS;CSV")
set(VW_ALL_FEATURES "CSV")
set(VW_ALL_FEATURES "CSV;FLATBUFFERS")

# option(VW_FEAT_FLATBUFFERS "Enable flatbuffers support" OFF)
option(VW_FEAT_FLATBUFFERS "Enable flatbuffers support" OFF)
option(VW_FEAT_CSV "Enable csv parser" OFF)
# option(VW_FEAT_LAS_SIMD "Enable large action space with explicit simd (only works with linux for now)" ON)

# Legacy options for feature enablement
# if(DEFINED BUILD_FLATBUFFERS)
# message(WARNING "BUILD_FLATBUFFERS is deprecated. Use -DVW_FEAT_FLATBUFFERS=On instead.")
# set(VW_FEAT_FLATBUFFERS ${BUILD_FLATBUFFERS} CACHE BOOL "" FORCE)
# endif()
if(DEFINED BUILD_FLATBUFFERS)
message(WARNING "BUILD_FLATBUFFERS is deprecated. Use -DVW_FEAT_FLATBUFFERS=On instead.")
set(VW_FEAT_FLATBUFFERS ${BUILD_FLATBUFFERS} CACHE BOOL "" FORCE)
endif()

if(DEFINED VW_BUILD_CSV)
message(WARNING "VW_BUILD_CSV is deprecated. Use -DVW_FEAT_CSV=On instead.")
Expand Down
2 changes: 1 addition & 1 deletion ext_libs/ext_libs.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ else()
add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/string-view-lite)
endif()

if(BUILD_FLATBUFFERS)
if(VW_FEAT_FLATBUFFERS)
find_package(Flatbuffers CONFIG QUIET)
if(FLATBUFFERS_FOUND)
get_property(flatc_location TARGET flatbuffers::flatc PROPERTY LOCATION)
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ add_subdirectory(text_parser)
if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME AND BUILD_TESTING)
add_subdirectory(test_common)
endif()
if(BUILD_FLATBUFFERS)
if(VW_FEAT_FLATBUFFERS)
add_subdirectory(fb_parser)
endif()
3 changes: 1 addition & 2 deletions vowpalwabbit/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,8 @@ if(VW_FEAT_CSV)
target_link_libraries(vw_core PRIVATE vw_csv_parser)
endif()

if(BUILD_FLATBUFFERS)
if(VW_FEAT_FLATBUFFERS)
target_link_libraries(vw_core PRIVATE vw_fb_parser)
target_compile_definitions(vw_core PUBLIC BUILD_FLATBUFFERS)
endif()

# Handle generated header
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/core/include/vw/core/global_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ class parser_runtime
std::thread parse_thread;
size_t max_examples; // for TLC
bool chain_hash_json = false;
#ifdef BUILD_FLATBUFFERS
#ifdef VW_FEAT_FLATBUFFERS_ENABLED
std::unique_ptr<VW::parsers::flatbuffer::parser> flat_converter;
#endif
};
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/core/src/global_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
#include <iostream>
#include <sstream>

#ifdef BUILD_FLATBUFFERS
#ifdef VW_FEAT_FLATBUFFERS_ENABLED
# include "vw/fb_parser/parse_example_flatbuffer.h"
#endif

Expand Down
4 changes: 2 additions & 2 deletions vowpalwabbit/core/src/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ int VW_GETPID() { return (int)::GetCurrentProcessId(); }
#include <cassert>
#include <cerrno>
#include <cstdio>
#ifdef BUILD_FLATBUFFERS
#ifdef VW_FEAT_FLATBUFFERS_ENABLED
# include "vw/fb_parser/parse_example_flatbuffer.h"
#endif

Expand Down Expand Up @@ -631,7 +631,7 @@ void VW::details::enable_sources(
}

if (input_options.json || input_options.dsjson) { set_json_reader(all, input_options.dsjson); }
#ifdef BUILD_FLATBUFFERS
#ifdef VW_FEAT_FLATBUFFERS_ENABLED
else if (input_options.flatbuffer)
{
all.parser_runtime.flat_converter = VW::make_unique<VW::parsers::flatbuffer::parser>();
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/fb_parser/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ vw_add_library(
TYPE "STATIC_ONLY"
SOURCES ${vw_fb_parser_sources}
PUBLIC_DEPS vw_core $<BUILD_INTERFACE:fb_generate_headers>
DESCRIPTION "Parser implementation that reads flatbuffer examples. Disabled by default. Enable with `BUILD_FLATBUFFERS`"
DESCRIPTION "Parser implementation that reads flatbuffer examples. Disabled by default. Enable with `VW_FEAT_FLATBUFFERS=ON`"
EXCEPTION_DESCRIPTION "Yes"
)
target_include_directories(vw_fb_parser PUBLIC ${FLATBUFFERS_INCLUDE_DIR})
Expand Down
2 changes: 1 addition & 1 deletion vowpalwabbit/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Some common targets were renamed to match the new consistent style:
| slim | vw_slim | STATIC_ONLY | Minimal inference only runtime | vw_common, vw_explore | | No |
| spanning_tree | vw_spanning_tree_bin | EXECUTABLE | Command line tool for connecting instances of vw for distributed learning | | vw_spanning_tree, vw_common, vw_config | N/A |
| spanning_tree | vw_spanning_tree | STATIC_ONLY | Supporting code for connecting instances of VW for distributed learning | vw_common | Threads::Threads | Yes |
| fb_parser | vw_fb_parser | STATIC_ONLY | Parser implementation that reads flatbuffer examples. Disabled by default. Enable with `BUILD_FLATBUFFERS`. | vw_core, fb_generate_headers | | Yes |
| fb_parser | vw_fb_parser | STATIC_ONLY | Parser implementation that reads flatbuffer examples. Disabled by default. Enable with `VW_FEAT_FLATBUFFERS=On`. | vw_core, fb_generate_headers | | Yes |

How to generate the above table:
1. Run CMake configure with `-DVW_OUPUT_LIB_DESCRIPTIONS=On`
Expand Down