Defer addStubs to class instantiation time.

pkg/compiler/lib/src/js_emitter/old_emitter/emitter.dart

 // Copyright (c) 2014, 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.
 
 part of dart2js.js_emitter;
 
 
 class OldEmitter implements Emitter {
   final Compiler compiler;
   final CodeEmitterTask task;
@@ skipping 317 matching lines @@
 
           for (var i = 0; i < fields.length; i++) {
             if(i != 0) str += ", ";
 
             var field = generateAccessor(fields[i], accessors, name);
             if (#hasIsolateSupport) { fieldNames += "'" + field + "',"; }
             var parameter = "p_" + field;
             str += parameter;
             body += ("this." + field + " = " + parameter + ";\n");
           }
+          if (supportsDirectProtoAccess)

[floitsch, 2015/03/06 14:54:09]

Sigurd put in an optimization that removes blocks if they only contain one
statement.
-> add the curly braces.

[herhut, 2015/03/09 14:28:34]

WHOOHOO!

Done.

+            body += "this." + #deferredAction + "();"; 
           str += ") {\n" + body + "}\n";
           str += name + ".builtin$cls=\"" + name + "\";\n";
           str += "$desc=$collectedClasses." + name + "[1];\n";
           str += name + ".prototype = $desc;\n";
           if (typeof defineClass.name != "string") {
             str += name + ".name=\"" + name + "\";\n";
           }
           if (#hasIsolateSupport) {
             str += name + "." + #fieldNamesProperty + "=[" + fieldNames
                    + "];\n";
           }
           str += accessors.join("");
 
           return str;
-        }''', { 'hasIsolateSupport': hasIsolateSupport,
+        }''', { 'deferredAction': js.string(namer.deferredAction),
+                'hasIsolateSupport': hasIsolateSupport,
                 'fieldNamesProperty': js.string(fieldNamesProperty)});
 
     // Declare a function called "generateAccessor".  This is used in
     // defineClassFunction.
     List result = <jsAst.Node>[
         generateAccessorFunction,
         new jsAst.FunctionDeclaration(
             new jsAst.VariableDeclaration('defineClass'), defineClass) ];
 
     if (compiler.hasIncrementalSupport) {
@@ skipping 53 matching lines @@
 
     return result;
   }
 
   /** Needs defineClass to be defined. */
   jsAst.Expression buildInheritFrom() {
     jsAst.Expression result = js(r"""
     // If the browser supports changing the prototype via __proto__, we make
     // use of that feature. Otherwise, we copy the properties into a new
     // constructor.
-    (function () {
-      var cls = function () {};
-      cls.prototype = {'p': {}};
-      var object = new cls();
-      return object.__proto__ &&
-             object.__proto__.p === cls.prototype.p;
-     })() ?
+     supportsDirectProtoAccess ?
         function(constructor, superConstructor) {
           var prototype = constructor.prototype;
           prototype.__proto__ = superConstructor.prototype;
           // Use a function for `true` here, as functions are stored in the
           // hidden class and not as properties in the object.
           prototype.constructor = constructor;
           prototype[#operatorIsPrefix + constructor.name] = constructor;
           return convertToFastObject(prototype);
         } :
         function() {
@@ skipping 79 matching lines @@
         // class.  The minifier together with noSuchMethod can put methods on
         // the Object.prototype object, and they show through here, so we check
         // that we have a string.
         if (!superclass || typeof superclass != "string") {
           // Inlined special case of InheritFrom here for performance reasons.
           // Fix up the the Dart Object class' prototype.
           var constructor = allClasses[cls];
           var prototype = constructor.prototype;
           prototype.constructor = constructor;
           prototype.#isObject = constructor;
+          prototype.#deferredAction = markerFun;

[floitsch, 2015/03/06 14:54:09]

given that this function comes from a different file, use a shared constant to
link them.

[floitsch, 2015/03/06 14:54:09]

Explain what the marker-fun is for (either here or at the marker-fun
declaration. maybe both).

[herhut, 2015/03/09 14:28:34]

zarah@ is working on putting the two chunks of JS into one file, so she would
undo it soon, anyway. I'll leave it as it is.

[herhut, 2015/03/09 14:28:34]

Done.

           return;
         }
         finishClass(superclass);
         var superConstructor = allClasses[superclass];
 
         if (!superConstructor)
           superConstructor = existingIsolateProperties[superclass];
 
         var constructor = allClasses[cls];
         var prototype = inheritFrom(constructor, superConstructor);
 
         if (#hasNativeClasses)
-          if (Object.prototype.hasOwnProperty.call(prototype, $specProperty))
-            #nativeInfoHandler
+          if (Object.prototype.hasOwnProperty.call(prototype, $specProperty)) {
+            #nativeInfoHandler;
+            // As native classes can come into existence without a constructor
+            // call, we have to ensure that the class has been fully
+            // initialized. 
+            if (constructor.prototype.#deferredAction) 

[floitsch, 2015/03/06 14:54:09]

ditto (add curly braces).

[herhut, 2015/03/09 14:28:34]

Done.

+              finishAddStubsHelper(constructor.prototype);
+          }
+        // Interceptors (or rather their prototypes) are also used without
+        // first instantiating them first.

[floitsch, 2015/03/06 14:54:09]

This looks like we are only eagerly running the deferredAction if the direct
superclass is the interceptor class.
Are we sure that there are not sub-subclasses? I'm particularly worried about
typed-arrays.

[floitsch, 2015/03/09 16:35:06]

any thoughts on this one?

+        if (superclass === #interceptorClassName && 
+            constructor.prototype.#deferredAction) {
+          finishAddStubsHelper(constructor.prototype);
+        }
       }
-    }''', {'finishedClassesAccess': finishedClassesAccess,
+    }''', {'deferredAction': namer.deferredAction,
+           'finishedClassesAccess': finishedClassesAccess,
            'needsMixinSupport': needsMixinSupport,
            'hasNativeClasses': hasNativeClasses,
            'nativeInfoHandler': nativeInfoHandler,
+           'interceptorClassName':
+             js.string(namer.getNameOfClass(backend.jsInterceptorClass)),
            'isObject' : namer.operatorIs(compiler.objectClass) });
   }
 
   void emitFinishIsolateConstructorInvocation(CodeOutput output) {
     String isolate = namer.isolateName;
     output.add("$isolate = $finishIsolateConstructorName($isolate)$N");
   }
 
   /// In minified mode we want to keep the name for the most common core types.
   bool _isNativeTypeNeedingReflectionName(Element element) {
@@ skipping 573 matching lines @@
       // mode.
       properties.__MAGIC_SLOW_PROPERTY = 1;
       delete properties.__MAGIC_SLOW_PROPERTY;
       return properties;
     }''');
 
     output.addBuffer(jsAst.prettyPrint(convertToSlowObject, compiler));
     output.add(N);
   }
 
+  void emitSupportsDirectProtoAccess(CodeOutput output) {
+    jsAst.Statement supportsDirectProtoAccess = js.statement(r'''

[floitsch, 2015/03/06 14:54:09]

Since this is used in a completely different part of the code, use a common
constant to link the places.

Maybe we should have a class: ClassConstructionNames constructionNames;
and then use it with constructionNames.supportsDirectProtoAccess.

This way we wouldn't have all these "final XyzName = '...'" and group them into
nice classes.
Ideally we should then run them through the namer to make sure that they are
safe (and don't shadow anything important).

[herhut, 2015/03/09 14:28:35]

I would actually prefer to emit this as part of the combined
reflectionDataParser with finishClasses, as this should not be visible to other
parts of the code. It currently is here as we have this artificial split between
finishClasses and parsing reflection data. 

I will work with zarah@ to make this happen in a follow up CL.

+      var supportsDirectProtoAccess = (function () {
+        var cls = function () {};
+        cls.prototype = {'p': {}};
+        var object = new cls();
+        return object.__proto__ &&
+               object.__proto__.p === cls.prototype.p;
+       })();
+    ''');
+
+    output.addBuffer(jsAst.prettyPrint(supportsDirectProtoAccess, compiler));
+    output.add(N);
+  }
+
   void writeLibraryDescriptors(CodeOutput output, LibraryElement library) {
     var uri = "";
     if (!compiler.enableMinification || backend.mustPreserveUris) {
       uri = library.canonicalUri;
       if (uri.scheme == 'file' && compiler.outputUri != null) {
         uri = relativize(compiler.outputUri, library.canonicalUri, false);
       }
     }
     ClassBuilder descriptor = elementDescriptors[library];
     if (descriptor == null) {
@@ skipping 209 matching lines @@
       /// The semicolon is important in minified mode, without it the
       /// following parenthesis looks like a call to the object literal.
       mainOutput.add(
           'self.${deferredInitializers} = self.${deferredInitializers} || '
           'Object.create(null);$n');
     }
 
     // Using a named function here produces easier to read stack traces in
     // Chrome/V8.
     mainOutput.add('(function(${namer.currentIsolate})$_{\n');
+    emitSupportsDirectProtoAccess(mainOutput);
     if (compiler.hasIncrementalSupport) {
       mainOutput.addBuffer(jsAst.prettyPrint(js.statement(
           """
 {
   #helper = #helper || Object.create(null);
   #helper.patch = function(a) { eval(a)};
   #helper.schemaChange = #schemaChange;
   #helper.addMethod = #addMethod;
   #helper.extractStubs = function(array, name, isStatic, originalDescriptor) {
     var descriptor = Object.create(null);
@@ skipping 651 matching lines @@
     for (Element element in compiler.enqueuer.codegen.newlyEnqueuedElements) {
       if (element.isInstanceMember) {
         cachedClassBuilders.remove(element.enclosingClass);
 
         nativeEmitter.cachedBuilders.remove(element.enclosingClass);
 
       }
     }
   }
 }