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

src/debug/debug.cc

Index: src/debug/debug.cc
diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index c8db633f7c3aa30b1b5ebd8ab8f2a60e01e7e1d3..28a0a3dd8be5fa50dfdc5cea750a5716916fb1c7 100644
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -1332,24 +1332,21 @@ void FindBreakablePositions(Handle<DebugInfo> debug_info, int start_position,
 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;
@@ -1459,15 +1456,16 @@ Handle<Object> Debug::FindSharedFunctionInfoInScript(Handle<Script> script,
     // 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;