Store SharedFunctionInfos of a Script in a FixedArray indexed by their ID

src/debug/debug.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/debug/debug.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/arguments.h"
@@ skipping 1314 matching lines @@
     BytecodeArrayBreakIterator it(debug_info, ALL_BREAK_LOCATIONS);
     GetBreakablePositions(&it, start_position, end_position, alignment,
                           positions);
   }
 }
 }  // namespace
 
 bool Debug::GetPossibleBreakpoints(Handle<Script> script, int start_position,
                                    int end_position, std::set<int>* positions) {
   while (true) {
-    if (!script->shared_function_infos()->IsWeakFixedArray()) return false;
-
-    WeakFixedArray* infos =
-        WeakFixedArray::cast(script->shared_function_infos());
+    FixedArray* infos = script->shared_function_infos();
     HandleScope scope(isolate_);
     List<Handle<SharedFunctionInfo>> candidates;
-    {
-      WeakFixedArray::Iterator iterator(infos);
-      SharedFunctionInfo* info;
-      while ((info = iterator.Next<SharedFunctionInfo>())) {
-        if (info->end_position() < start_position ||
-            info->start_position() >= end_position) {
-          continue;
-        }
-        if (!info->IsSubjectToDebugging()) continue;
-        if (!info->HasDebugCode() && !info->allows_lazy_compilation()) continue;
-        candidates.Add(i::handle(info));
+    for (int index = 0; index < infos->length(); ++index) {
+      Object* raw = infos->get(index);
+      if (raw->IsSmi() || WeakCell::cast(raw)->cleared()) continue;

[Toon Verwaest, 2016/12/06 21:10:14]

I guess we should just install a cleared weak-cell as default value; but it
probably doesn't matter much; and could even be worse due to gc.

[jochen (gone - plz use gerrit), 2016/12/07 15:57:00]

I considered that, but it would create a lot of overhead when allocating the
arrays (and make them considerably larger)

[Toon Verwaest, 2016/12/08 07:31:30]

Why? I presume we have a single preallocated cleared weak-cell somewhere? And if
we don't, we could create one?

[jochen (gone - plz use gerrit), 2016/12/08 12:31:14]

no, we don't have that. The WeakCell currently has an invariant that a newly
created WeakCell is never "cleared"

+      SharedFunctionInfo* info =
+          SharedFunctionInfo::cast(WeakCell::cast(raw)->value());

[Toon Verwaest, 2016/12/06 21:10:14]

This actually makes me wonder if we shouldn't just have something like
SharedFunctionInfo* info = WeakCell::cast(raw)->value<SharedFunctionInfo>();

That would save us 6 characters on basically each weak-cell read :)

[jochen (gone - plz use gerrit), 2016/12/07 15:57:00]

if we do that, I think it should be a separate clean-up replacing all existing
callsites of T::cast(WeakCell::cast(..)->value())

[Toon Verwaest, 2016/12/08 07:31:30]

Makes sense

+      if (info->end_position() < start_position ||
+          info->start_position() >= end_position) {
+        continue;
       }
+      if (!info->IsSubjectToDebugging()) continue;
+      if (!info->HasDebugCode() && !info->allows_lazy_compilation()) continue;
+      candidates.Add(i::handle(info));
     }
 
     bool was_compiled = false;
     for (int i = 0; i < candidates.length(); ++i) {
       // Code that cannot be compiled lazily are internal and not debuggable.
       DCHECK(candidates[i]->allows_lazy_compilation());
       if (!candidates[i]->HasDebugCode()) {
         if (!Compiler::CompileDebugCode(candidates[i])) {
           return false;
         } else {
@@ skipping 89 matching lines @@
 // While we are at this, also ensure code with debug break slots so that we do
 // not have to compile a SFI without JSFunction, which is paifu for those that
 // cannot be compiled without context (need to find outer compilable SFI etc.)
 Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
                                                      int position) {
   for (int iteration = 0;; iteration++) {
     // Go through all shared function infos associated with this script to
     // find the inner most function containing this position.
     // If there is no shared function info for this script at all, there is
     // no point in looking for it by walking the heap.
-    if (!script->shared_function_infos()->IsWeakFixedArray()) break;
 
     SharedFunctionInfo* shared;
     {
       SharedFunctionInfoFinder finder(position);
-      WeakFixedArray::Iterator iterator(script->shared_function_infos());
-      SharedFunctionInfo* candidate;
-      while ((candidate = iterator.Next<SharedFunctionInfo>())) {
-        finder.NewCandidate(candidate);
+      FixedArray* infos = script->shared_function_infos();
+      for (int index = 0; index < infos->length(); ++index) {

[Toon Verwaest, 2016/12/06 21:10:14]

Given that we iterate at least 3 times, does it make sense to have an iterator
that encapsulates this?

[jochen (gone - plz use gerrit), 2016/12/07 15:57:00]

done.

+        Object* raw = infos->get(index);
+        if (raw->IsSmi() || WeakCell::cast(raw)->cleared()) continue;
+        finder.NewCandidate(
+            SharedFunctionInfo::cast(WeakCell::cast(raw)->value()));
       }
       shared = finder.Result();
       if (shared == NULL) break;
       // We found it if it's already compiled and has debug code.
       if (shared->HasDebugCode()) {
         Handle<SharedFunctionInfo> shared_handle(shared);
         // If the iteration count is larger than 1, we had to compile the outer
         // function in order to create this shared function info. So there can
         // be no JSFunction referencing it. We can anticipate creating a debug
         // info while bypassing PrepareFunctionForBreakpoints.
@@ skipping 1028 matching lines @@
   logger_->DebugEvent("Put", message.text());
 }
 
 void LockingCommandMessageQueue::Clear() {
   base::LockGuard<base::Mutex> lock_guard(&mutex_);
   queue_.Clear();
 }
 
 }  // namespace internal
 }  // namespace v8