airborne12 commented on code in PR #70262:
URL: https://github.com/apache/doris/pull/70262#discussion_r4505027750
##########
be/src/storage/index/inverted/clucene/inverted_index_writer.cpp:
##########
@@ -200,7 +197,8 @@ Result<std::shared_ptr<lucene::analysis::Analyzer>>
InvertedIndexColumnWriter<field_type>::create_analyzer(
const InvertedIndexAnalyzerConfig& analyzer_config) {
try {
- return
inverted_index::InvertedIndexAnalyzer::create_analyzer(&analyzer_config);
+ return std::make_shared<inverted_index::LuceneAnalyzerBridge>(
Review Comment:
Agreed that this step costs the V1/V2 writer: until an analyzer is native,
`LuceneBackedTokenStream` makes a CLucene stream per value, as queries and the
SNII writer already did, where the V1/V2 writer used `reusableTokenStream()`.
Measured on that path (one stream for all rows) against the code before this
stack, tokenizer throughput at this head is 0.79–0.94 of before (geometric mean
over four corpora; 0.65 on the shortest rows). The next PRs in the stack remove
it by making the analyzers native, with streams that keep their components
across values: #70263 for custom analyzers, icu and ik (1.04–1.81 of before),
#70264 english, #70265 standard and #70266 chinese (all at or above 1.0 from
there). Reusing CLucene streams inside the adapter would need a private CLucene
analyzer per stream, since the shared one cannot hold reuse state across
threads, and the adapter is removed in #70273, so I'd keep this PR as is.
--
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]
