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

Do Not Merge : Upgrade fmt to 10.1.1 (from 8.0.1) (#7941) #8434

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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 .circleci/dist_compile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ commands:
executors:
build:
docker:
- image : ghcr.io/facebookincubator/velox-dev:circleci-avx
- image : ghcr.io/facebookincubator/velox-dev:new-cci
resource_class: 2xlarge
environment:
CC: /opt/rh/gcc-toolset-9/root/bin/gcc
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ jobs:
fetch-depth: 0
submodules: 'recursive'

- name: "Install dependencies"
if: ${{ github.event_name == 'pull_request' }}
run: source velox/scripts/setup-ubuntu.sh

- name: "Checkout Merge Base"
if: ${{ github.event_name == 'pull_request' }}
working-directory: velox
Expand All @@ -90,6 +86,10 @@ jobs:
git submodule update --init --recursive
echo $(git log -n 1)

- name: "Install dependencies"
if: ${{ github.event_name == 'pull_request' }}
run: source velox/scripts/setup-ubuntu.sh

- name: Build Baseline Benchmarks
if: ${{ github.event_name == 'pull_request' }}
working-directory: velox
Expand Down
8 changes: 3 additions & 5 deletions CMake/resolve_dependency_modules/fmt.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
# limitations under the License.
include_guard(GLOBAL)

set(VELOX_FMT_VERSION 8.0.1)
set(VELOX_FMT_VERSION 10.1.1)
set(VELOX_FMT_BUILD_SHA256_CHECKSUM
b06ca3130158c625848f3fb7418f235155a4d389b2abc3a6245fb01cb0eb1e01)
78b8c0a72b1c35e4443a7e308df52498252d1cefc2b08c9a97bc9ee6cfe61f8b)
set(VELOX_FMT_SOURCE_URL
"https://github.com/fmtlib/fmt/archive/${VELOX_FMT_VERSION}.tar.gz")

Expand All @@ -25,9 +25,7 @@ message(STATUS "Building fmt from source")
FetchContent_Declare(
fmt
URL ${VELOX_FMT_SOURCE_URL}
URL_HASH ${VELOX_FMT_BUILD_SHA256_CHECKSUM}
PATCH_COMMAND git apply ${CMAKE_CURRENT_LIST_DIR}/fmt/no-targets.patch)

URL_HASH ${VELOX_FMT_BUILD_SHA256_CHECKSUM})
# Force fmt to create fmt-config.cmake which can be found by other dependecies
# (e.g. folly)
set(FMT_INSTALL ON)
Expand Down
12 changes: 0 additions & 12 deletions CMake/resolve_dependency_modules/fmt/no-targets.patch

This file was deleted.

16 changes: 8 additions & 8 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,6 @@ if(VELOX_ENABLE_PARQUET)
set(VELOX_ENABLE_ARROW ON)
endif()

if(VELOX_ENABLE_REMOTE_FUNCTIONS)
# TODO: Move this to use resolve_dependency(). For some reason, FBThrift
# requires clients to explicitly install fizz and wangle.
find_package(fizz CONFIG REQUIRED)
find_package(wangle CONFIG REQUIRED)
find_package(FBThrift CONFIG REQUIRED)
endif()

# define processor variable for conditional compilation
if(${VELOX_CODEGEN_SUPPORT})
add_compile_definitions(CODEGEN_ENABLED=1)
Expand Down Expand Up @@ -457,6 +449,14 @@ add_compile_definitions(FOLLY_HAVE_INT128_T=1)
set_source(folly)
resolve_dependency(folly)

if(VELOX_ENABLE_REMOTE_FUNCTIONS)
# TODO: Move this to use resolve_dependency(). For some reason, FBThrift
# requires clients to explicitly install fizz and wangle.
find_package(fizz CONFIG REQUIRED)
find_package(wangle CONFIG REQUIRED)
find_package(FBThrift CONFIG REQUIRED)
endif()

if(DEFINED FOLLY_BENCHMARK_STATIC_LIB)
set(FOLLY_BENCHMARK ${FOLLY_BENCHMARK_STATIC_LIB})
else()
Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-centos8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog &
wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo &
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost &
wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy &
wget_and_untar https://github.com/fmtlib/fmt/archive/8.0.1.tar.gz fmt &
wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt &

wait # For cmake and source downloads to complete.

