velox
velox copied to clipboard
Add option to build monolithic, shared library
This introduces the option to build a shared version of the monolithic libvelox.a, the result is a much smaller build tree, library and executables. They are now so small that moving them is much easier, e.g. for test sharding.
I also fixed some miscellaneous things in this PR:
- Removed
FILESYSTEM, the link is not necessary with GCC >= 9 which is our minimum. - Replaced
FOLLYBENCHMARKwith the proper target - Explicitly linked the gpu targets to CUDA
- Improved bundled folly's use of glog and gflags (when also bundled)
- Cleaned up the usage of an addition proto wrapper target so we don't link generated proto files twice (which doesn't cause issues in a static build)
- Added a marker in the log so it's clear when we are running velox cmake vs. dependency cmake:
-- [xsimd] xsimd v10.0.0
-- [stemmer] Building stemmer from source
-- Setting Arrow source to AUTO
-- [Arrow] Checking for module 'thrift'
Initially I looked at forcing a bundled build of folly and gflags to make sure they are shared but that introduces weird rpath issues when the dependencies are also on the system (like our CI image), instead I just changed the build script to build folly as shared in the image, we maybe want to make that an option so people can keep folly as static (though even for static build shared folly will likely make the binary size much smaller.). If folly is build from source it will be built static or shared matching the velox build.
Deploy Preview for meta-velox canceled.
| Name | Link |
|---|---|
| Latest commit | 5920c93f3b0aa9821b8178d8e71611a7ea5c1984 |
| Latest deploy log | https://app.netlify.com/sites/meta-velox/deploys/675310f4f6e297000847d3f4 |
@assignUser, this is just a reply to the mentioned issue in PR description. In Gluten, we are using static folly lib, as well as gflags, for velox to link. Can upstream velox link these static libs also?
@PHILO-HE that's what we normaly do as well but when building velox as an .so it doesn't work because of the mentioned linking issue.
gflags(static -> linked into folly(static) -> linked intod velox(shared) -> linked to tests(shared) -> test also links to gflags (static)
@PHILO-HE that's what we normaly do as well but when building velox as an .so it doesn't work because of the mentioned linking issue. gflags(static -> linked into folly(static) -> linked intod velox(shared) -> linked to tests(shared) -> test also links to gflags (static)
@assignUser, I see. Thanks for your clarification!
Can we use link option --version-script to hide gflags symbols in the linking? We did this similarly in Gluten, which is a bit tricky maybe. https://github.com/apache/incubator-gluten/blob/b7918a9a4b3482c03283dc542a909669d2ef4534/cpp/core/symbols.map#L10
I am unclear on the one failed test, would be great if someone could have a look. But otherwise this is pretty done, we are currently using a temp CI image of course.
I tried doing the monolithic shared build and ran into the error: Undefined symbols for architecture arm64. It seems to have something to do with facebook::velox::fuzzer::ArgumentTypeFuzzer::randOrderableType()?
[2584/3469] Linking CXX shared library lib/libvelox.dylib
ld: warning: ignoring duplicate libraries: 'CMake/resolve_dependency_modules/arrow/arrow_ep/install/lib/libarrow.a', 'CMake/resolve_dependency_modules/arrow/arrow_ep/src/arrow_ep-build/thrift_ep-install/lib/libthrift.a', '_deps/duckdb-build/src/libduckdb_static.a', '_deps/fmt-build/libfmtd.a', '_deps/protobuf-build/libprotobufd.a', '_deps/re2-build/libre2.a', '_deps/simdjson-build/libsimdjson.a'
[2586/3469] Linking CXX shared library velox/vector/tests/utils/libvelox_vector_test_lib.dylib
ld: warning: ignoring duplicate libraries: '/opt/homebrew/lib/libgtest.a'
[2595/3469] Linking CXX shared library velox/dwio/common/tests/utils/libvelox_dwio_common_test_utils.dylib
ld: warning: ignoring duplicate libraries: '/opt/homebrew/lib/libgtest.a', '_deps/fmt-build/libfmtd.a'
[2611/3469] Linking CXX shared library velox/expression/fuzzer/libvelox_expression_test_utility.dylib
FAILED: velox/expression/fuzzer/libvelox_expression_test_utility.dylib
: && /Library/Developer/CommandLineTools/usr/bin/c++ -mcpu=apple-m1+crc -D USE_VELOX_COMMON_BASE -D HAS_UNCAUGHT_EXCEPTIONS -Wall -Wextra -Wno-unused -Wno-unused-parameter -Wno-sign-compare -Wno-ignored-qualifiers -Wno-range-loop-analysis -Wno-mismatched-tags -Wno-nullability-completeness -g -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk -mmacosx-version-min=14.6 -dynamiclib -Wl,-headerpad_max_install_names -Wl,-flat_namespace -o velox/expression/fuzzer/libvelox_expression_test_utility.dylib -install_name @rpath/libvelox_expression_test_utility.dylib velox/expression/fuzzer/CMakeFiles/velox_expression_test_utility.dir/ArgumentTypeFuzzer.cpp.o velox/expression/fuzzer/CMakeFiles/velox_expression_test_utility.dir/FuzzerToolkit.cpp.o -Wl,-rpath,/Users/danielhunte/velox/_build/debug/lib -Wl,-rpath,/Users/danielhunte/velox/_build/debug/_deps/folly-build -Wl,-rpath,/opt/homebrew/lib lib/libvelox.dylib /opt/homebrew/lib/libgtest.a _deps/folly-build/libfolly.0.58.0-dev.dylib /opt/homebrew/lib/libboost_context-mt.dylib /opt/homebrew/lib/libboost_filesystem-mt.dylib /opt/homebrew/lib/libboost_atomic-mt.dylib /opt/homebrew/lib/libboost_program_options-mt.dylib /opt/homebrew/lib/libboost_system-mt.dylib /opt/homebrew/lib/libboost_thread-mt.dylib /opt/homebrew/lib/libdouble-conversion.dylib /opt/homebrew/lib/libgflags.2.2.2.dylib /opt/homebrew/lib/libglog.dylib /opt/homebrew/lib/libevent.dylib /Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk/usr/lib/libz.tbd /opt/homebrew/Cellar/openssl@3/3.3.2/lib/libssl.dylib /opt/homebrew/Cellar/openssl@3/3.3.2/lib/libcrypto.dylib /Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk/usr/lib/libbz2.tbd /Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk/usr/lib/liblzma.tbd /opt/homebrew/lib/liblz4.dylib /opt/homebrew/lib/libzstd.dylib /opt/homebrew/lib/libsnappy.dylib /opt/homebrew/lib/libsodium.dylib -lc++abi _deps/fmt-build/libfmtd.a /opt/homebrew/lib/libgflags.2.2.2.dylib /opt/homebrew/lib/libglog.dylib _deps/re2-build/libre2.a /opt/homebrew/lib/libboost_regex-mt.dylib /opt/homebrew/lib/libdouble-conversion.3.3.0.dylib _deps/protobuf-build/libprotobufd.a /opt/homebrew/lib/libsnappy.dylib /opt/homebrew/lib/libzstd.dylib /opt/homebrew/lib/liblz4.dylib /opt/homebrew/lib/liblzo2.dylib /Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk/usr/lib/libz.tbd CMake/resolve_dependency_modules/arrow/arrow_ep/install/lib/libarrow.a CMake/resolve_dependency_modules/arrow/arrow_ep/src/arrow_ep-build/thrift_ep-install/lib/libthrift.a velox/tpch/gen/dbgen/libdbgen.a _deps/simdjson-build/libsimdjson.a _deps/libstemmer/src/libstemmer/libstemmer.a _deps/duckdb-build/src/libduckdb_static.a _deps/duckdb-build/third_party/fsst/libduckdb_fsst.a _deps/duckdb-build/third_party/fmt/libduckdb_fmt.a _deps/duckdb-build/third_party/libpg_query/libduckdb_pg_query.a _deps/duckdb-build/third_party/re2/libduckdb_re2.a _deps/duckdb-build/third_party/miniz/libduckdb_miniz.a _deps/duckdb-build/third_party/utf8proc/libduckdb_utf8proc.a _deps/duckdb-build/third_party/hyperloglog/libduckdb_hyperloglog.a _deps/duckdb-build/third_party/fastpforlib/libduckdb_fastpforlib.a _deps/duckdb-build/third_party/mbedtls/libduckdb_mbedtls.a && :
Undefined symbols for architecture arm64:
"facebook::velox::randOrderableType(std::__1::mersenne_twister_engine<unsigned int, 32ul, 624ul, 397ul, 31ul, 2567483615u, 11ul, 4294967295u, 7ul, 2636928640u, 15ul, 4022730752u, 18ul, 1812433253u>&, int)", referenced from:
facebook::velox::fuzzer::ArgumentTypeFuzzer::randOrderableType() in ArgumentTypeFuzzer.cpp.o
"facebook::velox::randType(std::__1::mersenne_twister_engine<unsigned int, 32ul, 624ul, 397ul, 31ul, 2567483615u, 11ul, 4294967295u, 7ul, 2636928640u, 15ul, 4022730752u, 18ul, 1812433253u>&, int)", referenced from:
facebook::velox::fuzzer::ArgumentTypeFuzzer::randType() in ArgumentTypeFuzzer.cpp.o
ld: symbol(s) not found for architecture arm64
c++: error: linker command failed with exit code 1 (use -v to see invocation)
[2620/3469] Building CXX object velox/expression/fuzzer/CMakeFiles/velox_expression_fuzzer.dir/ExpressionFuzzer.cpp.o
In file included from /Users/danielhunte/velox/velox/expression/fuzzer/ExpressionFuzzer.cpp:28:
In file included from /Users/danielhunte/velox/./velox/expression/SimpleFunctionRegistry.h:21:
/Users/danielhunte/velox/./velox/expression/SimpleFunctionAdapter.h:286:62: warning: 'unique' is deprecated [-Wdeprecated-declarations]
286 | if (args[POSITION]->isFlatEncoding() && args[POSITION].unique() &&
| ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk/usr/include/c++/v1/__memory/shared_ptr.h:730:3: note: 'unique' has been explicitly marked deprecated here
730 | _LIBCPP_DEPRECATED_IN_CXX17 _LIBCPP_HIDE_FROM_ABI bool unique() const _NOEXCEPT { return use_count() == 1; }
| ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk/usr/include/c++/v1/__config:1022:41: note: expanded from macro '_LIBCPP_DEPRECATED_IN_CXX17'
1022 | # define _LIBCPP_DEPRECATED_IN_CXX17 _LIBCPP_DEPRECATED
| ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.0.sdk/usr/include/c++/v1/__config:995:49: note: expanded from macro '_LIBCPP_DEPRECATED'
995 | # define _LIBCPP_DEPRECATED __attribute__((__deprecated__))
| ^
1 warning generated.
ninja: build stopped: subcommand failed.
make[1]: *** [build] Error 1
make: *** [debug] Error 2```
Importing to check internal build.
@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@kgpai, @assignUser Should we wait until https://github.com/facebookincubator/velox/pull/11745 lands?
@kgpai merged this pull request in facebookincubator/velox@164b4c2beb8ec926fdb1e270c24aa6d38ccb195a.