airborne12 opened a new pull request, #70267:
URL: https://github.com/apache/doris/pull/70267
> Stacked PR in the SNII / text analysis / ANN CLucene removal series (index
at the end). It depends on #70266 (T7), whose commits show in this diff until
they merge; review only this PR's own commits: `ec41d65cd75` [improvement](be)
Cut the cost around ICU's break iterator in the icu tokenizer. It stays a draft
until its dependencies merge.
### What problem does this PR solve?
Issue Number: None
Related PR: #70266
Problem Summary:
The icu tokenizer asked ICU for the script of every character
and whether it is a combining mark or an emoji, allocated a UText per script
run, converted every value through UnicodeString::fromUTF8 and every term
back
through UnicodeString and std::string, and the builtin icu parser lowercased
every token again in LowerCaseFilter. RuleBasedBreakIterator, which holds the
rules, does not change; the cost around it does.
- icu_char_table.{h,cpp}: tables built once per process for the BMP: an int16
script per code point and combining-mark, emoji and emoji-RK bitmaps, about
152 KB; supplementary code points still ask ICU.
- script_iterator: reads the table, runs its loop on locals with a test of
whether a character joins the run that does not branch on each condition,
and skips ASCII eight units at a time inside a Latin run, which is exact
because every ASCII character is Latin or Common (a test checks it).
- break_iterator_wrapper and composite_break_iterator: the emoji test from
the
table, the previous boundary tracked instead of asked of ICU, and one UText
owned by the composite iterator and reopened for each run.
- icu_tokenizer: a native batch tokenizer. An ASCII value is widened to
UTF-16
directly, a valid non-ASCII value converted by simdutf's validating
converter, an invalid one by u_strFromUTF8WithSub with U+FFFD, as
UnicodeString::fromUTF8 did; strict mode throws where it threw. The terms
of
an ASCII value are views of it, the others are encoded straight into the
batch, still cut at 255 UTF-16 units without splitting a surrogate pair.
The builtin parser lowercases in the tokenizer: an ASCII term byte by byte,
a term of U+3000 to U+9FFF only (no case there) as it is, any other with
ICU's root-locale case mapping, which is what LowerCaseFilter did; the
custom icu_tokenizer does not lowercase. The combine_cj option, always on,
is gone: the table builds the Han, Hiragana and Katakana grouping in.
- analyzer.cpp: the icu preset uses the lowercasing tokenizer and no
LowerCaseFilter, as the basic preset does; tokenizer_chain() has no user
left and is removed.
ICU's break iterator keeps some state from one value to the next: after a
Common-script kana mark with no dictionary, such as U+309B, a later U+30FC
U+0301 in the same stream comes out as one token instead of two. The old code
does the same, and this change keeps it, so a reused stream still matches the
old reused stream token for token.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
Details:
- BE UT under ASAN: the analysis, index and search suites with the T5 to
T8 ones, 4203 of 4210 tests pass and the 7 skipped are the opt-in ones;
the code point scan passes with ANALYSIS_GOLDEN_FULL=1. The golden
files
freeze the icu outputs, supplementary characters around the 255th
UTF-16
unit and invalid UTF-8 included. New: ICUCharTableTest (every BMP entry
and every code point's lookup against the old ICU calls, and that every
ASCII character is Latin or Common) and ICUTokenizerStreamTest (the
builtin parser against the custom tokenizer followed by LowerCaseFilter
on 6,000 random values, lenient and strict; each lowercase path; no
case
mapping in U+3000 to U+9FFF; a Turkish default locale gives the root
output; one stream reused across scripts gives what new streams give;
strict mode throws; 255-unit cuts with surrogate pairs; the buffers
released after a long value).
- The analysis golden files are unchanged, and the benchmark's digest
mode
gives the same tokens as the T1 baseline binary in all 5,240,060 rows,
icu among them.
- The new code was also compared with the old tokenizer and
LowerCaseFilter
directly, outside the BE build: 7,285,055 values per mode (builtin and
custom, lenient and strict): every code point alone and in context,
30,132 values around the 255th unit, 300k random values with invalid
bytes and every row of the six corpora, through kept streams and with
242,287 new-stream checks per mode. 0 differences, the same exceptions
in strict mode, and none under a Turkish default locale, in a release
and in an ASAN/UBSan build.
- Throughput with be/benchmark/analyzer_ab.py, on a host loaded far above
its usual level (load 118 to 246), so the ranges are wide. Against the
T1 baseline, reuse/ and row/: a first run of 5 rounds gave icu 2.06 and
2.01, the four mixed corpora 2.07 to 2.77, zh_wiki 1.48 and 1.51 and
ja_wiki 1.68 and 1.48; a second run of 9 rounds at load 220 gave
zh_wiki
1.41 and 1.32 and ja_wiki 1.36 and 1.46. Against T7: zh_wiki 1.32 and
1.34, ja_wiki 1.30 and 1.31, which the standalone harness confirms at a
lower load (zh_wiki 1.34, ja_wiki 1.30, the four mixed corpora 2.11 in
geometric mean). With T4's 1.14 and 1.12 over the T1 baseline, that
puts
zh_wiki at about 1.5 and ja_wiki at about 1.47 against it. ICU's break
iterator and its CJK dictionary now take about 88% of the time on
Chinese and Japanese text, so 1.5 on each is out of reach without
changing ICU's dictionary breaking, which this change keeps.
- check-build-hygiene.sh passes; clang-format 16 leaves the changed files
as they are; run-clang-tidy.sh reports nothing on the changed lines.
- Behavior changed:
- [x] No.
- [ ] Yes.
Every token of the icu parser and the custom icu_tokenizer is the
same as before (above).
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
