Eliminate the need for code delete events in CPU profiler.

Events are still generated for tick processor on performance testing
server to work, as soon as scripts will be updated, it will be safe
to remove code delete events emitting code.

[email protected]
BUG=v8:1466
TEST=existing tests in test-profile-generator,test-cpu-profiler and mjsunit/tools

Review URL: http://codereview.chromium.org/7864017

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

Author: [email protected]

src/cpu-profiler-inl.h

@@ -51,11 +51,6 @@ void CodeMoveEventRecord::UpdateCodeMap(CodeMap* code_map) {
 }
 
 
-void CodeDeleteEventRecord::UpdateCodeMap(CodeMap* code_map) {
-  code_map->DeleteCode(start);
-}
-
-
 void SharedFunctionInfoMoveEventRecord::UpdateCodeMap(CodeMap* code_map) {
   code_map->MoveCode(from, to);
 }

src/cpu-profiler.cc

@@ -137,16 +137,6 @@ void ProfilerEventsProcessor::CodeMoveEvent(Address from, Address to) {
 }
 
 
-void ProfilerEventsProcessor::CodeDeleteEvent(Address from) {
-  CodeEventsContainer evt_rec;
-  CodeDeleteEventRecord* rec = &evt_rec.CodeDeleteEventRecord_;
-  rec->type = CodeEventRecord::CODE_DELETE;
-  rec->order = ++enqueue_order_;
-  rec->start = from;
-  events_buffer_.Enqueue(evt_rec);
-}
-
-
 void ProfilerEventsProcessor::SharedFunctionInfoMoveEvent(Address from,
                                                           Address to) {
   CodeEventsContainer evt_rec;
@@ -425,7 +415,6 @@ void CpuProfiler::CodeMoveEvent(Address from, Address to) {
 
 
 void CpuProfiler::CodeDeleteEvent(Address from) {
-  Isolate::Current()->cpu_profiler()->processor_->CodeDeleteEvent(from);
 }
 
 

src/cpu-profiler.h

@@ -48,7 +48,6 @@ class TokenEnumerator;
 #define CODE_EVENTS_TYPE_LIST(V)                                   \
   V(CODE_CREATION,    CodeCreateEventRecord)                       \
   V(CODE_MOVE,        CodeMoveEventRecord)                         \
-  V(CODE_DELETE,      CodeDeleteEventRecord)                       \
   V(SHARED_FUNC_MOVE, SharedFunctionInfoMoveEventRecord)
 
 
@@ -87,14 +86,6 @@ class CodeMoveEventRecord : public CodeEventRecord {
 };
 
 
-class CodeDeleteEventRecord : public CodeEventRecord {
- public:
-  Address start;
-
-  INLINE(void UpdateCodeMap(CodeMap* code_map));
-};
-
-
 class SharedFunctionInfoMoveEventRecord : public CodeEventRecord {
  public:
   Address from;

src/log.cc

@@ -1527,6 +1527,51 @@ void Logger::LogCodeObjects() {
 }
 
 
+void Logger::LogExistingFunction(Handle<SharedFunctionInfo> shared,
+                                 Handle<Code> code) {
+  Handle<String> func_name(shared->DebugName());
+  if (shared->script()->IsScript()) {
+    Handle<Script> script(Script::cast(shared->script()));
+    if (script->name()->IsString()) {
+      Handle<String> script_name(String::cast(script->name()));
+      int line_num = GetScriptLineNumber(script, shared->start_position());
+      if (line_num > 0) {
+        PROFILE(ISOLATE,
+                CodeCreateEvent(
+                    Logger::ToNativeByScript(Logger::LAZY_COMPILE_TAG, *script),
+                    *code, *shared,
+                    *script_name, line_num + 1));
+      } else {
+        // Can't distinguish eval and script here, so always use Script.
+        PROFILE(ISOLATE,
+                CodeCreateEvent(
+                    Logger::ToNativeByScript(Logger::SCRIPT_TAG, *script),
+                    *code, *shared, *script_name));
+      }
+    } else {
+      PROFILE(ISOLATE,
+              CodeCreateEvent(
+                  Logger::ToNativeByScript(Logger::LAZY_COMPILE_TAG, *script),
+                  *code, *shared, *func_name));
+    }
+  } else if (shared->IsApiFunction()) {
+    // API function.
+    FunctionTemplateInfo* fun_data = shared->get_api_func_data();
+    Object* raw_call_data = fun_data->call_code();
+    if (!raw_call_data->IsUndefined()) {
+      CallHandlerInfo* call_data = CallHandlerInfo::cast(raw_call_data);
+      Object* callback_obj = call_data->callback();
+      Address entry_point = v8::ToCData<Address>(callback_obj);
+      PROFILE(ISOLATE, CallbackEvent(*func_name, entry_point));
+    }
+  } else {
+    PROFILE(ISOLATE,
+            CodeCreateEvent(
+                Logger::LAZY_COMPILE_TAG, *code, *shared, *func_name));
+  }
+}
+
+
 void Logger::LogCompiledFunctions() {
   HandleScope scope;
   const int compiled_funcs_count = EnumerateCompiledFunctions(NULL, NULL);
@@ -1540,48 +1585,7 @@ void Logger::LogCompiledFunctions() {
     if (*code_objects[i] == Isolate::Current()->builtins()->builtin(
         Builtins::kLazyCompile))
       continue;
-    Handle<SharedFunctionInfo> shared = sfis[i];
-    Handle<String> func_name(shared->DebugName());
-    if (shared->script()->IsScript()) {
-      Handle<Script> script(Script::cast(shared->script()));
-      if (script->name()->IsString()) {
-        Handle<String> script_name(String::cast(script->name()));
-        int line_num = GetScriptLineNumber(script, shared->start_position());
-        if (line_num > 0) {
-          PROFILE(ISOLATE,
-                  CodeCreateEvent(
-                    Logger::ToNativeByScript(Logger::LAZY_COMPILE_TAG, *script),
-                    *code_objects[i], *shared,
-                    *script_name, line_num + 1));
-        } else {
-          // Can't distinguish eval and script here, so always use Script.
-          PROFILE(ISOLATE,
-                  CodeCreateEvent(
-                      Logger::ToNativeByScript(Logger::SCRIPT_TAG, *script),
-                      *code_objects[i], *shared, *script_name));
-        }
-      } else {
-        PROFILE(ISOLATE,
-                CodeCreateEvent(
-                    Logger::ToNativeByScript(Logger::LAZY_COMPILE_TAG, *script),
-                    *code_objects[i], *shared, *func_name));
-      }
-    } else if (shared->IsApiFunction()) {
-      // API function.
-      FunctionTemplateInfo* fun_data = shared->get_api_func_data();
-      Object* raw_call_data = fun_data->call_code();
-      if (!raw_call_data->IsUndefined()) {
-        CallHandlerInfo* call_data = CallHandlerInfo::cast(raw_call_data);
-        Object* callback_obj = call_data->callback();
-        Address entry_point = v8::ToCData<Address>(callback_obj);
-        PROFILE(ISOLATE, CallbackEvent(*func_name, entry_point));
-      }
-    } else {
-      PROFILE(ISOLATE,
-              CodeCreateEvent(
-                  Logger::LAZY_COMPILE_TAG, *code_objects[i],
-                  *shared, *func_name));
-    }
+    LogExistingFunction(sfis[i], code_objects[i]);
   }
 }
 

src/log.h

@@ -281,6 +281,8 @@ class Logger {
   void ResumeProfiler();
   bool IsProfilerPaused();
 
+  void LogExistingFunction(Handle<SharedFunctionInfo> shared,
+                           Handle<Code> code);
   // Logs all compiled functions found in the heap.
   void LogCompiledFunctions();
   // Logs all accessor callbacks found in the heap.

src/profile-generator-inl.h

@@ -78,22 +78,6 @@ ProfileNode::ProfileNode(ProfileTree* tree, CodeEntry* entry)
 }
 
 
-void CodeMap::AddCode(Address addr, CodeEntry* entry, unsigned size) {
-  CodeTree::Locator locator;
-  tree_.Insert(addr, &locator);
-  locator.set_value(CodeEntryInfo(entry, size));
-}
-
-
-void CodeMap::MoveCode(Address from, Address to) {
-  tree_.Move(from, to);
-}
-
-void CodeMap::DeleteCode(Address addr) {
-  tree_.Remove(addr);
-}
-
-
 CodeEntry* ProfileGenerator::EntryForVMState(StateTag tag) {
   switch (tag) {
     case GC:

src/profile-generator.cc

@@ -492,6 +492,28 @@ const CodeMap::CodeTreeConfig::Value CodeMap::CodeTreeConfig::kNoValue =
     CodeMap::CodeEntryInfo(NULL, 0);
 
 
+void CodeMap::AddCode(Address addr, CodeEntry* entry, unsigned size) {
+  DeleteAllCoveredCode(addr, addr + size);
+  CodeTree::Locator locator;
+  tree_.Insert(addr, &locator);
+  locator.set_value(CodeEntryInfo(entry, size));
+}
+
+
+void CodeMap::DeleteAllCoveredCode(Address start, Address end) {
+  List<Address> to_delete;
+  Address addr = end - 1;
+  while (addr >= start) {
+    CodeTree::Locator locator;
+    if (!tree_.FindGreatestLessThan(addr, &locator)) break;
+    Address start2 = locator.key(), end2 = start2 + locator.value().size;
+    if (start2 < end && start < end2) to_delete.Add(start2);
+    addr = start2 - 1;
+  }
+  for (int i = 0; i < to_delete.length(); ++i) tree_.Remove(to_delete[i]);
+}
+
+
 CodeEntry* CodeMap::FindEntry(Address addr) {
   CodeTree::Locator locator;
   if (tree_.FindGreatestLessThan(addr, &locator)) {
@@ -520,6 +542,16 @@ int CodeMap::GetSharedId(Address addr) {
 }
 
 
+void CodeMap::MoveCode(Address from, Address to) {
+  if (from == to) return;
+  CodeTree::Locator locator;
+  if (!tree_.Find(from, &locator)) return;
+  CodeEntryInfo entry = locator.value();
+  tree_.Remove(from);
+  AddCode(to, entry.entry, entry.size);
+}
+
+
 void CodeMap::CodeTreePrinter::Call(
     const Address& key, const CodeMap::CodeEntryInfo& value) {
   OS::Print("%p %5d %s\n", key, value.size, value.entry->name());

src/profile-generator.h

@@ -238,9 +238,8 @@ class CpuProfile {
 class CodeMap {
  public:
   CodeMap() : next_shared_id_(1) { }
-  INLINE(void AddCode(Address addr, CodeEntry* entry, unsigned size));
-  INLINE(void MoveCode(Address from, Address to));
-  INLINE(void DeleteCode(Address addr));
+  void AddCode(Address addr, CodeEntry* entry, unsigned size);
+  void MoveCode(Address from, Address to);
   CodeEntry* FindEntry(Address addr);
   int GetSharedId(Address addr);
 
@@ -270,6 +269,8 @@ class CodeMap {
     void Call(const Address& key, const CodeEntryInfo& value);
   };
 
+  void DeleteAllCoveredCode(Address start, Address end);
+
   // Fake CodeEntry pointer to distinguish shared function entries.
   static CodeEntry* const kSharedFunctionCodeEntry;
 

src/runtime.cc

@@ -2243,6 +2243,11 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetCode) {
     // are guaranteed to be in old space.
     target->set_literals(*literals, SKIP_WRITE_BARRIER);
     target->set_next_function_link(isolate->heap()->undefined_value());
+
+    if (isolate->logger()->is_logging() || CpuProfiler::is_profiling(isolate)) {
+      isolate->logger()->LogExistingFunction(
+          shared, Handle<Code>(shared->code()));
+    }
   }
 
   target->set_context(*context);

test/cctest/log-eq-of-logging-and-traversal.js

@@ -43,8 +43,7 @@ function LogProcessor() {
           processor: this.processCodeCreation },
       'code-move': { parsers: [parseInt, parseInt],
           processor: this.processCodeMove },
-      'code-delete': { parsers: [parseInt],
-          processor: this.processCodeDelete },
+      'code-delete': null,
       'sfi-move': { parsers: [parseInt, parseInt],
           processor: this.processFunctionMove },
       'shared-library': null,
@@ -73,10 +72,6 @@ LogProcessor.prototype.processCodeMove = function(from, to) {
   this.profile.moveCode(from, to);
 };
 
-LogProcessor.prototype.processCodeDelete = function(start) {
-  this.profile.deleteCode(start);
-};
-
 LogProcessor.prototype.processFunctionMove = function(from, to) {
   this.profile.moveFunc(from, to);
 };
@@ -132,8 +127,8 @@ function RunTest() {
      "Script", "String", "RegExp", "Date", "Error"];
 
   function entitiesEqual(entityA, entityB) {
-    if (entityA === null && entityB !== null) return true;
-    if (entityA !== null && entityB === null) return false;
+    if ((entityA === null && entityB !== null) ||
+      (entityA !== null && entityB === null)) return true;
     return entityA.size === entityB.size && entityNamesEqual(entityA, entityB);
   }
 
@@ -145,6 +140,8 @@ function RunTest() {
   // find the same entries. We skip builtins during log parsing, but compiled
   // functions traversal may erroneously recognize them as functions, so we are
   // expecting more functions in traversal vs. logging.
+  // Since we don't track code deletions, logging can also report more entries
+  // than traversal.
   while (l_pos < l_len && t_pos < t_len) {
     var entryA = logging_entries[l_pos];
     var entryB = traversal_entries[t_pos];
@@ -166,11 +163,6 @@ function RunTest() {
     if (!entities_equal) equal = false;
     comparison.push([entities_equal, address, entityA, entityB]);
   }
-  if (l_pos < l_len) equal = false;
-  while (l_pos < l_len) {
-    var entryA = logging_entries[l_pos++];
-    comparison.push([false, entryA[0], entryA[1], null]);
-  }
   return [equal, comparison];
 }
 

test/cctest/test-cpu-profiler.cc

@@ -107,7 +107,7 @@ TEST(CodeEvents) {
                             0x80);
   processor.CodeMoveEvent(ToAddress(0x1400), ToAddress(0x1500));
   processor.CodeCreateEvent(i::Logger::STUB_TAG, 3, ToAddress(0x1600), 0x10);
-  processor.CodeDeleteEvent(ToAddress(0x1600));
+  processor.CodeCreateEvent(i::Logger::STUB_TAG, 4, ToAddress(0x1605), 0x10);
   // Enqueue a tick event to enable code events processing.
   EnqueueTickSampleEvent(&processor, ToAddress(0x1000));
 

test/cctest/test-log.cc

@@ -25,6 +25,7 @@ using v8::internal::StrLength;
 
 namespace {
 
+
 class ScopedLoggerInitializer {
  public:
   explicit ScopedLoggerInitializer(bool prof_lazy)
@@ -470,8 +471,9 @@ TEST(IsLoggingPreserved) {
 
 typedef i::NativesCollection<i::TEST> TestSources;
 
-// Test that logging of code create / move / delete events
-// is equivalent to traversal of a resulting heap.
+
+// Test that logging of code create / move events is equivalent to traversal of
+// a resulting heap.
 TEST(EquivalenceOfLoggingAndTraversal) {
   // This test needs to be run on a "clean" V8 to ensure that snapshot log
   // is loaded. This is always true when running using tools/test.py because

test/cctest/test-profile-generator.cc

@@ -549,13 +549,14 @@ TEST(CodeMapMoveAndDeleteCode) {
   code_map.AddCode(ToAddress(0x1700), &entry2, 0x100);
   CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1500)));
   CHECK_EQ(&entry2, code_map.FindEntry(ToAddress(0x1700)));
-  code_map.MoveCode(ToAddress(0x1500), ToAddress(0x1800));
+  code_map.MoveCode(ToAddress(0x1500), ToAddress(0x1700));  // Deprecate bbb.
   CHECK_EQ(NULL, code_map.FindEntry(ToAddress(0x1500)));
-  CHECK_EQ(&entry2, code_map.FindEntry(ToAddress(0x1700)));
-  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1800)));
-  code_map.DeleteCode(ToAddress(0x1700));
+  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1700)));
+  CodeEntry entry3(i::Logger::FUNCTION_TAG, "", "ccc", "", 0,
+                   TokenEnumerator::kNoSecurityToken);
+  code_map.AddCode(ToAddress(0x1750), &entry3, 0x100);
   CHECK_EQ(NULL, code_map.FindEntry(ToAddress(0x1700)));
-  CHECK_EQ(&entry1, code_map.FindEntry(ToAddress(0x1800)));
+  CHECK_EQ(&entry3, code_map.FindEntry(ToAddress(0x1750)));
 }