From c51bb67028c7ed668e5c4a37dfc90984d362c233 Mon Sep 17 00:00:00 2001
From: aravind <aravind@fb.com>
Date: Sun, 19 May 2013 19:47:22 -0700
Subject: [PATCH] Use AtomicHashArray for PCRE

---
 hphp/compiler/compiler.cpp              |  3 ++
 hphp/runtime/base/preg.cpp              | 65 ++++++++++++++++++++-----
 hphp/runtime/base/program_functions.cpp |  6 +++
 hphp/runtime/base/program_functions.h   |  2 +
 hphp/runtime/base/runtime_option.h      |  3 ++
 5 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/hphp/compiler/compiler.cpp b/hphp/compiler/compiler.cpp
index 05ad448db..d65abe385 100644
--- a/hphp/compiler/compiler.cpp
+++ b/hphp/compiler/compiler.cpp
@@ -160,6 +160,9 @@ int compiler_main(int argc, char **argv) {
     RuntimeOption::Load(empty);
     initialize_repo();
 
+    // we need to initialize pcre cache table very early
+    pcre_init();
+
     CompilerOptions po;
 #ifdef FACEBOOK
     compiler_hook_initialize();
diff --git a/hphp/runtime/base/preg.cpp b/hphp/runtime/base/preg.cpp
index 2001f8658..8603e071e 100644
--- a/hphp/runtime/base/preg.cpp
+++ b/hphp/runtime/base/preg.cpp
@@ -71,28 +71,67 @@ public:
   int compile_options;
 };
 
-typedef tbb::concurrent_hash_map<const StringData*,const pcre_cache_entry*,
-                                StringDataHashCompare> PCREStringMap;
+struct ahm_string_data_same {
+  bool operator()(const StringData* s1, const StringData* s2) {
+    // ahm uses -1, -2, -3 as magic values
+    return int64_t(s1) > 0 && s1->same(s2);
+  }
+};
+typedef folly::AtomicHashArray<const StringData*, const pcre_cache_entry*,
+                         string_data_hash, ahm_string_data_same> PCREStringMap;
+typedef std::pair<const StringData*, const pcre_cache_entry*> PCREEntry;
 
-static PCREStringMap s_pcreCacheMap;
+static PCREStringMap* s_pcreCacheMap;
+
+void pcre_init() {
+  if (!s_pcreCacheMap) {
+    PCREStringMap::Config config;
+    config.maxLoadFactor = 0.5;
+    s_pcreCacheMap = PCREStringMap::create(
+                       RuntimeOption::EvalPCRETableSize, config).release();
+  }
+}
+
+void pcre_reinit() {
+  PCREStringMap::Config config;
+  config.maxLoadFactor = 0.5;
+  PCREStringMap* newMap = PCREStringMap::create(
+                     RuntimeOption::EvalPCRETableSize, config).release();
+  if (s_pcreCacheMap) {
+    PCREStringMap::iterator it;
+    for (it = s_pcreCacheMap->begin(); it != s_pcreCacheMap->end(); it++) {
+      // there should not be a lot of entries created before runtime
+      // options were parsed.
+      delete(it->second);
+    }
+    PCREStringMap::destroy(s_pcreCacheMap);
+  }
+  s_pcreCacheMap = newMap;
+}
 
 static const pcre_cache_entry* lookup_cached_pcre(CStrRef regex) {
-  PCREStringMap::const_accessor acc;
-  if (s_pcreCacheMap.find(acc, regex.get())) {
-    return acc->second;
+  assert(s_pcreCacheMap);
+  PCREStringMap::iterator it;
+  if ((it = s_pcreCacheMap->find(regex.get())) != s_pcreCacheMap->end()) {
+    return it->second;
   }
   return 0;
 }
 
 static const pcre_cache_entry*
 insert_cached_pcre(CStrRef regex, const pcre_cache_entry* ent) {
-  PCREStringMap::accessor acc;
-  if (s_pcreCacheMap.insert(acc, StringData::GetStaticString(regex.get()))) {
-    acc->second = ent;
-    return ent;
+  assert(s_pcreCacheMap);
+  auto pair = s_pcreCacheMap->insert(
+    PCREEntry(StringData::GetStaticString(regex.get()), ent));
+  if (!pair.second) {
+    delete ent;
+    if (s_pcreCacheMap->size() < RuntimeOption::EvalPCRETableSize) {
+      return pair.first->second;
+    }
+    // if the AHA is too small, fail.
+    raise_error("PCRE cache full");
   }
-  delete ent;
-  return acc->second;
+  return ent;
 }
 
 /*
@@ -1361,7 +1400,7 @@ int preg_last_error() {
 }
 
 size_t preg_pcre_cache_size() {
-  return (size_t)s_pcreCacheMap.size();
+  return (size_t)s_pcreCacheMap->size();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/hphp/runtime/base/program_functions.cpp b/hphp/runtime/base/program_functions.cpp
index c0a00c035..3b2c33036 100644
--- a/hphp/runtime/base/program_functions.cpp
+++ b/hphp/runtime/base/program_functions.cpp
@@ -905,6 +905,9 @@ static int execute_program_impl(int argc, char **argv) {
 
   po.isTempFile = vm.count("temp-file");
 
+  // we need to initialize pcre cache table very early
+  pcre_init();
+
   Hdf config;
   if (!po.config.empty()) {
     config.open(po.config);
@@ -1190,6 +1193,9 @@ void hphp_process_init() {
   ClassInfo::Load();
   Process::InitProcessStatics();
 
+  // reinitialize pcre table
+  pcre_reinit();
+
   // the liboniguruma docs say this isnt needed,
   // but the implementation of init is not
   // thread safe due to bugs
diff --git a/hphp/runtime/base/program_functions.h b/hphp/runtime/base/program_functions.h
index 534a92a65..9ca2a94ec 100644
--- a/hphp/runtime/base/program_functions.h
+++ b/hphp/runtime/base/program_functions.h
@@ -63,6 +63,8 @@ time_t start_time();
 
 class ExecutionContext;
 
+void pcre_init();
+void pcre_reinit();
 void hphp_process_init() ATTRIBUTE_COLD;
 void hphp_session_init();
 
diff --git a/hphp/runtime/base/runtime_option.h b/hphp/runtime/base/runtime_option.h
index acc73794c..01c944230 100644
--- a/hphp/runtime/base/runtime_option.h
+++ b/hphp/runtime/base/runtime_option.h
@@ -377,6 +377,8 @@ public:
 
   static std::set<std::string, stdltistr> DynamicInvokeFunctions;
 
+  static const uint32_t kPCREInitialTableSize = 96 * 1024;
+
 #define EVALFLAGS()                                                     \
   /* F(type, name, defaultVal) */                                       \
   /*                                                                    \
@@ -447,6 +449,7 @@ public:
   F(bool, DisableSomeRepoAuthNotices,  true)                            \
   F(uint32_t, InitialNamedEntityTableSize,  30000)                      \
   F(uint32_t, InitialStaticStringTableSize, 100000)                     \
+  F(uint32_t, PCRETableSize, kPCREInitialTableSize)                     \
   /* */                                                                 \
 
 #define F(type, name, unused) \