Expand Down
39 changes: 24 additions & 15 deletions scripts/setup-circleci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function dnf_install {

dnf_install epel-release dnf-plugins-core # For ccache, ninja
dnf config-manager --set-enabled powertools
dnf_install ninja-build ccache gcc-toolset-9 git wget which libevent-devel \
dnf_install ninja-build cmake curl ccache gcc-toolset-9 git wget which libevent-devel \
openssl-devel re2-devel libzstd-devel lz4-devel double-conversion-devel \
libdwarf-devel curl-devel libicu-devel

Expand Down Expand Up @@ -58,29 +58,34 @@ function wget_and_untar {
local URL=$1
local DIR=$2
mkdir -p "${DIR}"
wget -q --max-redirect 3 -O - "${URL}" | tar -xz -C "${DIR}" --strip-components=1
pushd "${DIR}"
curl -L "${URL}" > $2.tar.gz
tar -xz --strip-components=1 -f $2.tar.gz
popd
}

# untar cmake binary release directly to /usr.
wget_and_untar https://github.com/Kitware/CMake/releases/download/v3.17.5/cmake-3.17.5-Linux-x86_64.tar.gz /usr &

# Fetch sources.
wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags &
wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog &
wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo &
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost &
wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy &
wget_and_untar https://github.com/fmtlib/fmt/archive/8.0.1.tar.gz fmt &
# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 &
wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags
wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog
wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost
wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy
wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt

# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3
wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop
wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf &
wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf

FB_OS_VERSION="v2022.11.14.00"
FB_OS_VERSION="v2023.12.04.00"

wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz folly &
wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz &
wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle &
wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift &
wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz
wget_and_untar https://github.com/facebook/folly/archive/refs/tags/${FB_OS_VERSION}.tar.gz folly
wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle
wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift
wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst

wait # For cmake and source downloads to complete.

Expand Down Expand Up @@ -113,9 +118,13 @@ cmake_install glog -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr
cmake_install snappy -DSNAPPY_BUILD_TESTS=OFF
cmake_install fmt -DFMT_TEST=OFF
cmake_install folly -DFOLLY_HAVE_INT128_T=ON
# remove in #7700
sed -i 's/\[\[deprecated.*\]\]//g' /usr/local/include/folly/init/Init.h


cmake_install fizz/fizz -DBUILD_TESTS=OFF
cmake_install wangle/wangle -DBUILD_TESTS=OFF
cmake_install mvfst -DBUILD_TESTS=OFF
cmake_install fbthrift -Denable_tests=OFF
# cmake_install ranges-v3

Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-macos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function install_build_prerequisites {
}

function install_fmt {
github_checkout fmtlib/fmt 8.0.1
github_checkout fmtlib/fmt 10.1.1
cmake_install -DFMT_TEST=OFF
}

Expand Down
7 changes: 1 addition & 6 deletions scripts/setup-ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ sudo --preserve-env apt update && sudo --preserve-env apt install -y libunwind-d
libboost-all-dev \
libicu-dev \
libdouble-conversion-dev \
libfmt-dev \
libgoogle-glog-dev \
libbz2-dev \
libgflags-dev \
Expand Down Expand Up @@ -86,11 +87,6 @@ function prompt {
) 2> /dev/null
}

function install_fmt {
github_checkout fmtlib/fmt 8.0.1
cmake_install -DFMT_TEST=OFF
}

