HappenLee opened a new pull request, #69688:
URL: https://github.com/apache/doris/pull/69688

   ### What problem does this PR solve?
   
   Issue Number: None
   
   Related PR: https://github.com/StarRocks/starrocks/pull/81384
   
   Problem Summary:
   
   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 standalone tests cover control 
masks, unaligned reads, special slots, conversion bounds, map operations, 
rehash and dump/load. A benchmark runner compares verified dependency sources 
and retains the existing int128 patch on **both** sides so that future runs 
isolate NEON scanning on the current master patch stack.
   
   ### 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; the updated runner is ready for that comparison.
   
   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. Downloader and benchmark runner produce byte-identical patched headers.
   - clang-format 16, repository clang-tidy entry point for both standalone C++ 
files, Python compilation, shell syntax and `git diff --check` passed.
   
   Full Doris BE compilation, SQL regression/performance tests and new ARM 
measurements against the combined int128+NEON patch stack have not been run.
   
   To reproduce on ARM, use an unused work directory and an available isolated 
CPU:
   
   ```bash
   bash thirdparty/download-thirdparty.sh parallel_hashmap
   python3 thirdparty/benchmarks/run_parallel_hashmap_benchmark.py   --work-dir 
/tmp/phmap-neon-run --cxx clang++   --rounds 9 --seconds 0.2 --cpu 15
   ```
   
   Add `--tests-only` for correctness and sanitizer checks without timing runs.
   
   ### Release note
   
   Accelerate phmap control-byte scanning on little-endian AArch64 while 
preserving eight-byte ARM groups and the portable fallback.
   
   ### Check List (For Author)
   
   - Test:
       - [x] Unit Test: standalone dependency correctness tests and ASAN/UBSAN 
on x86_64.
       - [x] Manual test: bidirectional dump/load, downloader idempotency, 
benchmark smoke checks and static/style checks listed above.
       - [ ] Regression test: full Doris SQL regression was not run.
   - Behavior changed:
       - [x] No. SQL semantics and the existing ARM group/dump layout are 
unchanged.
   - Does this need documentation?
       - [x] No. Internal dependency optimization; reproduction steps are 
included above.
   
   ### Check List (For Reviewer who merge this PR)
   
   - [ ] Confirm the release note
   - [ ] Confirm test cases and ARM performance on the current patch stack
   - [ ] Confirm document
   - [ ] Add branch pick label
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to