core-jit:

runtime/vm/compilation_trace.cc

 // Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/compilation_trace.h"
 
 #include "vm/longjump.h"
 #include "vm/object_store.h"
 #include "vm/resolver.h"
 #include "vm/symbols.h"
 
 namespace dart {
 
 CompilationTraceSaver::CompilationTraceSaver(Zone* zone)
     : buf_(zone, 4 * KB),
       func_name_(String::Handle(zone)),
       cls_(Class::Handle(zone)),
       cls_name_(String::Handle(zone)),
       lib_(Library::Handle(zone)),
       uri_(String::Handle(zone)) {}
 
 
 void CompilationTraceSaver::Visit(const Function& function) {
   if (!function.HasCode()) {
+    return;  // Not compiled.
+  }
+  if (function.parent_function() != Function::null()) {
+    // Lookup works poorly for local functions. We compile all local functions
+    // in a compiled function instead.
     return;
   }
 
   func_name_ = function.name();
   func_name_ = String::RemovePrivateKey(func_name_);
   cls_ = function.Owner();
   cls_name_ = cls_.Name();
   cls_name_ = String::RemovePrivateKey(cls_name_);
   lib_ = cls_.library();
   uri_ = lib_.url();
@@ skipping 10 matching lines @@
       function_name_(String::Handle(zone_)),
       function_name2_(String::Handle(zone_)),
       lib_(Library::Handle(zone_)),
       cls_(Class::Handle(zone_)),
       function_(Function::Handle(zone_)),
       function2_(Function::Handle(zone_)),
       field_(Field::Handle(zone_)),
       error_(Object::Handle(zone_)) {}
 
 
-RawObject* CompilationTraceLoader::CompileTrace(char* buffer) {
+static char* FindCharacter(char* str, char goal, char* limit) {
+  while (str < limit) {
+    if (*str == goal) {
+      return str;
+    }
+    str++;
+  }
+  return NULL;
+}
+
+
+RawObject* CompilationTraceLoader::CompileTrace(uint8_t* buffer,
+                                                intptr_t size) {
   // First compile functions named in the trace.
-  char* cursor = buffer;
-  while (cursor != NULL) {
+  char* cursor = reinterpret_cast<char*>(buffer);
+  char* limit = cursor + size;
+  while (cursor < limit) {
     char* uri = cursor;
-    char* comma1 = strchr(uri, ',');
+    char* comma1 = FindCharacter(uri, ',', limit);

[zra, 2017/06/08 03:10:30]

Is memchr() portable?

[rmacnak, 2017/06/08 22:15:09]

I think so, but it uses a length that would need to be updated at each step
instead of a constant limit.

     if (comma1 == NULL) break;

[zra, 2017/06/08 03:10:30]

Curly braces for single-line ifs.

[rmacnak, 2017/06/08 22:15:09]

Done.

     *comma1 = 0;
     char* cls_name = comma1 + 1;
-    char* comma2 = strchr(cls_name, ',');
+    char* comma2 = FindCharacter(cls_name, ',', limit);
     if (comma2 == NULL) break;
     *comma2 = 0;
     char* func_name = comma2 + 1;
-    char* newline = strchr(func_name, '\n');
+    char* newline = FindCharacter(func_name, '\n', limit);
     if (newline == NULL) break;
     *newline = 0;
     error_ = CompileTriple(uri, cls_name, func_name);
     if (error_.IsError()) {
       return error_.raw();
     }
     cursor = newline + 1;
   }
 
   // Next, compile common dispatchers. These aren't found with the normal
   // lookup above because they have irregular lookup that depends on the
   // arguments descriptor (e.g. call() versus call(x)).
   const Class& closure_class =
       Class::Handle(zone_, thread_->isolate()->object_store()->closure_class());
   Array& arguments_descriptor = Array::Handle(zone_);
   Function& dispatcher = Function::Handle(zone_);
   for (intptr_t argc = 1; argc <= 4; argc++) {
     const intptr_t kTypeArgsLen = 0;
     arguments_descriptor = ArgumentsDescriptor::New(kTypeArgsLen, argc);
     dispatcher = closure_class.GetInvocationDispatcher(
         Symbols::Call(), arguments_descriptor,
         RawFunction::kInvokeFieldDispatcher, true /* create_if_absent */);
     error_ = CompileFunction(dispatcher);
     if (error_.IsError()) {
       return error_.raw();
     }
   }
 
-  // Finally, compile closures in all compiled functions.
+  // Finally, compile closures in all compiled functions. Compiling inner
+  // functions can cause inner inner functions to be registered, so iterate
+  // to a fixed point.
   const GrowableObjectArray& closure_functions = GrowableObjectArray::Handle(
       zone_, thread_->isolate()->object_store()->closure_functions());
-  for (intptr_t i = 0; i < closure_functions.Length(); i++) {
-    function_ ^= closure_functions.At(i);
-    function2_ = function_.parent_function();
-    if (function2_.HasCode()) {
-      error_ = CompileFunction(function_);
-      if (error_.IsError()) {
-        return error_.raw();
+  intptr_t previous_length = 0;
+  while (closure_functions.Length() != previous_length) {
+    previous_length = closure_functions.Length();
+    for (intptr_t i = 0; i < closure_functions.Length(); i++) {

[zra, 2017/06/08 03:10:30]

How is closure_functions modified by CompileFunction()? The way this loop is
written it looks like a lot of compilation is being repeated (the loop always
starts from 0 instead of where you left off the last time). Maybe add a comment
to clarify.

[rmacnak, 2017/06/08 22:15:09]

CompileFunction may append to this list, since we register closure functions in
the after building its parent's flow graph.
Undid this because we're safe as long as we don't cache the length for the loop;
added comment to this effect.

+      function_ ^= closure_functions.At(i);
+      function2_ = function_.parent_function();
+      if (function2_.HasCode()) {
+        error_ = CompileFunction(function_);
+        if (error_.IsError()) {
+          return error_.raw();
+        }
       }
     }
   }
 
   return Object::null();
 }
 
 
 RawObject* CompilationTraceLoader::CompileTriple(const char* uri_cstr,

[zra, 2017/06/08 03:10:30]

A comment about the algorithm used for the fuzzy matching might be useful here.

[rmacnak, 2017/06/08 22:15:09]

Added

                                                  const char* cls_cstr,
                                                  const char* func_cstr) {
   uri_ = Symbols::New(thread_, uri_cstr);
   class_name_ = Symbols::New(thread_, cls_cstr);
   function_name_ = Symbols::New(thread_, func_cstr);
 
   if (function_name_.Equals("_getMainClosure")) {
     // The scheme for invoking main relies on compiling _getMainClosure after
     // synthetically importing the root library.
     return Object::null();
@@ skipping 25 matching lines @@
       return Object::null();
     }
 
     error_ = cls_.EnsureIsFinalized(thread_);
     if (error_.IsError()) {
       return error_.raw();
     }
 
     function_ = cls_.LookupFunctionAllowPrivate(function_name_);
     field_ = cls_.LookupFieldAllowPrivate(function_name_);
-    if (field_.IsNull() && is_getter) {
-      // Maybe this is a tear off.
+    if (function_.IsNull() && is_getter) {
+      // Maybe this was a tear off.
       add_closure = true;
       function_name2_ = Field::NameFromGetter(function_name_);
       function_ = cls_.LookupFunctionAllowPrivate(function_name2_);
       field_ = cls_.LookupFieldAllowPrivate(function_name2_);
       if (!function_.IsNull() && !function_.is_static()) {
         // Maybe this was a method extractor.
         function2_ =
             Resolver::ResolveDynamicAnyArgs(zone_, cls_, function_name_);
         if (!function2_.IsNull()) {
           error_ = CompileFunction(function2_);
@@ skipping 46 matching lines @@
   } else {
     Thread* thread = Thread::Current();
     const Error& error = Error::Handle(thread->sticky_error());
     thread->clear_sticky_error();
     return error.raw();
   }
   return Object::null();
 }
 
 }  // namespace dart