Use pcre2 regex & enable auto async#6991
Conversation
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
|
augment reviw |
|
augment review |
🤖 Augment PR SummarySummary: This PR replaces Lua script async-call detection from Changes:
Technical Notes: PCRE2 is built statically with tests/grep disabled and JIT support enabled. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| sregex_iterator it{body.begin(), body.end(), kRegex}; | ||
| sregex_iterator end{}; | ||
| pcre2_match_data* match_data = pcre2_match_data_create_from_pattern(kRe, nullptr); |
There was a problem hiding this comment.
src/core/interpreter.cc:850 — pcre2_match_data_create_from_pattern(kRe, nullptr) can return nullptr (e.g., allocation failure), but match_data is used unconditionally in pcre2_match, which could crash. Consider handling a null match_data before entering the match loop.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| cxx_link(dash_bench dfly_core redis_test_lib) | ||
|
|
||
| helio_cxx_test(dfly_core_test dfly_core TRDP::fast_float ${PCRE2_LIB} ${RE2_LIB} LABELS DFLY) | ||
| helio_cxx_test(dfly_core_test dfly_core TRDP::fast_float TRDP::pcre2 ${RE2_LIB} LABELS DFLY) |
There was a problem hiding this comment.
src/core/CMakeLists.txt:34 — This file still has an if(LIB_PCRE2) block (later in the file) that sets USE_PCRE2, but LIB_PCRE2 is no longer discovered here, so that branch will never run and PCRE2-gated code/benchmarks in dfly_core_test will stay disabled unintentionally.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| static pcre2_code* kRe = [] { | ||
| int errnum; | ||
| PCRE2_SIZE erroffset; | ||
| // PCRE2_MULTILINE makes ^ match at line starts, PCRE2_DOTALL makes . match \n |
There was a problem hiding this comment.
src/core/interpreter.cc:819 — The comment mentions PCRE2_DOTALL but the compile flags only pass PCRE2_MULTILINE, which makes the comment misleading about what options are actually enabled.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Pull request overview
This PR switches Lua script “auto async” detection from std::regex to PCRE2 and updates the build to always provide/link PCRE2.
Changes:
- Add PCRE2 as a bundled third-party dependency (static build, JIT enabled).
- Replace
std::regex-based matching inInterpreter::DetectPossibleAsyncCallswith PCRE2 matching to avoid fiber stack overflows. - Update core build/test linking to use
TRDP::pcre2instead of a systemfind_libraryresult.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/external_libs.cmake |
Adds PCRE2 as a third-party dependency to ensure availability. |
src/core/interpreter.cc |
Migrates async-call detection to PCRE2 to reduce stack usage on fibers. |
src/core/CMakeLists.txt |
Removes system PCRE2 detection and links against the bundled TRDP::pcre2. |
| find_library(LIB_RE2 NAMES re2) | ||
| if(LIB_RE2) | ||
| set(RE2_LIB ${LIB_RE2}) |
There was a problem hiding this comment.
LIB_PCRE2 is no longer set (the find_library block was removed), but it’s still used later for if(LIB_PCRE2) to enable USE_PCRE2. As a result the define will never be set; switch the condition to something that’s actually defined now (e.g. if(TARGET TRDP::pcre2)), or just set USE_PCRE2 unconditionally since TRDP::pcre2 is always linked.
No description provided.