Add a script cache to the debugger

When loaded scripts are requested this cache is filled with all the script objects in the heap. Hereafter its content is kept in sync with the active scripts in the heap through the notifications of new scripts compiled and by using weak handles to get notified when a script is collected.

Through the tracking of collected scripts the debugger event OnScriptCollected have been added to notify a debugger that a script previously returned through the scripts command is no longer in use.

Make the ComputeIntegerHash globally available.

Moved clearing of the mirror cache to when debugger is really left. Previously recursive invocations of the debugger cause the mirror cache to be cleared causing handles to become either stale or reference other objects.
Review URL: http://codereview.chromium.org/115462

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1988 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8-debug.h

@@ -75,7 +75,8 @@ enum DebugEvent {
   Exception = 2,
   NewFunction = 3,
   BeforeCompile = 4,
-  AfterCompile  = 5
+  AfterCompile  = 5,
+  ScriptCollected = 6
 };
 
 

src/debug-delay.js

@@ -43,7 +43,8 @@ Debug.DebugEvent = { Break: 1,
                      Exception: 2,
                      NewFunction: 3,
                      BeforeCompile: 4,
-                     AfterCompile: 5 };
+                     AfterCompile: 5,
+                     ScriptCollected: 6 };
 
 // Types of exceptions that can be broken upon.
 Debug.ExceptionBreak = { All : 0,
@@ -1015,6 +1016,37 @@ NewFunctionEvent.prototype.setBreakPoint = function(p) {
 };
 
 
+function MakeScriptCollectedEvent(exec_state, id) {
+  return new ScriptCollectedEvent(exec_state, id);
+}
+
+
+function ScriptCollectedEvent(exec_state, id) {
+  this.exec_state_ = exec_state;
+  this.id_ = id;
+}
+
+
+ScriptCollectedEvent.prototype.id = function() {
+  return this.id_;
+};
+
+
+ScriptCollectedEvent.prototype.executionState = function() {
+  return this.exec_state_;
+};
+
+
+ScriptCollectedEvent.prototype.toJSONProtocol = function() {
+  var o = new ProtocolMessage();
+  o.running = true;
+  o.event = "scriptCollected";
+  o.body = {};
+  o.body.script = { id: this.id() };
+  return o.toJSONProtocol();
+}
+
+
 function MakeScriptObject_(script, include_source) {
   var o = { id: script.id(),
             name: script.name(),

src/debug.h

@@ -33,6 +33,7 @@
 #include "debug-agent.h"
 #include "execution.h"
 #include "factory.h"
+#include "hashmap.h"
 #include "platform.h"
 #include "string-stream.h"
 #include "v8threads.h"
@@ -144,6 +145,42 @@ class BreakLocationIterator {
 };
 
 
+// Cache of all script objects in the heap. When a script is added a weak handle
+// to it is created and that weak handle is stored in the cache. The weak handle
+// callback takes care of removing the script from the cache. The key used in
+// the cache is the script id.
+class ScriptCache : private HashMap {
+ public:
+  ScriptCache() : HashMap(ScriptMatch), collected_scripts_(10) {}
+  virtual ~ScriptCache() { Clear(); }
+
+  // Add script to the cache.
+  void Add(Handle<Script> script);
+
+  // Return the scripts in the cache.
+  Handle<FixedArray> GetScripts();
+
+  // Generate debugger events for collected scripts.
+  void ProcessCollectedScripts();
+
+ private:
+  // Calculate the hash value from the key (script id).
+  static uint32_t Hash(int key) { return ComputeIntegerHash(key); }
+
+  // Scripts match if their keys (script id) match.
+  static bool ScriptMatch(void* key1, void* key2) { return key1 == key2; }
+
+  // Clear the cache releasing all the weak handles.
+  void Clear();
+
+  // Weak handle callback for scripts in the cache.
+  static void HandleWeakScript(v8::Persistent<v8::Value> obj, void* data);
+
+  // List used during GC to temporarily store id's of collected scripts.
+  List<int> collected_scripts_;
+};
+
+
 // Linked list holding debug info objects. The debug info objects are kept as
 // weak handles to avoid a debug info object to keep a function alive.
 class DebugInfoListNode {
@@ -230,9 +267,6 @@ class Debug {
   }
   static int break_id() { return thread_local_.break_id_; }
 
-
-
-
   static bool StepInActive() { return thread_local_.step_into_fp_ != 0; }
   static void HandleStepIn(Handle<JSFunction> function,
                            Address fp,
@@ -307,6 +341,15 @@ class Debug {
   // Mirror cache handling.
   static void ClearMirrorCache();
 
+  // Script cache handling.
+  static void CreateScriptCache();
+  static void DestroyScriptCache();
+  static void AddScriptToScriptCache(Handle<Script> script);
+  static Handle<FixedArray> GetLoadedScripts();
+
+  // Garbage collection notifications.
+  static void AfterGarbageCollection();
+
   // Code generation assumptions.
   static const int kIa32CallInstructionLength = 5;
   static const int kIa32JSReturnSequenceLength = 6;
@@ -343,6 +386,11 @@ class Debug {
 
   // Boolean state indicating whether any break points are set.
   static bool has_break_points_;
+
+  // Cache of all scripts in the heap.
+  static ScriptCache* script_cache_;
+
+  // List of active debug info objects.
   static DebugInfoListNode* debug_info_list_;
 
   static bool disable_break_;
@@ -532,12 +580,15 @@ class Debugger {
   static Handle<Object> MakeCompileEvent(Handle<Script> script,
                                          bool before,
                                          bool* caught_exception);
+  static Handle<Object> MakeScriptCollectedEvent(int id,
+                                                 bool* caught_exception);
   static void OnDebugBreak(Handle<Object> break_points_hit, bool auto_continue);
   static void OnException(Handle<Object> exception, bool uncaught);
   static void OnBeforeCompile(Handle<Script> script);
   static void OnAfterCompile(Handle<Script> script,
                            Handle<JSFunction> fun);
   static void OnNewFunction(Handle<JSFunction> fun);
+  static void OnScriptCollected(int id);
   static void ProcessDebugEvent(v8::DebugEvent event,
                                 Handle<JSObject> event_data,
                                 bool auto_continue);
@@ -670,8 +721,15 @@ class EnterDebugger BASE_EMBEDDED {
       StackGuard::DebugCommand();
     }
 
-    // If leaving the debugger with the debugger no longer active unload it.
     if (prev_ == NULL) {
+      // Clear mirror cache when leaving the debugger. Skip this if there is a
+      // pending exception as clearing the mirror cache calls back into
+      // JavaScript. This can happen if the v8::Debug::Call is used in which
+      // case the exception should end up in the calling code.
+      if (!Top::has_pending_exception()) {
+        Debug::ClearMirrorCache();
+      }
+      // If leaving the debugger with the debugger no longer active unload it.
       if (!Debugger::IsDebuggerActive()) {
         Debugger::UnloadDebugger();
       }

src/heap.cc

@@ -283,6 +283,9 @@ void Heap::GarbageCollectionEpilogue() {
 #if defined(DEBUG) || defined(ENABLE_LOGGING_AND_PROFILING)
   ReportStatisticsAfterGC();
 #endif
+#ifdef ENABLE_DEBUGGER_SUPPORT
+  Debug::AfterGarbageCollection();
+#endif
 }
 
 

src/objects-debug.cc

@@ -941,6 +941,8 @@ void Script::ScriptPrint() {
   column_offset()->ShortPrint();
   PrintF("\n - type: ");
   type()->ShortPrint();
+  PrintF("\n - id: ");
+  id()->ShortPrint();
   PrintF("\n");
 }
 

src/objects.cc

@@ -5936,20 +5936,6 @@ int JSObject::GetEnumElementKeys(FixedArray* storage) {
 }
 
 
-// Thomas Wang, Integer Hash Functions.
-// http://www.concentric.net/~Ttwang/tech/inthash.htm
-static uint32_t ComputeIntegerHash(uint32_t key) {
-  uint32_t hash = key;
-  hash = ~hash + (hash << 15);  // hash = (hash << 15) - hash - 1;
-  hash = hash ^ (hash >> 12);
-  hash = hash + (hash << 2);
-  hash = hash ^ (hash >> 4);
-  hash = hash * 2057;  // hash = (hash + (hash << 3)) + (hash << 11);
-  hash = hash ^ (hash >> 16);
-  return hash;
-}
-
-
 // The NumberKey uses carries the uint32_t as key.
 // This avoids allocation in HasProperty.
 class NumberKey : public HashTableKey {

src/runtime.cc

@@ -6638,67 +6638,15 @@ static Object* Runtime_DebugEvaluateGlobal(Arguments args) {
 }
 
 
-// If an object given is an external string, check that the underlying
-// resource is accessible. For other kinds of objects, always return true.
-static bool IsExternalStringValid(Object* str) {
-  if (!str->IsString() || !StringShape(String::cast(str)).IsExternal()) {
-    return true;
-  }
-  if (String::cast(str)->IsAsciiRepresentation()) {
-    return ExternalAsciiString::cast(str)->resource() != NULL;
-  } else if (String::cast(str)->IsTwoByteRepresentation()) {
-    return ExternalTwoByteString::cast(str)->resource() != NULL;
-  } else {
-    return true;
-  }
-}
-
-
-// Helper function used by Runtime_DebugGetLoadedScripts below.
-static int DebugGetLoadedScripts(FixedArray* instances, int instances_size) {
-  NoHandleAllocation ha;
-  AssertNoAllocation no_alloc;
-
-  // Scan heap for Script objects.
-  int count = 0;
-  HeapIterator iterator;
-  while (iterator.has_next()) {
-    HeapObject* obj = iterator.next();
-    ASSERT(obj != NULL);
-    if (obj->IsScript() && IsExternalStringValid(Script::cast(obj)->source())) {
-      if (instances != NULL && count < instances_size) {
-        instances->set(count, obj);
-      }
-      count++;
-    }
-  }
-
-  return count;
-}
-
-
 static Object* Runtime_DebugGetLoadedScripts(Arguments args) {
   HandleScope scope;
   ASSERT(args.length() == 0);
 
-  // Perform two GCs to get rid of all unreferenced scripts. The first GC gets
-  // rid of all the cached script wrappers and the second gets rid of the
-  // scripts which is no longer referenced.
-  Heap::CollectAllGarbage();
-  Heap::CollectAllGarbage();
-
-  // Get the number of scripts.
-  int count;
-  count = DebugGetLoadedScripts(NULL, 0);
-
-  // Allocate an array to hold the result.
-  Handle<FixedArray> instances = Factory::NewFixedArray(count);
-
   // Fill the script objects.
-  count = DebugGetLoadedScripts(*instances, count);
+  Handle<FixedArray> instances = Debug::GetLoadedScripts();
 
   // Convert the script objects to proper JS objects.
-  for (int i = 0; i < count; i++) {
+  for (int i = 0; i < instances->length(); i++) {
     Handle<Script> script = Handle<Script>(Script::cast(instances->get(i)));
     // Get the script wrapper in a local handle before calling GetScriptWrapper,
     // because using

src/utils.cc

@@ -86,6 +86,20 @@ byte* EncodeUnsignedIntBackward(byte* p, unsigned int x) {
 }
 
 
+// Thomas Wang, Integer Hash Functions.
+// http://www.concentric.net/~Ttwang/tech/inthash.htm
+uint32_t ComputeIntegerHash(uint32_t key) {
+  uint32_t hash = key;
+  hash = ~hash + (hash << 15);  // hash = (hash << 15) - hash - 1;
+  hash = hash ^ (hash >> 12);
+  hash = hash + (hash << 2);
+  hash = hash ^ (hash >> 4);
+  hash = hash * 2057;  // hash = (hash + (hash << 3)) + (hash << 11);
+  hash = hash ^ (hash >> 16);
+  return hash;
+}
+
+
 void PrintF(const char* format, ...) {
   va_list arguments;
   va_start(arguments, format);

src/utils.h

@@ -204,6 +204,12 @@ inline byte* DecodeUnsignedIntBackward(byte* p, unsigned int* x) {
 }
 
 
+// ----------------------------------------------------------------------------
+// Hash function.
+
+uint32_t ComputeIntegerHash(uint32_t key);
+
+
 // ----------------------------------------------------------------------------
 // I/O support.
 

test/cctest/test-debug.cc

@@ -4531,6 +4531,8 @@ class EmptyExternalStringResource : public v8::String::ExternalStringResource {
 TEST(DebugGetLoadedScripts) {
   v8::HandleScope scope;
   DebugLocalContext env;
+  env.ExposeDebug();
+
   EmptyExternalStringResource source_ext_str;
   v8::Local<v8::String> source = v8::String::NewExternal(&source_ext_str);
   v8::Handle<v8::Script> evil_script = v8::Script::Compile(source);
@@ -4544,11 +4546,15 @@ TEST(DebugGetLoadedScripts) {
   i::FLAG_allow_natives_syntax = true;
   CompileRun(
       "var scripts = %DebugGetLoadedScripts();"
-      "for (var i = 0; i < scripts.length; ++i) {"
-      "    scripts[i].line_ends;"
+      "var count = scripts.length;"
+      "for (var i = 0; i < count; ++i) {"
+      "  scripts[i].line_ends;"
       "}");
   // Must not crash while accessing line_ends.
   i::FLAG_allow_natives_syntax = allow_natives_syntax;
+
+  // Some scripts are retrieved - at least the number of native scripts.
+  CHECK_GT((*env)->Global()->Get(v8::String::New("count"))->Int32Value(), 8);
 }
 
 
@@ -4686,3 +4692,48 @@ TEST(ContextData) {
   // Two times compile event and two times break event.
   CHECK_GT(message_handler_hit_count, 4);
 }
+
+
+// Debug event listener which counts the script collected events.
+int script_collected_count = 0;
+static void DebugEventScriptCollectedEvent(v8::DebugEvent event,
+                                           v8::Handle<v8::Object> exec_state,
+                                           v8::Handle<v8::Object> event_data,
+                                           v8::Handle<v8::Value> data) {
+  // Count the number of breaks.
+  if (event == v8::ScriptCollected) {
+    script_collected_count++;
+  }
+}
+
+
+// Test that scripts collected are reported through the debug event listener.
+TEST(ScriptCollectedEvent) {
+  break_point_hit_count = 0;
+  v8::HandleScope scope;
+  DebugLocalContext env;
+
+  // Request the loaded scripte to initialize the debugger script cache.
+  Debug::GetLoadedScripts();
+
+  // Do garbage collection to ensure that only the script in this test will be
+  // collected afterwards.
+  Heap::CollectAllGarbage();
+
+  script_collected_count = 0;
+  v8::Debug::SetDebugEventListener(DebugEventScriptCollectedEvent,
+                                   v8::Undefined());
+  {
+    v8::Script::Compile(v8::String::New("eval('a=1')"))->Run();
+    v8::Script::Compile(v8::String::New("eval('a=2')"))->Run();
+  }
+
+  // Do garbage collection to collect the script above which is no longer
+  // referenced.
+  Heap::CollectAllGarbage();
+
+  CHECK_EQ(2, script_collected_count);
+
+  v8::Debug::SetDebugEventListener(NULL);
+  CheckDebuggerUnloaded();
+}

test/mjsunit/debug-references.js

@@ -113,6 +113,6 @@ q = new Point(1,2);
 // Enter debugger causing the event listener to be called.
 debugger;
 
-// Make sure that the debug event listener vas invoked.
+// Make sure that the debug event listener was invoked.
 assertFalse(exception, "exception in listener")
 assertTrue(listenerComplete, "listener did not run to completion");