dart2js: Use an abstract Name class for names in the generated JavaScript ast.

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

 // Copyright (c) 2013, 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 NsmEmitter extends CodeEmitterHelper {
   final List<Selector> trivialNsmHandlers = <Selector>[];
 
   /// If this is true then we can generate the noSuchMethod handlers at startup
   /// time, instead of them being emitted as part of the Object class.
   bool get generateTrivialNsmHandlers => true;
 
   // If we need fewer than this many noSuchMethod handlers we can save space by
   // just emitting them in JS, rather than emitting the JS needed to generate
   // them at run time.
   static const VERY_FEW_NO_SUCH_METHOD_HANDLERS = 10;
 
   static const MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING = 4;
 
   void emitNoSuchMethodHandlers(AddPropertyFunction addProperty) {
 
     ClassStubGenerator generator =
         new ClassStubGenerator(compiler, namer, backend);
 
     // Keep track of the JavaScript names we've already added so we
     // do not introduce duplicates (bad for code size).
-    Map<String, Selector> addedJsNames
+    Map<jsAst.Name, Selector> addedJsNames
         = generator.computeSelectorsForNsmHandlers();
 
     // Set flag used by generateMethod helper below.  If we have very few
     // handlers we use addProperty for them all, rather than try to generate
     // them at runtime.
     bool haveVeryFewNoSuchMemberHandlers =
         (addedJsNames.length < VERY_FEW_NO_SUCH_METHOD_HANDLERS);
-    for (String jsName in addedJsNames.keys.toList()..sort()) {
+    List<jsAst.Name> names = addedJsNames.keys.toList()
+        ..sort();
+    for (jsAst.Name jsName in names) {
       Selector selector = addedJsNames[jsName];
       String reflectionName = emitter.getReflectionName(selector, jsName);
 
       if (reflectionName != null) {
         emitter.mangledFieldNames[jsName] = reflectionName;
       }
 
       List<jsAst.Expression> argNames =
           selector.callStructure.getOrderedNamedArguments().map((String name) =>
               js.string(name)).toList();
       int type = selector.invocationMirrorKind;
       if (!haveVeryFewNoSuchMemberHandlers &&
           isTrivialNsmHandler(type, argNames, selector, jsName) &&
           reflectionName == null) {
         trivialNsmHandlers.add(selector);
       } else {
         StubMethod method =
             generator.generateStubForNoSuchMethod(jsName, selector);
         addProperty(method.name, method.code);
         if (reflectionName != null) {
           bool accessible =
               compiler.world.allFunctions.filter(selector, null).any(
               (Element e) => backend.isAccessibleByReflection(e));
-          addProperty('+$reflectionName', js(accessible ? '2' : '0'));
+          addProperty(namer.asName('+$reflectionName'),
+                      js(accessible ? '2' : '0'));
         }
       }
     }
   }
 
   // Identify the noSuchMethod handlers that are so simple that we can
   // generate them programatically.
   bool isTrivialNsmHandler(
-      int type, List argNames, Selector selector, String internalName) {
+      int type, List argNames, Selector selector, jsAst.Name internalName) {
     if (!generateTrivialNsmHandlers) return false;
-    // Check for interceptor calling convention.
-    if (backend.isInterceptedName(selector.name)) {
-      // We can handle the calling convention used by intercepted names in the
-      // diff encoding, but we don't use that for non-minified code.
-      if (!compiler.enableMinification) return false;
-      String shortName = namer.invocationMirrorInternalName(selector);
-      if (shortName.length > MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING) {
-        return false;
-      }
-    }
     // Check for named arguments.
     if (argNames.length != 0) return false;
     // Check for unexpected name (this doesn't really happen).
-    if (internalName.startsWith(namer.getterPrefix[0])) return type == 1;
-    if (internalName.startsWith(namer.setterPrefix[0])) return type == 2;
+    if (internalName is GetterName) return type == 1;
+    if (internalName is SetterName) return type == 2;
     return type == 0;
   }
 
   /**
    * Adds (at runtime) the handlers to the Object class which catch calls to
    * methods that the object does not have.  The handlers create an invocation
    * mirror object.
    *
    * The current version only gives you the minified name when minifying (when
    * not minifying this method is not called).
@@ skipping 25 matching lines @@
    * The reason we don't encode long minified names with this method is that
    * decoding the base 88 numbers would overflow JavaScript's puny integers.
    *
    * There are some selectors that have a special calling convention (because
    * they are called with the receiver as the first argument).  They need a
    * slightly different noSuchMethod handler, so we handle these first.
    */
   List<jsAst.Statement> buildTrivialNsmHandlers() {
     List<jsAst.Statement> statements = <jsAst.Statement>[];
     if (trivialNsmHandlers.length == 0) return statements;
-    // Sort by calling convention, JS name length and by JS name.
-    trivialNsmHandlers.sort((a, b) {
-      bool aIsIntercepted = backend.isInterceptedName(a.name);
-      bool bIsIntercepted = backend.isInterceptedName(b.name);
-      if (aIsIntercepted != bIsIntercepted) return aIsIntercepted ? -1 : 1;
-      String aName = namer.invocationMirrorInternalName(a);
-      String bName = namer.invocationMirrorInternalName(b);
-      if (aName.length != bName.length) return aName.length - bName.length;
-      return aName.compareTo(bName);
-    });
 
     // Find out how many selectors there are with the special calling
     // convention.
-    int firstNormalSelector = trivialNsmHandlers.length;
-    for (int i = 0; i < trivialNsmHandlers.length; i++) {
-      if (!backend.isInterceptedName(trivialNsmHandlers[i].name)) {
-        firstNormalSelector = i;
-        break;
-      }
+    bool hasSpecialCallingConvention(Selector selector) {
+        return backend.isInterceptedName(selector.name);
     }
+    Iterable<Selector> specialSelectors = trivialNsmHandlers.where(
+        (Selector s) => backend.isInterceptedName(s.name));
+    Iterable<Selector> ordinarySelectors = trivialNsmHandlers.where(
+        (Selector s) => !backend.isInterceptedName(s.name));
 
     // Get the short names (JS names, perhaps minified).
-    Iterable<String> shorts = trivialNsmHandlers.map((selector) =>
-         namer.invocationMirrorInternalName(selector));
-    var diffEncoding = new StringBuffer();
-
-    // Treat string as a number in base 88 with digits in ASCII order from # to
-    // z.  The short name sorting is based on length, and uses ASCII order for
-    // equal length strings so this means that names are ascending.  The hash
-    // character, #, is never given as input, but we need it because it's the
-    // implicit leading zero (otherwise we could not code names with leading
-    // dollar signs).
-    int fromBase88(String x) {
-      int answer = 0;
-      for (int i = 0; i < x.length; i++) {
-        int c = x.codeUnitAt(i);
-        // No support for Unicode minified identifiers in JS.
-        assert(c >= $$ && c <= $z);
-        answer *= 88;
-        answer += c - $HASH;
-      }
-      return answer;
-    }
-
-    // Big endian encoding, A = 0, B = 1...
-    // A lower case letter terminates the number.
-    String toBase26(int x) {
-      int c = x;
-      var encodingChars = <int>[];
-      encodingChars.add($a + (c % 26));
-      while (true) {
-        c ~/= 26;
-        if (c == 0) break;
-        encodingChars.add($A + (c % 26));
-      }
-      return new String.fromCharCodes(encodingChars.reversed.toList());
-    }
+    List<jsAst.Name> specialShorts =
+        specialSelectors.map(namer.invocationMirrorInternalName);
+    List<jsAst.Name> ordinaryShorts =
+        ordinarySelectors.map(namer.invocationMirrorInternalName);
 
     bool minify = compiler.enableMinification;
-    bool useDiffEncoding = minify && shorts.length > 30;
+    bool useDiffEncoding = minify && trivialNsmHandlers.length > 30;
 
-    int previous = 0;
-    int nameCounter = 0;
-    for (String short in shorts) {
-      // Emit period that resets the diff base to zero when we switch to normal
-      // calling convention (this avoids the need to code negative diffs).
-      if (useDiffEncoding && nameCounter == firstNormalSelector) {
-        diffEncoding.write(".");
-        previous = 0;
-      }
-      if (short.length <= MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING &&
-          useDiffEncoding) {
-        int base63 = fromBase88(short);
-        int diff = base63 - previous;
-        previous = base63;
-        String base26Diff = toBase26(diff);
-        diffEncoding.write(base26Diff);
-      } else {
-        if (useDiffEncoding || diffEncoding.length != 0) {
-          diffEncoding.write(",");
-        }
-        diffEncoding.write(short);
-      }
-      nameCounter++;
-    }
+    jsAst.Expression diffEncoding = new _DiffEncodedListOfNames(
+        [specialShorts, ordinaryShorts],
+        useDiffEncoding);
 
     // Startup code that loops over the method names and puts handlers on the
     // Object class to catch noSuchMethod invocations.
     ClassElement objectClass = compiler.objectClass;
     jsAst.Expression createInvocationMirror = backend.emitter
         .staticFunctionAccess(backend.getCreateInvocationMirror());
     if (useDiffEncoding) {
       statements.add(js.statement('''{
           var objectClassObject = processedClasses.collected[#objectClass],
-              shortNames = #diffEncoding.split(","),
-              nameNumber = 0,
-              diffEncodedString = shortNames[0],
-              calculatedShortNames = [0, 1];    // 0, 1 are args for splice.
-          // If we are loading a deferred library the object class will not be in
-          // the collectedClasses so objectClassObject is undefined, and we skip
-          // setting up the names.
-
-          if (objectClassObject) {
-            if (objectClassObject instanceof Array)
-              objectClassObject = objectClassObject[1];
-            for (var i = 0; i < diffEncodedString.length; i++) {
-              var codes = [],
-                  diff = 0,
-                  digit = diffEncodedString.charCodeAt(i);
-              if (digit == ${$PERIOD}) {
-                nameNumber = 0;
-                digit = diffEncodedString.charCodeAt(++i);
+              nameSequences = #diffEncoding.split("."),
+              shortNames = [];
+          for (var j = 0; j < nameSequences.length; ++j) {
+              var sequence = nameSequences[j].split(","),
+                nameNumber = 0,
+                diffEncodedString = sequence[0];

[sra1, 2015/06/23 20:54:16]

Move under 'if (sequence.length)'.
If sequence can be empty, this might be a deopt.

[herhut, 2015/06/24 07:55:07]

Done.

+            // If we are loading a deferred library the object class will not be
+            // in the collectedClasses so objectClassObject is undefined, and we
+            // skip setting up the names.
+            // Likewise, if the current sequence is empty, we don't process it.
+            if (objectClassObject && sequence.length) {
+              if (objectClassObject instanceof Array)
+                objectClassObject = objectClassObject[1];

[sra1, 2015/06/23 20:54:16]

This should probably be hoisted out of the j loop.

[herhut, 2015/06/24 07:55:07]

Done.

+              for (var i = 0; i < diffEncodedString.length; i++) {
+                var codes = [],
+                    diff = 0,
+                    digit = diffEncodedString.charCodeAt(i);
+                for (; digit <= ${$Z};) {
+                  diff *= 26;
+                  diff += (digit - ${$A});
+                  digit = diffEncodedString.charCodeAt(++i);
+                }
+                diff *= 26;
+                diff += (digit - ${$a});
+                nameNumber += diff;
+                for (var remaining = nameNumber;
+                     remaining > 0;
+                     remaining = (remaining / 88) | 0) {
+                  codes.unshift(${$HASH} + remaining % 88);

[sra1, 2015/06/23 20:54:16]

I'm curious of you noticed this showing up in startup profiling?
In one big app, a 13k string encodes 8k stubs.

[herhut, 2015/06/24 07:55:07]

No, I was not even aware this existed before working on the namer change. 

+                }
+                shortNames.push(
+                  String.fromCharCode.apply(String, codes));
               }
-              for (; digit <= ${$Z};) {
-                diff *= 26;
-                diff += (digit - ${$A});
-                digit = diffEncodedString.charCodeAt(++i);
-              }
-              diff *= 26;
-              diff += (digit - ${$a});
-              nameNumber += diff;
-              for (var remaining = nameNumber;
-                   remaining > 0;
-                   remaining = (remaining / 88) | 0) {
-                codes.unshift(${$HASH} + remaining % 88);
-              }
-              calculatedShortNames.push(
-                String.fromCharCode.apply(String, codes));
+              Array.prototype.push.apply(shortNames, sequence.shift());
             }
-            shortNames.splice.apply(shortNames, calculatedShortNames);
           }
-        }''', {'objectClass': js.string(namer.className(objectClass)),
-               'diffEncoding': js.string('$diffEncoding')}));
+        }''', {'objectClass': js.quoteName(namer.className(objectClass)),
+               'diffEncoding': diffEncoding}));
     } else {
       // No useDiffEncoding version.
       Iterable<String> longs = trivialNsmHandlers.map((selector) =>
              selector.invocationMirrorMemberName);
       statements.add(js.statement(
           'var objectClassObject = processedClasses.collected[#objectClass],'
           '    shortNames = #diffEncoding.split(",")',
-          {'objectClass': js.string(namer.className(objectClass)),
-           'diffEncoding': js.string('$diffEncoding')}));
+          {'objectClass': js.quoteName(namer.className(objectClass)),
+           'diffEncoding': diffEncoding}));
       if (!minify) {
         statements.add(js.statement('var longNames = #longs.split(",")',
                 {'longs': js.string(longs.join(','))}));
       }
       statements.add(js.statement(
           'if (objectClassObject instanceof Array)'
           '  objectClassObject = objectClassObject[1];'));
     }
 
     List<jsAst.Expression> sliceOffsetArguments =
-        firstNormalSelector == 0
+        specialSelectors.isEmpty
         ? []
-        : (firstNormalSelector == shorts.length
+        : (ordinarySelectors.isEmpty
             ? [js.number(1)]
-            : [js('(j < #) ? 1 : 0', js.number(firstNormalSelector))]);
+            : [js('(j < #) ? 1 : 0', js.number(specialSelectors.length))]);

[sra1, 2015/06/23 20:54:16]

I fixed a bug in this code, sorry that you will have to merge.
(https://chromiumcodereview.appspot.com/1181063005/)

[herhut, 2015/06/24 07:55:07]

Acknowledged.

 
     var sliceOffsetParams = sliceOffsetArguments.isEmpty ? [] : ['sliceOffset'];
 
     statements.add(js.statement('''
       // If we are loading a deferred library the object class will not be in
       // the collectedClasses so objectClassObject is undefined, and we skip
       // setting up the names.
       if (objectClassObject) {
         for (var j = 0; j < shortNames.length; j++) {
           var type = 0;
@@ skipping 19 matching lines @@
       }''', {
           'sliceOffsetParams': sliceOffsetParams,
           'noSuchMethodName': namer.noSuchMethodName,
           'createInvocationMirror': createInvocationMirror,
           'names': minify ? 'shortNames' : 'longNames',
           'sliceOffsetArguments': sliceOffsetArguments}));
 
     return statements;
   }
 }
+
+/// When pretty printed, this node computes a diff-encoded string for the list
+/// of given names.
+///
+/// See [buildTrivialNsmHandlers].
+class _DiffEncodedListOfNames extends jsAst.DeferredString
+                              implements AstContainer {
+  String _cachedValue;
+  jsAst.ArrayInitializer ast;
+  bool useDiffEncoding;
+
+  _DiffEncodedListOfNames(Iterable<Iterable<jsAst.Name>> names,
+                          this.useDiffEncoding) {
+    // Store the names in ArrayInitializer nodes to make them discoverable
+    // by traversals of the ast.
+    ast = new jsAst.ArrayInitializer(
+        names.map((Iterable i) => new jsAst.ArrayInitializer(i.toList()))
+             .toList());
+  }
+
+  void _computeDiffEncodingForList(Iterable<jsAst.Name> names,
+                                   StringBuffer diffEncoding) {
+    // Treat string as a number in base 88 with digits in ASCII order from # to
+    // z.  The short name sorting is based on length, and uses ASCII order for
+    // equal length strings so this means that names are ascending.  The hash
+    // character, #, is never given as input, but we need it because it's the
+    // implicit leading zero (otherwise we could not code names with leading
+    // dollar signs).
+    int fromBase88(String x) {
+      int answer = 0;
+      for (int i = 0; i < x.length; i++) {
+        int c = x.codeUnitAt(i);
+        // No support for Unicode minified identifiers in JS.
+        assert(c >= $$ && c <= $z);
+        answer *= 88;
+        answer += c - $HASH;
+      }
+      return answer;
+    }
+
+    // Big endian encoding, A = 0, B = 1...
+    // A lower case letter terminates the number.
+    String toBase26(int x) {
+      int c = x;
+      var encodingChars = <int>[];
+      encodingChars.add($a + (c % 26));
+      while (true) {
+        c ~/= 26;
+        if (c == 0) break;
+        encodingChars.add($A + (c % 26));
+      }
+      return new String.fromCharCodes(encodingChars.reversed.toList());
+    }
+
+    // Sort by length, then lexicographic.
+    int compare(String a, String b) {
+      if (a.length != b.length) return a.length - b.length;
+      return a.compareTo(b);
+    }
+
+    List<String> shorts =
+        names.map((jsAst.Name name) => name.name)
+        .toList()
+        ..sort(compare);
+
+    int previous = 0;
+    for (String short in shorts) {
+      if (useDiffEncoding &&
+          short.length <= NsmEmitter.MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING) {
+        int base63 = fromBase88(short);
+        int diff = base63 - previous;
+        previous = base63;
+        String base26Diff = toBase26(diff);
+        diffEncoding.write(base26Diff);
+      } else {
+        if (useDiffEncoding || diffEncoding.length != 0) {
+          diffEncoding.write(',');
+        }
+        diffEncoding.write(short);
+      }
+    }
+  }
+
+  String _computeDiffEncoding() {
+    StringBuffer buffer = new StringBuffer();
+    for (jsAst.ArrayInitializer list in ast.elements) {
+      if (buffer.isNotEmpty) {
+        if (useDiffEncoding) {
+          // Emit period that resets the diff base to zero when we switch to
+          // normal calling convention (this avoids the need to code negative
+          // diffs).
+          buffer.write(".");
+        } else {
+          // Write a separator for the next sequence.
+          buffer.write(",");
+        }
+      }
+      List<jsAst.Name> names = list.elements;
+      _computeDiffEncodingForList(names, buffer);
+    }
+    return buffer.toString();
+  }
+
+  String get value {
+    if (_cachedValue == null) {
+      _cachedValue = _computeDiffEncoding();
+    }
+
+    return _cachedValue;
+  }
+}