mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
AArch64 currently uses portable scalar phmap control-byte scans. The subtraction-based H2 mask can include adjacent false-positive candidates: control bytes `0x12, 0x13` can both be returned when looking for `0x12`. Key comparison preserves correctness, but the extra candidates cost work. Add exact NEON control comparisons on little-endian AArch64 while keeping eight-byte groups, the portable fallback and the existing ARM dump/load layout. x86 keeps its existing SSE implementation. `PHMAP_HAVE_ARM_NEON=0` disables the new implementation at compile time. The normal third-party downloader applies this patch alongside the existing AArch64 int128 hash-mixer patch, with separate markers and a subshell that restores the caller's working directory. The NEON patch now includes context so a replay cannot silently insert the same code again. The downloader checks the actual source with a direction-locked reverse dry-run, recovers a missing marker when all hunks are already applied, and preflights every hunk before a fresh application. Partial or duplicated sources fail without further edits and do not retain a success marker. The standalone tests cover control masks, unaligned reads, special slots, conversion bounds, map operations, rehash and dump/load. The standalone C++ benchmark can be compiled directly. For an incremental comparison, retain the existing int128 patch in both the baseline and optimized dependency sources, adding NEON only to the optimized copy. ### Supplied ARM measurements The following measurements come from the supplied report on original Doris baseline `70f7dcbe11c8e142b81536c1dfaaf505ec05437b`, not a new ARM run of this rebased PR. That baseline predates master's AArch64 int128 patch. These numbers therefore do **not** establish the incremental gains of the final combined patch stack; a new ARM run with the combined patch stack is still needed. Environment: Neoverse-N2, 16 cores, nominal 3 GHz; Clang 20.1.8; C++17, `-O3 -DNDEBUG -march=armv8-a+crc -pthread`; one process pinned to CPU 15. Nine alternating baseline/NEON rounds, each measurement at least 0.2 seconds of thread CPU time. Values are median thread CPU ns/op. Reported intervals use 10,000 paired bootstrap resamples. Positive time reduction means faster; negative means slower. | Scenario | Baseline ns/op | NEON ns/op | Time reduction | Reported 95% interval | |---|---:|---:|---:|---:| | Hit lookup, 4,096 keys | 4.0773 | 3.8350 | +5.94% | [+5.84%, +6.11%] | | Hit lookup, 100,000 keys | 10.6120 | 9.6412 | +9.15% | [+8.99%, +12.44%] | | Hit lookup, 114,000 keys | 11.4606 | 10.5508 | +7.94% | [+7.73%, +11.42%] | | Hit lookup, 1,000,000 keys | 16.3554 | 14.4228 | +11.82% | [+10.20%, +12.74%] | | Miss lookup, 4,096 keys | 3.4818 | 3.3069 | +5.02% | [+4.81%, +5.08%] | | Miss lookup, 100,000 keys | 9.7027 | 9.9144 | -2.18% | [-2.91%, -1.27%] | | Miss lookup, 114,000 keys | 16.8860 | 17.5147 | -3.72% | [-3.93%, -2.89%] | | Miss lookup, 1,000,000 keys | 8.6927 | 8.1035 | +6.78% | [+4.93%, +9.01%] | | Insert, 100,000 keys, including allocation/destruction | 9.8116 | 9.2758 | +5.46% | [+3.94%, +6.25%] | | Erase/reinsert, 20,000 operations per iteration | 16.3032 | 16.3810 | -0.48% | [-1.19%, +0.97%] | | Synthetic adjacent-H2 scan, 32,768 groups | 2.7578 | 1.7641 | +36.03% | [+35.75%, +36.09%] | Load factors differ: approximately 50.0%, 76.3%, 87.0% and 47.7% at the four lookup sizes. Two miss-lookup workloads regress reproducibly. The erase/reinsert interval crosses zero. The adjacent-H2 case measures group scans and candidate-key comparisons, not complete table lookups. These are library microbenchmarks, not end-to-end SQL gains. ### Validation **Reported in the supplied ARM bundle:** randomized scalar-oracle checks, baseline/NEON/forced-portable paths, bidirectional dump/load and ASAN/UBSAN passed. The archive contains the patch and report; these ARM executions were not repeated during this submission. **Executed locally after rebasing onto master `3d2d9aac54148056529c13ef4b9fbdbcb65fab77` (x86_64, Clang 16.0.6):** - Standalone correctness tests passed for baseline, patched and explicitly NEON-disabled builds; x86 selected the existing 16-byte SSE groups in all three. Tests also exercise the portable group implementation directly. - Baseline-to-patched and patched-to-baseline dump/load passed; patched ASAN/UBSAN passed. - Benchmark source compiled with `-Wall -Wextra -Werror`; all 11 scenarios passed a short checksum smoke run. No x86 performance conclusion is drawn from it. - Standard third-party download/patch script passed twice with `OLDPWD` unset. The standard downloader produces byte-identical headers to those used by the standalone tests. - clang-format 16, repository clang-tidy entry point for both standalone C++ files, shell syntax and `git diff --check` passed. **Downloader recovery follow-up:** `bash thirdparty/test/parallel-hashmap-patch-test.sh` passed using the pinned cached archive and the real downloader; no dependency downloads were needed. It covers fresh extraction, normal reruns, an applied patch with a missing marker, a stale marker over pristine sources, partially applied sources with/without a marker, and duplicate NEON definitions. Successful reruns compare both complete headers; failing cases verify unchanged source and absence of the success marker. The same test detects the original replay bug. The regenerated patch inserts exactly the same 67 C++ lines, and its applied header bytes match the original optimized source. Bash syntax and shell whitespace checks passed; generated patch context preserves the pinned dependency's existing whitespace. Full Doris BE compilation, SQL regression/performance tests and new ARM measurements against the combined int128+NEON patch stack have not been run. The C++ test and benchmark have no Python dependency and can be compiled directly: ```bash bash thirdparty/download-thirdparty.sh parallel_hashmap clang++ -std=c++17 -O2 -pthread -Wall -Wextra -Werror \ -isystem thirdparty/src/parallel-hashmap-1.3.8 \ thirdparty/tests/parallel_hashmap_neon_test.cpp -o /tmp/phmap-neon-test /tmp/phmap-neon-test clang++ -std=c++17 -O3 -DNDEBUG -pthread -Wall -Wextra -Werror \ -isystem thirdparty/src/parallel-hashmap-1.3.8 \ thirdparty/benchmarks/parallel_hashmap_bench.cpp -o /tmp/phmap-neon-bench taskset -c 15 /tmp/phmap-neon-bench 0.2 ``` Select an available isolated CPU instead of CPU 15 when necessary. For the reported ARM compiler settings, add `-march=armv8-a+crc`. The benchmark emits one JSON line per scenario with CPU/wall timings and checksums. To compare against baseline on current master, extract the verified source archive into another directory, apply only `parallel-hashmap-1.3.8-aarch64-int128.patch`, compile the same C++ benchmark against that directory with identical flags, and alternate baseline/optimized execution order. ### Release note Accelerate phmap control-byte scanning on little-endian AArch64 while preserving eight-byte ARM groups and the portable fallback.
144 lines
5.8 KiB
Bash
Executable File
144 lines
5.8 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Licensed to the Apache Software Foundation (ASF) under one
|
|
# or more contributor license agreements. See the NOTICE file
|
|
# distributed with this work for additional information
|
|
# regarding copyright ownership. The ASF licenses this file
|
|
# to you 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.
|
|
|
|
set -euo pipefail
|
|
|
|
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." &>/dev/null && pwd)"
|
|
ARCHIVE_DIR="${1:-${ROOT}/src}"
|
|
ARCHIVE_DIR="$(cd "${ARCHIVE_DIR}" && pwd)"
|
|
TP_DIR="${ROOT}"
|
|
source "${ROOT}/vars.sh"
|
|
[[ -f "${ARCHIVE_DIR}/${PARALLEL_HASHMAP_NAME}" ]] || {
|
|
echo "FAIL: provide the directory containing ${PARALLEL_HASHMAP_NAME}" >&2
|
|
exit 1
|
|
}
|
|
|
|
mkdir -p "${ROOT}/src"
|
|
tmpdir="$(mktemp -d "${ROOT}/src/phmap-patch-test.XXXXXX")"
|
|
trap 'rm -rf "${tmpdir}"' EXIT
|
|
|
|
fail() {
|
|
echo "FAIL: $*" >&2
|
|
exit 1
|
|
}
|
|
|
|
prepare() {
|
|
local dest="$1"
|
|
mkdir -p "${dest}/src"
|
|
cp "${ROOT}/vars.sh" "${dest}/vars.sh"
|
|
ln -s "${ROOT}/patches" "${dest}/patches"
|
|
cp "${ARCHIVE_DIR}/${PARALLEL_HASHMAP_NAME}" "${dest}/src/"
|
|
}
|
|
|
|
run_download() {
|
|
TP_DIR="$1" DORIS_HOME="${tmpdir}" bash "${ROOT}/download-thirdparty.sh" parallel_hashmap
|
|
}
|
|
|
|
check_sources() {
|
|
local source="$1/src/${PARALLEL_HASHMAP_SOURCE}"
|
|
[[ -f "${source}/patched_mark_aarch64_int128" ]] || fail "missing int128 marker"
|
|
[[ -f "${source}/patched_mark_arm_neon" ]] || fail "missing NEON marker"
|
|
[[ "$(grep -c '^struct GroupAArch64Impl {' "${source}/parallel_hashmap/phmap.h")" == 1 ]] \
|
|
|| fail "NEON group must appear exactly once"
|
|
cmp "${tmpdir}/expected/phmap.h" "${source}/parallel_hashmap/phmap.h"
|
|
cmp "${tmpdir}/expected/phmap_config.h" "${source}/parallel_hashmap/phmap_config.h"
|
|
}
|
|
|
|
snapshot() {
|
|
local source="$1/src/${PARALLEL_HASHMAP_SOURCE}/parallel_hashmap"
|
|
cp "${source}/phmap.h" "${tmpdir}/before-phmap.h"
|
|
cp "${source}/phmap_config.h" "${tmpdir}/before-phmap_config.h"
|
|
}
|
|
|
|
expect_failure() {
|
|
local dest="$1"
|
|
if run_download "${dest}" >"${dest}/failure.log" 2>&1; then
|
|
fail "incompatible or partial patch unexpectedly succeeded"
|
|
fi
|
|
local source="${dest}/src/${PARALLEL_HASHMAP_SOURCE}"
|
|
[[ ! -f "${source}/patched_mark_arm_neon" ]] || fail "failed patch retained its marker"
|
|
cmp "${tmpdir}/before-phmap.h" "${source}/parallel_hashmap/phmap.h"
|
|
cmp "${tmpdir}/before-phmap_config.h" "${source}/parallel_hashmap/phmap_config.h"
|
|
}
|
|
|
|
prepare "${tmpdir}/fresh"
|
|
run_download "${tmpdir}/fresh"
|
|
source_dir="${tmpdir}/fresh/src/${PARALLEL_HASHMAP_SOURCE}"
|
|
mkdir "${tmpdir}/expected"
|
|
cp "${source_dir}/parallel_hashmap/phmap.h" "${tmpdir}/expected/"
|
|
cp "${source_dir}/parallel_hashmap/phmap_config.h" "${tmpdir}/expected/"
|
|
check_sources "${tmpdir}/fresh"
|
|
touch "${source_dir}/cache_reuse_sentinel"
|
|
run_download "${tmpdir}/fresh"
|
|
check_sources "${tmpdir}/fresh"
|
|
[[ -f "${source_dir}/cache_reuse_sentinel" ]] || fail "rerun replaced cached sources"
|
|
echo "PASS: fresh sources and normal rerun"
|
|
|
|
# Model a process stopping between the completed patch and the marker write.
|
|
rm "${source_dir}/patched_mark_arm_neon"
|
|
run_download "${tmpdir}/fresh"
|
|
check_sources "${tmpdir}/fresh"
|
|
run_download "${tmpdir}/fresh"
|
|
check_sources "${tmpdir}/fresh"
|
|
echo "PASS: completed patch without a marker recovers without reapplying"
|
|
|
|
# A stale marker must not suppress applying the patch to pristine sources.
|
|
prepare "${tmpdir}/stale"
|
|
tar xzf "${ARCHIVE_DIR}/${PARALLEL_HASHMAP_NAME}" -C "${tmpdir}/stale/src"
|
|
touch "${tmpdir}/stale/src/${PARALLEL_HASHMAP_SOURCE}/patched_mark_arm_neon"
|
|
run_download "${tmpdir}/stale"
|
|
check_sources "${tmpdir}/stale"
|
|
echo "PASS: stale marker on pristine sources"
|
|
|
|
# One patched file and one unpatched file must be rejected before either changes.
|
|
# Keep the real int128 patch applied, as it precedes NEON in the downloader.
|
|
for marker in missing present; do
|
|
prepare "${tmpdir}/partial-${marker}"
|
|
tar xzf "${ARCHIVE_DIR}/${PARALLEL_HASHMAP_NAME}" -C "${tmpdir}/partial-${marker}/src"
|
|
partial_source="${tmpdir}/partial-${marker}/src/${PARALLEL_HASHMAP_SOURCE}"
|
|
(
|
|
cd "${partial_source}"
|
|
patch --batch --forward --fuzz=0 -p1 \
|
|
<"${ROOT}/patches/parallel-hashmap-1.3.8-aarch64-int128.patch"
|
|
touch patched_mark_aarch64_int128
|
|
)
|
|
cp "${tmpdir}/expected/phmap.h" "${partial_source}/parallel_hashmap/phmap.h"
|
|
if [[ "${marker}" == present ]]; then
|
|
touch "${partial_source}/patched_mark_arm_neon"
|
|
fi
|
|
snapshot "${tmpdir}/partial-${marker}"
|
|
expect_failure "${tmpdir}/partial-${marker}"
|
|
done
|
|
echo "PASS: partial sources fail without further edits or a success marker"
|
|
|
|
# Duplicate definitions reproduce the old insertion-only patch's replay result.
|
|
prepare "${tmpdir}/duplicate"
|
|
run_download "${tmpdir}/duplicate"
|
|
duplicate_source="${tmpdir}/duplicate/src/${PARALLEL_HASHMAP_SOURCE}"
|
|
awk '
|
|
/^#if PHMAP_HAVE_ARM_NEON && defined\(PHMAP_IS_LITTLE_ENDIAN\)/ && !found { copying = 1 }
|
|
copying { block = block $0 "\n" }
|
|
copying && /^#endif/ { copying = 0; found = 1 }
|
|
{ print }
|
|
found && !inserted { printf "%s", block; inserted = 1 }
|
|
' "${duplicate_source}/parallel_hashmap/phmap.h" >"${tmpdir}/duplicate-phmap.h"
|
|
mv "${tmpdir}/duplicate-phmap.h" "${duplicate_source}/parallel_hashmap/phmap.h"
|
|
snapshot "${tmpdir}/duplicate"
|
|
expect_failure "${tmpdir}/duplicate"
|
|
echo "PASS: duplicate NEON definitions are rejected despite an existing marker"
|