 Chromium Code Reviews
 Chromium Code Reviews Issue 11453032:
  Reapply class/method/field minification  (Closed) 
  Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
    
  
    Issue 11453032:
  Reapply class/method/field minification  (Closed) 
  Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart| Index: sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart | 
| diff --git a/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart b/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart | 
| index 506b8308671cb6bdf214912f30123949d428dec7..53f5d8f91d68b66f4a32dd5ce2d2c65996184ef9 100644 | 
| --- a/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart | 
| +++ b/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart | 
| @@ -110,32 +110,54 @@ class CodeEmitterTask extends CompilerTask { | 
| String get lazyInitializerName | 
| => '${namer.ISOLATE}.\$lazy'; | 
| - final String GETTER_SUFFIX = "?"; | 
| - final String SETTER_SUFFIX = "!"; | 
| - final String GETTER_SETTER_SUFFIX = "="; | 
| + // Property name suffixes. If the accessors are renaming then the format | 
| + // is <accessorName>:<fieldName><suffix>. We use the suffix to know whether | 
| + // to look for the ':' separator in order to avoid doing the indexOf operation | 
| + // on every single property (they are quite rare). None of these characters | 
| + // are legal in an identifier and they are related by bit patterns. | 
| + // setter < 0x3c | 
| + // both = 0x3d | 
| + // getter > 0x3e | 
| + // renaming setter | 0x7c | 
| + // renaming both } 0x7d | 
| + // renaming getter ~ 0x7e | 
| + const SUFFIX_MASK = 0x3f; | 
| + const FIRST_SUFFIX_CODE = 0x3c; | 
| + const SETTER_CODE = 0x3c; | 
| + const GETTER_SETTER_CODE = 0x3d; | 
| + const GETTER_CODE = 0x3e; | 
| + const RENAMING_FLAG = 0x40; | 
| + String needsGetterCode(String variable) => '($variable & 3) > 0'; | 
| + String needsSetterCode(String variable) => '($variable & 2) == 0'; | 
| + String isRenaming(String variable) => '($variable & $RENAMING_FLAG) != 0'; | 
| String get generateAccessorFunction { | 
| return """ | 
| function generateAccessor(field, prototype) { | 
| var len = field.length; | 
| - var lastChar = field[len - 1]; | 
| - var needsGetter = lastChar == '$GETTER_SUFFIX' || lastChar == '$GETTER_SETTER_SUFFIX'; | 
| - var needsSetter = lastChar == '$SETTER_SUFFIX' || lastChar == '$GETTER_SETTER_SUFFIX'; | 
| - if (needsGetter || needsSetter) field = field.substring(0, len - 1); | 
| - if (needsGetter) { | 
| - var getterString = "return this." + field + ";"; | 
| -""" | 
| - /* The supportsProtoCheck below depends on the getter/setter convention. | 
| - When changing here, update the protoCheck too. */ | 
| - """ | 
| - prototype["get\$" + field] = new Function(getterString); | 
| + var lastCharCode = field.charCodeAt(len - 1); | 
| + var needsAccessor = (lastCharCode & $SUFFIX_MASK) >= $FIRST_SUFFIX_CODE; | 
| + if (needsAccessor) { | 
| + var needsGetter = ${needsGetterCode('lastCharCode')}; | 
| + var needsSetter = ${needsSetterCode('lastCharCode')}; | 
| + var renaming = ${isRenaming('lastCharCode')}; | 
| + var accessorName = field = field.substring(0, len - 1); | 
| + if (renaming) { | 
| + var divider = field.indexOf(":"); | 
| + accessorName = field.substring(0, divider); | 
| + field = field.substring(divider + 1); | 
| + } | 
| + if (needsGetter) { | 
| + var getterString = "return this." + field + ";"; | 
| + prototype["get\$" + accessorName] = new Function(getterString); | 
| } | 
| if (needsSetter) { | 
| var setterString = "this." + field + " = v;"; | 
| - prototype["set\$" + field] = new Function("v", setterString); | 
| + prototype["set\$" + accessorName] = new Function("v", setterString); | 
| } | 
| - return field; | 
| - }"""; | 
| + } | 
| + return field; | 
| +}"""; | 
| } | 
| String get defineClassFunction { | 
| @@ -193,7 +215,7 @@ var $supportsProtoName = false; | 
| var tmp = $defineClassName('c', ['f?'], {}).prototype; | 
| if (tmp.__proto__) { | 
| tmp.__proto__ = {}; | 
| - if (typeof tmp.get\$f !== "undefined") $supportsProtoName = true; | 
| + if (typeof tmp.get\$f !== 'undefined') $supportsProtoName = true; | 
| } | 
| '''; | 
| } | 
| @@ -527,7 +549,7 @@ $lazyInitializerLogic | 
| // on A and a typed selector on B could yield the same stub. | 
| Set<String> generatedStubNames = new Set<String>(); | 
| if (compiler.enabledFunctionApply | 
| - && member.name == Namer.CLOSURE_INVOCATION_NAME) { | 
| + && member.name == namer.CLOSURE_INVOCATION_NAME) { | 
| // If [Function.apply] is called, we pessimistically compile all | 
| // possible stubs for this closure. | 
| FunctionSignature signature = member.computeSignature(compiler); | 
| @@ -622,7 +644,7 @@ $lazyInitializerLogic | 
| String compiledFieldName(Element member) { | 
| assert(member.isField()); | 
| return member.isNative() | 
| - ? member.name.slowToString() | 
| + ? member.nativeName() | 
| : namer.getName(member); | 
| } | 
| @@ -726,6 +748,7 @@ $lazyInitializerLogic | 
| void visitClassFields(ClassElement classElement, | 
| void addField(Element member, | 
| String name, | 
| + String accessorName, | 
| bool needsGetter, | 
| bool needsSetter, | 
| bool needsCheckedSetter)) { | 
| @@ -764,7 +787,7 @@ $lazyInitializerLogic | 
| ? namer.shadowedFieldName(member) | 
| : namer.getName(member); | 
| String fieldName = member.isNative() | 
| - ? member.nativeName() | 
| + ? member.nativeName() | 
| 
floitsch
2012/12/07 12:01:01
accidental space?
 
erikcorry
2012/12/10 08:36:01
Done.
 | 
| : accessorName; | 
| bool needsCheckedSetter = false; | 
| if (needsSetter && compiler.enableTypeAssertions | 
| @@ -775,6 +798,7 @@ $lazyInitializerLogic | 
| // Getters and setters with suffixes will be generated dynamically. | 
| addField(member, | 
| fieldName, | 
| + accessorName, | 
| needsGetter, | 
| needsSetter, | 
| needsCheckedSetter); | 
| @@ -791,16 +815,6 @@ $lazyInitializerLogic | 
| includeSuperMembers: isInstantiated && !classElement.isNative()); | 
| } | 
| - void generateGetter(Element member, String fieldName, CodeBuffer buffer) { | 
| - String getterName = namer.getterName(member.getLibrary(), member.name); | 
| - buffer.add("$getterName: function() { return this.$fieldName; }"); | 
| - } | 
| - | 
| - void generateSetter(Element member, String fieldName, CodeBuffer buffer) { | 
| - String setterName = namer.setterName(member.getLibrary(), member.name); | 
| - buffer.add("$setterName: function(v) { this.$fieldName = v; }"); | 
| - } | 
| - | 
| bool canGenerateCheckedSetter(Element member) { | 
| DartType type = member.computeType(compiler); | 
| if (type.element.isTypeVariable() | 
| @@ -814,6 +828,7 @@ $lazyInitializerLogic | 
| void generateCheckedSetter(Element member, | 
| String fieldName, | 
| + String accessorName, | 
| CodeBuffer buffer) { | 
| assert(canGenerateCheckedSetter(member)); | 
| DartType type = member.computeType(compiler); | 
| @@ -824,7 +839,8 @@ $lazyInitializerLogic | 
| if (helperElement.computeSignature(compiler).parameterCount != 1) { | 
| additionalArgument = ", '${namer.operatorIs(type.element)}'"; | 
| } | 
| - String setterName = namer.setterName(member.getLibrary(), member.name); | 
| + String setterName = | 
| + namer.setterNameFromAccessorName(accessorName); | 
| 
sra1
2012/12/07 06:03:18
Single line?
 
erikcorry
2012/12/10 08:36:01
Done.
 | 
| buffer.add("$setterName: function(v) { " | 
| "this.$fieldName = $helperName(v$additionalArgument); }"); | 
| } | 
| @@ -846,13 +862,10 @@ $lazyInitializerLogic | 
| } | 
| visitClassFields(classElement, (Element member, | 
| String name, | 
| + String accessorName, | 
| bool needsGetter, | 
| bool needsSetter, | 
| bool needsCheckedSetter) { | 
| - if (!getterAndSetterCanBeImplementedByFieldSpec( | 
| - member, name, needsGetter, needsSetter)) { | 
| - return; | 
| - } | 
| if (!isNative || needsCheckedSetter || needsGetter || needsSetter) { | 
| if (isFirstField) { | 
| isFirstField = false; | 
| @@ -863,13 +876,19 @@ $lazyInitializerLogic | 
| } else { | 
| buffer.add(","); | 
| } | 
| - buffer.add('$name'); | 
| + buffer.add('$accessorName'); | 
| + int flag = 0; | 
| + if (name != accessorName) { | 
| + buffer.add(':$name'); | 
| + assert(needsGetter || needsSetter); | 
| + flag = RENAMING_FLAG; | 
| + } | 
| if (needsGetter && needsSetter) { | 
| - buffer.add(GETTER_SETTER_SUFFIX); | 
| + buffer.addCharCode(GETTER_SETTER_CODE + flag); | 
| } else if (needsGetter) { | 
| - buffer.add(GETTER_SUFFIX); | 
| + buffer.addCharCode(GETTER_CODE + flag); | 
| } else if (needsSetter) { | 
| - buffer.add(SETTER_SUFFIX); | 
| + buffer.addCharCode(SETTER_CODE + flag); | 
| } | 
| } | 
| }); | 
| @@ -895,48 +914,18 @@ $lazyInitializerLogic | 
| visitClassFields(classElement, (Element member, | 
| String name, | 
| + String accessorName, | 
| bool needsGetter, | 
| bool needsSetter, | 
| bool needsCheckedSetter) { | 
| - if (name == null) throw 123; | 
| - if (getterAndSetterCanBeImplementedByFieldSpec( | 
| - member, name, needsGetter, needsSetter)) { | 
| - needsGetter = false; | 
| - needsSetter = false; | 
| - } | 
| - if (needsGetter) { | 
| - emitComma(); | 
| - generateGetter(member, name, buffer); | 
| - } | 
| - if (needsSetter) { | 
| - emitComma(); | 
| - generateSetter(member, name, buffer); | 
| - } | 
| if (needsCheckedSetter) { | 
| assert(!needsSetter); | 
| emitComma(); | 
| - generateCheckedSetter(member, name, buffer); | 
| + generateCheckedSetter(member, name, accessorName, buffer); | 
| } | 
| }); | 
| } | 
| - bool getterAndSetterCanBeImplementedByFieldSpec(Element member, | 
| - String name, | 
| - bool needsGetter, | 
| - bool needsSetter) { | 
| - if (needsGetter) { | 
| - if (namer.getterName(member.getLibrary(), member.name) != 'get\$$name') { | 
| - return false; | 
| - } | 
| - } | 
| - if (needsSetter) { | 
| - if (namer.setterName(member.getLibrary(), member.name) != 'set\$$name') { | 
| - return false; | 
| - } | 
| - } | 
| - return true; | 
| - } | 
| - | 
| /** | 
| * Documentation wanted -- johnniwinther | 
| * | 
| @@ -1158,7 +1147,7 @@ $lazyInitializerLogic | 
| // create a fake element with the correct name. | 
| // Note: the callElement will not have any enclosingElement. | 
| FunctionElement callElement = | 
| - new ClosureInvocationElement(Namer.CLOSURE_INVOCATION_NAME, element); | 
| + new ClosureInvocationElement(namer.CLOSURE_INVOCATION_NAME, element); | 
| 
kasperl
2012/12/07 10:37:45
It looks a bit fishy to try to grab a constant (we
 
erikcorry
2012/12/10 08:36:01
Done and changed:
ISOLATE -> isolateName
ISOLATE_
 | 
| String staticName = namer.getName(element); | 
| String invocationName = namer.instanceMethodName(callElement); | 
| String fieldAccess = '$isolateProperties.$staticName'; | 
| @@ -1255,8 +1244,8 @@ $classesCollector.$mangledName = {'': | 
| // its stubs we simply create a fake element with the correct name. | 
| // Note: the callElement will not have any enclosingElement. | 
| FunctionElement callElement = | 
| - new ClosureInvocationElement(Namer.CLOSURE_INVOCATION_NAME, member); | 
| - | 
| + new ClosureInvocationElement(namer.CLOSURE_INVOCATION_NAME, member); | 
| + | 
| String invocationName = namer.instanceMethodName(callElement); | 
| List<String> arguments = new List<String>(parameterCount); | 
| for (int i = 0; i < parameterCount; i++) { | 
| @@ -1326,7 +1315,7 @@ $classesCollector.$mangledName = {'': | 
| String invocationName = | 
| namer.instanceMethodInvocationName(memberLibrary, member.name, | 
| selector); | 
| - SourceString callName = Namer.CLOSURE_INVOCATION_NAME; | 
| + SourceString callName = namer.CLOSURE_INVOCATION_NAME; | 
| String closureCallName = | 
| namer.instanceMethodInvocationName(memberLibrary, callName, | 
| selector); | 
| @@ -1523,11 +1512,14 @@ $classesCollector.$mangledName = {'': | 
| } | 
| String internalName = namer.instanceMethodInvocationName( | 
| selector.library, new SourceString(methodName), selector); | 
| + Element createInvocationMirror = | 
| + compiler.findHelper(const SourceString('createInvocationMirror')); | 
| CodeBuffer buffer = new CodeBuffer(); | 
| buffer.add('function($args) {\n'); | 
| buffer.add(' return this.$noSuchMethodName(' | 
| - '\$.createInvocationMirror("$methodName", "$internalName",' | 
| - ' $type, [$args], [$argNames]));\n'); | 
| + '${namer.isolateAccess(createInvocationMirror)}(' | 
| + '"$methodName", "$internalName",' | 
| + '$type, [$args], [$argNames]));\n'); | 
| buffer.add(' }'); | 
| return buffer; | 
| } | 
| @@ -1700,10 +1692,10 @@ $mainEnsureGetter | 
| // | 
| // BEGIN invoke [main]. | 
| // | 
| -if (typeof document != 'undefined' && document.readyState != 'complete') { | 
| +if (typeof document !== 'undefined' && document.readyState != 'complete') { | 
| 
floitsch
2012/12/07 12:01:01
not sure that's needed. In theory it should be cle
 
erikcorry
2012/12/10 08:36:01
Done.
 | 
| document.addEventListener('readystatechange', function () { | 
| if (document.readyState == 'complete') { | 
| - if (typeof dartMainRunner == 'function') { | 
| + if (typeof dartMainRunner === 'function') { | 
| dartMainRunner(function() { ${mainCall}; }); | 
| } else { | 
| ${mainCall}; | 
| @@ -1711,7 +1703,7 @@ if (typeof document != 'undefined' && document.readyState != 'complete') { | 
| } | 
| }, false); | 
| } else { | 
| - if (typeof dartMainRunner == 'function') { | 
| + if (typeof dartMainRunner === 'function') { | 
| dartMainRunner(function() { ${mainCall}; }); | 
| } else { | 
| ${mainCall}; |