[debug] Don't disable the RegExp compilation cache when debugger is active.

Disabling the RegExp compilation cache comes with performance implications,
and it doesn't seem to be necessary for debugging.

Bug: chromium:992277
Change-Id: I24841f4814bcacb18a3968c37490f201c0c1ccac
Cq-Include-Trybots: luci.chromium.try:linux-rel,win7-rel
Cq-Include-Trybots: luci.v8.try:v8_linux_gc_stress_dbg,v8_linux64_gc_stress_custom_snapshot_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1805637
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63876}

src/codegen/compilation-cache.cc

@@ -28,7 +28,7 @@ CompilationCache::CompilationCache(Isolate* isolate)
       eval_global_(isolate),
       eval_contextual_(isolate),
       reg_exp_(isolate, kRegExpGenerations),
-      enabled_(true) {
+      enabled_script_and_eval_(true) {
   CompilationSubCache* subcaches[kSubCacheCount] = {
       &script_, &eval_global_, &eval_contextual_, &reg_exp_};
   for (int i = 0; i < kSubCacheCount; ++i) {
@@ -254,7 +254,7 @@ void CompilationCacheRegExp::Put(Handle<String> source, JSRegExp::Flags flags,
 }
 
 void CompilationCache::Remove(Handle<SharedFunctionInfo> function_info) {
-  if (!IsEnabled()) return;
+  if (!IsEnabledScriptAndEval()) return;
 
   eval_global_.Remove(function_info);
   eval_contextual_.Remove(function_info);
@@ -265,7 +265,7 @@ MaybeHandle<SharedFunctionInfo> CompilationCache::LookupScript(
     Handle<String> source, MaybeHandle<Object> name, int line_offset,
     int column_offset, ScriptOriginOptions resource_options,
     Handle<Context> native_context, LanguageMode language_mode) {
-  if (!IsEnabled()) return MaybeHandle<SharedFunctionInfo>();
+  if (!IsEnabledScriptAndEval()) return MaybeHandle<SharedFunctionInfo>();
 
   return script_.Lookup(source, name, line_offset, column_offset,
                         resource_options, native_context, language_mode);
@@ -277,7 +277,7 @@ InfoCellPair CompilationCache::LookupEval(Handle<String> source,
                                           LanguageMode language_mode,
                                           int position) {
   InfoCellPair result;
-  if (!IsEnabled()) return result;
+  if (!IsEnabledScriptAndEval()) return result;
 
   const char* cache_type;
 
@@ -303,8 +303,6 @@ InfoCellPair CompilationCache::LookupEval(Handle<String> source,
 
 MaybeHandle<FixedArray> CompilationCache::LookupRegExp(Handle<String> source,
                                                        JSRegExp::Flags flags) {
-  if (!IsEnabled()) return MaybeHandle<FixedArray>();
-
   return reg_exp_.Lookup(source, flags);
 }
 
@@ -312,7 +310,7 @@ void CompilationCache::PutScript(Handle<String> source,
                                  Handle<Context> native_context,
                                  LanguageMode language_mode,
                                  Handle<SharedFunctionInfo> function_info) {
-  if (!IsEnabled()) return;
+  if (!IsEnabledScriptAndEval()) return;
   LOG(isolate(), CompilationCacheEvent("put", "script", *function_info));
 
   script_.Put(source, native_context, language_mode, function_info);
@@ -324,7 +322,7 @@ void CompilationCache::PutEval(Handle<String> source,
                                Handle<SharedFunctionInfo> function_info,
                                Handle<FeedbackCell> feedback_cell,
                                int position) {
-  if (!IsEnabled()) return;
+  if (!IsEnabledScriptAndEval()) return;
 
   const char* cache_type;
   HandleScope scope(isolate());
@@ -344,8 +342,6 @@ void CompilationCache::PutEval(Handle<String> source,
 
 void CompilationCache::PutRegExp(Handle<String> source, JSRegExp::Flags flags,
                                  Handle<FixedArray> data) {
-  if (!IsEnabled()) return;
-
   reg_exp_.Put(source, flags, data);
 }
 
@@ -367,10 +363,12 @@ void CompilationCache::MarkCompactPrologue() {
   }
 }
 
-void CompilationCache::Enable() { enabled_ = true; }
+void CompilationCache::EnableScriptAndEval() {
+  enabled_script_and_eval_ = true;
+}
 
-void CompilationCache::Disable() {
-  enabled_ = false;
+void CompilationCache::DisableScriptAndEval() {
+  enabled_script_and_eval_ = false;
   Clear();
 }
 

src/codegen/compilation-cache.h

@@ -202,9 +202,14 @@ class V8_EXPORT_PRIVATE CompilationCache {
   void MarkCompactPrologue();
 
   // Enable/disable compilation cache. Used by debugger to disable compilation
-  // cache during debugging to make sure new scripts are always compiled.
-  void Enable();
-  void Disable();
+  // cache during debugging so that eval and new scripts are always compiled.
+  // TODO(bmeurer, chromium:992277): The RegExp cache cannot be enabled and/or
+  // disabled, since it doesn't affect debugging. However ideally the other
+  // caches should also be always on, even in the presence of the debugger,
+  // but at this point there are too many unclear invariants, and so I decided
+  // to just fix the pressing performance problem for RegExp individually first.
+  void EnableScriptAndEval();
+  void DisableScriptAndEval();
 
  private:
   explicit CompilationCache(Isolate* isolate);
@@ -215,7 +220,9 @@ class V8_EXPORT_PRIVATE CompilationCache {
   // The number of sub caches covering the different types to cache.
   static const int kSubCacheCount = 4;
 
-  bool IsEnabled() const { return FLAG_compilation_cache && enabled_; }
+  bool IsEnabledScriptAndEval() const {
+    return FLAG_compilation_cache && enabled_script_and_eval_;
+  }
 
   Isolate* isolate() const { return isolate_; }
 
@@ -227,8 +234,8 @@ class V8_EXPORT_PRIVATE CompilationCache {
   CompilationCacheRegExp reg_exp_;
   CompilationSubCache* subcaches_[kSubCacheCount];
 
-  // Current enable state of the compilation cache.
-  bool enabled_;
+  // Current enable state of the compilation cache for scripts and eval.
+  bool enabled_script_and_eval_;
 
   friend class Isolate;
 

src/debug/debug.cc

@@ -1968,11 +1968,11 @@ void Debug::UpdateState() {
   if (is_active) {
     // Note that the debug context could have already been loaded to
     // bootstrap test cases.
-    isolate_->compilation_cache()->Disable();
+    isolate_->compilation_cache()->DisableScriptAndEval();
     is_active = true;
     feature_tracker()->Track(DebugFeatureTracker::kActive);
   } else {
-    isolate_->compilation_cache()->Enable();
+    isolate_->compilation_cache()->EnableScriptAndEval();
     Unload();
   }
   is_active_ = is_active;

test/cctest/test-compiler.cc

@@ -907,7 +907,7 @@ TEST(DeepEagerCompilationPeakMemory) {
       "  }"
       "}");
   v8::ScriptCompiler::Source script_source(source);
-  CcTest::i_isolate()->compilation_cache()->Disable();
+  CcTest::i_isolate()->compilation_cache()->DisableScriptAndEval();
 
   v8::HeapStatistics heap_statistics;
   CcTest::isolate()->GetHeapStatistics(&heap_statistics);

test/cctest/test-serialize.cc

@@ -1520,7 +1520,8 @@ TEST(CodeSerializerWithProfiler) {
 
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -1561,7 +1562,8 @@ TEST(CodeSerializerWithProfiler) {
 void TestCodeSerializerOnePlusOneImpl(bool verify_builtins_count = true) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -1674,7 +1676,8 @@ TEST(CodeSerializerPromotedToCompilationCache) {
 TEST(CodeSerializerInternalizedString) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -1732,7 +1735,8 @@ TEST(CodeSerializerInternalizedString) {
 TEST(CodeSerializerLargeCodeObject) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -1791,7 +1795,8 @@ TEST(CodeSerializerLargeCodeObjectWithIncrementalMarking) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
   Heap* heap = isolate->heap();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -1861,7 +1866,8 @@ TEST(CodeSerializerLargeStrings) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
   Factory* f = isolate->factory();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -1917,7 +1923,8 @@ TEST(CodeSerializerThreeBigStrings) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
   Factory* f = isolate->factory();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -2036,7 +2043,8 @@ class SerializerTwoByteResource : public v8::String::ExternalStringResource {
 TEST(CodeSerializerExternalString) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -2102,7 +2110,8 @@ TEST(CodeSerializerExternalString) {
 TEST(CodeSerializerLargeExternalString) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   Factory* f = isolate->factory();
 
@@ -2162,7 +2171,8 @@ TEST(CodeSerializerLargeExternalString) {
 TEST(CodeSerializerExternalScriptName) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   Factory* f = isolate->factory();
 
@@ -3898,7 +3908,7 @@ UNINITIALIZED_TEST(WeakArraySerializationInSnapshot) {
 TEST(WeakArraySerializationInCodeCache) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();
+  isolate->compilation_cache()->DisableScriptAndEval();
 
   v8::HandleScope scope(CcTest::isolate());
 
@@ -3927,7 +3937,8 @@ TEST(CachedCompileFunctionInContext) {
   DisableAlwaysOpt();
   LocalContext env;
   Isolate* isolate = CcTest::i_isolate();
-  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+  isolate->compilation_cache()
+      ->DisableScriptAndEval();  // Disable same-isolate code cache.
 
   v8::HandleScope scope(CcTest::isolate());