function install_folly {
github_checkout facebook/folly "${FB_OS_VERSION}"
cmake_install -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON
Expand Down Expand Up @@ -119,7 +115,6 @@ function install_conda {
}

function install_velox_deps {
run_and_time install_fmt
run_and_time install_folly
run_and_time install_fizz
run_and_time install_wangle
Expand Down
2 changes: 1 addition & 1 deletion scripts/setup-velox-torcharrow.sh
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ wget_and_untar https://github.com/gflags/gflags/archive/refs/tags/v2.2.2.tar.gz
wget_and_untar https://ftp.openssl.org/source/openssl-1.1.1k.tar.gz openssl &
wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.69.0/source/boost_1_69_0.tar.gz boost &
wget_and_untar https://github.com/facebook/folly/archive/v2022.11.14.00.tar.gz folly &
wget_and_untar https://github.com/fmtlib/fmt/archive/refs/tags/8.0.1.tar.gz fmt &
wget_and_untar https://github.com/fmtlib/fmt/archive/refs/tags/10.1.1.tar.gz fmt &

wait

Expand Down
9 changes: 9 additions & 0 deletions velox/benchmarks/basic/LikeTpchBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,16 @@ enum class TpchBenchmarkCase {
TpchQuery16Supplier,
TpchQuery20,
};
}

template <>
struct fmt::formatter<TpchBenchmarkCase> : fmt::formatter<int> {
auto format(const TpchBenchmarkCase& s, format_context& ctx) {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};

namespace {
class LikeFunctionsBenchmark : public FunctionBaseTest,
public FunctionBenchmarkBase {
public:
Expand Down
1 change: 1 addition & 0 deletions velox/common/base/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <folly/Conv.h>
#include <folly/Exception.h>
#include <folly/Preprocessor.h>
#include "FmtStdFormatters.h"
#include "velox/common/base/VeloxException.h"

namespace facebook::velox {
Expand Down
106 changes: 106 additions & 0 deletions velox/common/base/FmtStdFormatters.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* 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.
*/

#pragma once

#include <fmt/format.h>
#if FMT_VERSION >= 100100
#include <fmt/std.h>
#endif

#include <atomic>
#include <bitset>
#include <string_view>
#include <system_error>
#include <type_traits>
#include <vector>

template <typename Char>
struct fmt::formatter<std::errc, Char>
: formatter<std::underlying_type<std::errc>::type, Char> {
template <typename FormatContext>
auto format(std::errc v, FormatContext& ctx) const -> decltype(ctx.out()) {
using underlying_type = std::underlying_type<std::errc>::type;
return formatter<underlying_type, Char>::format(
static_cast<underlying_type>(v), ctx);
}
};

#if FMT_VERSION < 100100
// This should be 100101 but FMT_VERSION was not bumped in 10.1.1
// but under a month has passed since 10.1.0 release so we can assume 10.1.1
//
// Backport from fmt 10.1.1 see fmtlib/fmt#3574
// Formats std::atomic
template <typename T, typename Char>
struct fmt::formatter<
std::atomic<T>,
Char,
std::enable_if_t<fmt::is_formattable<T, Char>::value>>
: formatter<T, Char> {
template <typename FormatContext>
auto format(const std::atomic<T>& v, FormatContext& ctx) const
-> decltype(ctx.out()) {
return formatter<T, Char>::format(v.load(), ctx);
}
};
#endif

#if FMT_VERSION < 100100
// Backport from fmt 10.1 see fmtlib/fmt#3570
// Formats std::vector<bool>
namespace fmt::detail {
template <typename T, typename Enable = void>
struct has_flip : std::false_type {};

template <typename T>
struct has_flip<T, std::void_t<decltype(std::declval<T>().flip())>>
: std::true_type {};

template <typename T>
struct is_bit_reference_like {
static constexpr const bool value = std::is_convertible<T, bool>::value &&
std::is_nothrow_assignable<T, bool>::value && has_flip<T>::value;
};

#ifdef _LIBCPP_VERSION

// Workaround for libc++ incompatibility with C++ standard.
// According to the Standard, `bitset::operator[] const` returns bool.
template <typename C>
struct is_bit_reference_like<std::__bit_const_reference<C>> {
static constexpr const bool value = true;
};

#endif
} // namespace fmt::detail

// We can't use std::vector<bool, Allocator>::reference and
// std::bitset<N>::reference because the compiler can't deduce Allocator and N
// in partial specialization.
template <typename BitRef, typename Char>
struct fmt::formatter<
BitRef,
Char,
std::enable_if_t<fmt::detail::is_bit_reference_like<BitRef>::value>>
: formatter<bool, Char> {
template <typename FormatContext>
FMT_CONSTEXPR auto format(const BitRef& v, FormatContext& ctx) const
-> decltype(ctx.out()) {
return formatter<bool, Char>::format(v, ctx);
}
};
#endif
6 changes: 6 additions & 0 deletions velox/common/base/RuntimeMetrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,9 @@ class RuntimeStatWriterScopeGuard {
};

} // namespace facebook::velox
template <>
struct fmt::formatter<facebook::velox::RuntimeCounter::Unit> : formatter<int> {
auto format(facebook::velox::RuntimeCounter::Unit s, format_context& ctx) {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
1 change: 1 addition & 0 deletions velox/common/caching/AsyncDataCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "velox/common/caching/SsdCache.h"

#include "velox/common/base/Counters.h"
#include "velox/common/base/Exceptions.h"
#include "velox/common/base/StatsReporter.h"
#include "velox/common/base/SuccinctPrinter.h"
#include "velox/common/caching/FileIds.h"
Expand Down
1 change: 1 addition & 0 deletions velox/common/caching/SsdCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "velox/common/caching/SsdCache.h"
#include <folly/Executor.h>
#include <folly/portability/SysUio.h>
#include "velox/common/base/Exceptions.h"
#include "velox/common/caching/FileIds.h"
#include "velox/common/file/FileSystems.h"
#include "velox/common/testutil/TestValue.h"
Expand Down
3 changes: 2 additions & 1 deletion velox/common/compression/Compression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ CompressionKind codecTypeToCompressionKind(folly::io::CodecType type) {
case folly::io::CodecType::GZIP:
return CompressionKind_GZIP;
default:
VELOX_UNSUPPORTED("Not support folly codec type {}", type);
VELOX_UNSUPPORTED(
"Not support folly codec type {}", folly::to<std::string>(type));
}
}

Expand Down
Loading