chore: use stringmatchlen in GlobMatcher (#5547)

GlobMatcher constructor is extremely slow sometimes as it requires to compile the regex produced from the glob expression. A simple example is the small glob tested in the benchmark: a*a*a*.pt which takes roughly 70ms to compile, diminishing any of the performance gains produced by the fast matching.

* disable reflex
* fall back to stringmatch for GlobMatcher

Signed-off-by: kostas <[email protected]>

Author: kostasrim

src/core/dfly_core_test.cc

@@ -660,4 +660,11 @@ BENCHMARK(BM_MatchPcre2JitExp);
 
 #endif
 
+static void BM_MatchGlobSlow(benchmark::State& state) {
+  GlobMatcher matcher("a*a*a*a*a*.pt", false);
+  while (state.KeepRunning()) {
+    DoNotOptimize(GlobMatcher("a*a*a*a*a*.pt", false));
+  }
+}
+BENCHMARK(BM_MatchGlobSlow);
 }  // namespace dfly

src/core/glob_matcher.cc

@@ -251,7 +251,7 @@ GlobMatcher::GlobMatcher(string_view pattern, bool case_sensitive)
     regex.append(Glob2Regex(pattern));
   }
   matcher_.pattern(regex);
-#elif USE_PCRE2
+#elif defined(USE_PCRE2)
   string regex("(?s");  // dotall mode
   if (!case_sensitive) {
     regex.push_back('i');
@@ -300,7 +300,7 @@ bool GlobMatcher::Matches(std::string_view str) const {
   }
 
   return true;
-#elif USE_PCRE2
+#elif defined(USE_PCRE2)
   if (!re_ || str.size() < 16) {
     return stringmatchlen(glob_.data(), glob_.size(), str.data(), str.size(), !case_sensitive_);
   }
@@ -319,7 +319,7 @@ bool GlobMatcher::Matches(std::string_view str) const {
 
 GlobMatcher::~GlobMatcher() {
 #ifdef REFLEX_PERFORMANCE
-#elif USE_PCRE2
+#elif defined(USE_PCRE2)
   if (re_) {
     pcre2_code_free(re_);
     pcre2_match_data_free(match_data_);

src/core/glob_matcher.h

@@ -10,16 +10,19 @@
 
 // We opt for using Reflex library for glob matching.
 // While I find PCRE2 faster, it's not substantially faster to justify the shared lib dependency.
-#define REFLEX_PERFORMANCE
+
+// For some regex, Reflex (and pcre2) have extremely slow compile times(70+ms).
+// This latency is significant for the hot path and therefore both are disabled
+// and we fall back to the plain old stringmatchlen. For more info, refer to #5547 on gh.
+//#define REFLEX_PERFORMANCE
 
 #ifndef REFLEX_PERFORMANCE
 #ifdef USE_PCRE2
-  #define PCRE2_CODE_UNIT_WIDTH 8
-  #include <pcre2.h>
+#define PCRE2_CODE_UNIT_WIDTH 8
+#include <pcre2.h>
 #endif
 #endif
 
-
 namespace dfly {
 
 class GlobMatcher {
@@ -36,21 +39,21 @@ class GlobMatcher {
   static std::string Glob2Regex(std::string_view glob);
 
  private:
- // TODO: we fix the problem of stringmatchlen being much
- // faster when the result is immediately known to be false, for example: "a*" vs "bxxxxx".
- // The goal is to demonstrate on-par performance for the following case:
- // > debug populate 5000000 keys 32 RAND
- // > while true; do time valkey-cli scan 0 match 'foo*bar'; done
- // Also demonstrate that the "improved" performance via SCAN command and not only via
- // micro-benchmark.
- // The performance of naive algorithm becomes worse in cases where string is long enough,
- // and the pattern has a star at the start (or it matches at first).
+  // TODO: we fix the problem of stringmatchlen being much
+  // faster when the result is immediately known to be false, for example: "a*" vs "bxxxxx".
+  // The goal is to demonstrate on-par performance for the following case:
+  // > debug populate 5000000 keys 32 RAND
+  // > while true; do time valkey-cli scan 0 match 'foo*bar'; done
+  // Also demonstrate that the "improved" performance via SCAN command and not only via
+  // micro-benchmark.
+  // The performance of naive algorithm becomes worse in cases where string is long enough,
+  // and the pattern has a star at the start (or it matches at first).
 #ifdef REFLEX_PERFORMANCE
   mutable reflex::Matcher matcher_;
 
   bool starts_with_star_ = false;
   bool ends_with_star_ = false;
-#elif USE_PCRE2
+#elif defined(USE_PCRE2)
   pcre2_code_8* re_ = nullptr;
   pcre2_match_data_8* match_data_ = nullptr;
 #endif

tests/dragonfly/set_test.py

@@ -0,0 +1,25 @@
+import pytest
+from redis import asyncio as aioredis
+from .instance import DflyInstance, DflyInstanceFactory
+import logging
+import asyncio
+
+
+@pytest.mark.asyncio
+async def test_sscan_regression(df_factory: DflyInstanceFactory):
+    df = df_factory.create(
+        proactor_threads=2,
+    )
+    df.start()
+
+    client = df.client()
+
+    await client.execute_command(f"SADD key el1 el2")
+
+    element = "a*" * 3
+
+    cursor = await client.execute_command(f"SSCAN key 0 match {element}.pt")
+    length = len(cursor[1])
+    # Takes 3 seconds
+    res = await client.execute_command("SLOWLOG GET 100")
+    assert res == []