From e3102c5f6f0877eb1c5841e648625117c0ee0f6a Mon Sep 17 00:00:00 2001 From: dbulner Date: Wed, 14 Dec 2016 11:34:32 -0800 Subject: [PATCH] Automated g4 rollback of changelist 141926849. *** Reason for rollback *** Breakages *** Original change description *** Improve the performance of Disambiguate Properties when faced with large numbers of type invalidations. Avoid building the "JSType <--> JSError" map until we actual encounter an disambiguation failure that needs to be reported. *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=142042521 --- .../jscomp/DisambiguateProperties.java | 134 ++++++++---------- 1 file changed, 56 insertions(+), 78 deletions(-) diff --git a/src/com/google/javascript/jscomp/DisambiguateProperties.java b/src/com/google/javascript/jscomp/DisambiguateProperties.java index 20bd6faddc8..164c83f7a4e 100644 --- a/src/com/google/javascript/jscomp/DisambiguateProperties.java +++ b/src/com/google/javascript/jscomp/DisambiguateProperties.java @@ -118,7 +118,7 @@ static class Warnings { * Map of a type to all the related errors that invalidated the type * for disambiguation. */ - private Multimap invalidationMap; + private final Multimap invalidationMap; /** * In practice any large code base will have thousands and thousands of @@ -326,7 +326,8 @@ boolean scheduleRenaming(Node node, JSType type) { AbstractCompiler compiler, Map propertiesToErrorFor) { this.compiler = compiler; this.registry = compiler.getTypeRegistry(); - this.BOTTOM_OBJECT = this.registry.getNativeType(JSTypeNative.NO_OBJECT_TYPE).toObjectType(); + this.BOTTOM_OBJECT = + this.registry.getNativeType(JSTypeNative.NO_OBJECT_TYPE).toObjectType(); this.invalidatingTypes = new HashSet<>(ImmutableSet.of( registry.getNativeType(JSTypeNative.ALL_TYPE), registry.getNativeType(JSTypeNative.NO_OBJECT_TYPE), @@ -337,6 +338,11 @@ boolean scheduleRenaming(Node node, JSType type) { registry.getNativeType(JSTypeNative.TOP_LEVEL_PROTOTYPE), registry.getNativeType(JSTypeNative.UNKNOWN_TYPE))); this.propertiesToErrorFor = propertiesToErrorFor; + if (!this.propertiesToErrorFor.isEmpty()) { + this.invalidationMap = LinkedHashMultimap.create(); + } else { + this.invalidationMap = null; + } } @Override @@ -345,9 +351,18 @@ public void process(Node externs, Node root) { compiler.getLifeCycleStage() == LifeCycleStage.NORMALIZED); this.ancestorInterfaces = new HashMap<>(); this.gtwpCache = new HashMap<>(); - - recordInvalidations(); - + // TypeValidator records places where a type A is used in a context that + // expects a type B. + // For each pair (A, B), here we mark both A and B as types whose properties + // cannot be renamed. + for (TypeMismatch mis : compiler.getTypeMismatches()) { + recordInvalidatingType(mis.typeA, mis.src); + recordInvalidatingType(mis.typeB, mis.src); + } + for (TypeMismatch mis : compiler.getImplicitInterfaceUses()) { + recordInvalidatingType(mis.typeA, mis.src); + recordInvalidatingType(mis.typeB, mis.src); + } // Gather names of properties in externs; these properties can't be renamed. NodeTraversal.traverseEs6(compiler, externs, new FindExternProperties()); // Look at each unquoted property access and decide if that property will @@ -357,87 +372,45 @@ public void process(Node externs, Node root) { renameProperties(); } - abstract class InvalidationVisitor { - abstract void visit(JSType type, JSError src); - - void process(JSType type, JSError src) { - type = type.restrictByNotNullOrUndefined(); - if (type.isUnionType()) { - for (JSType alt : type.toMaybeUnionType().getAlternatesWithoutStructuralTyping()) { - process(alt, src); - } - } else if (type.isEnumElementType()) { - process(type.toMaybeEnumElementType().getPrimitiveType(), src); - } else { - visit(type, src); - - ObjectType objType = ObjectType.cast(type); - if (objType != null && objType.getImplicitPrototype() != null) { - process(objType.getImplicitPrototype(), src); - } - if (objType != null && objType.isConstructor() && objType.isFunctionType()) { - process(objType.toMaybeFunctionType().getInstanceType(), src); - } - } + private void recordInvalidationError(JSType t, JSError error) { + if (!t.isObject()) { + return; } - - void processAllTypeMismatches() { - // TypeValidator records places where a type A is used in a context that - // expects a type B. - // For each pair (A, B), here we mark both A and B as types whose properties - // cannot be renamed. - for (TypeMismatch mis : compiler.getTypeMismatches()) { - process(mis.typeA, mis.src); - process(mis.typeB, mis.src); - } - for (TypeMismatch mis : compiler.getImplicitInterfaceUses()) { - process(mis.typeA, mis.src); - process(mis.typeB, mis.src); + if (invalidationMap != null) { + Collection errors = this.invalidationMap.get(t); + if (errors.size() < MAX_INVALIDATION_WARNINGS_PER_PROPERTY) { + errors.add(error); } } } - class TypeInvalidationVisitor extends InvalidationVisitor { - @Override - void visit(JSType type, JSError src) { - // add the type to the invalidation set. - invalidatingTypes.add(type); - } - } - - public void recordInvalidations() { - // Uses an identity hash set to quickly remove duplicates. - TypeInvalidationVisitor visitor = new TypeInvalidationVisitor(); - visitor.processAllTypeMismatches(); - } - - class ErrorInvalidationVisitor extends InvalidationVisitor { - Multimap map = LinkedHashMultimap.create(); - - @Override - void visit(JSType type, JSError src) { - if (!type.isObject()) { - return; + /** + * Invalidates the given type, so that no properties on it will be renamed. + */ + private void recordInvalidatingType(JSType type, JSError error) { + type = type.restrictByNotNullOrUndefined(); + if (type.isUnionType()) { + for (JSType alt : type.toMaybeUnionType().getAlternatesWithoutStructuralTyping()) { + recordInvalidatingType(alt, error); } - Collection errors = map.get(type); - if (errors.size() < MAX_INVALIDATION_WARNINGS_PER_PROPERTY) { - errors.add(src); + } else if (type.isEnumElementType()) { + recordInvalidatingType( + type.toMaybeEnumElementType().getPrimitiveType(), error); + } else { + addInvalidatingType(type); + recordInvalidationError(type, error); + ObjectType objType = ObjectType.cast(type); + if (objType != null && objType.getImplicitPrototype() != null) { + addInvalidatingType(objType.getImplicitPrototype()); + recordInvalidationError(objType.getImplicitPrototype(), error); + } + if (objType != null + && objType.isConstructor() && objType.isFunctionType()) { + addInvalidatingType(objType.toMaybeFunctionType().getInstanceType()); } } } - private Multimap getInvalidationsErrors() { - Preconditions.checkState(!this.propertiesToErrorFor.isEmpty()); - if (this.invalidationMap == null) { - // Building the type invalidation map gets expensive for projects with large numbers - // of invalidations so we defer building this map as often we don't need it as - // reporting invalidation errors should be rare. - ErrorInvalidationVisitor visitor = new ErrorInvalidationVisitor(); - visitor.processAllTypeMismatches(); - this.invalidationMap = visitor.map; - } - return this.invalidationMap; - } /** Returns the property for the given name, creating it if necessary. */ protected Property getProperty(String name) { @@ -673,7 +646,7 @@ private void printErrorLocations(List errors, JSType t) { return; } - for (JSError error : getInvalidationsErrors().get(t)) { + for (JSError error : invalidationMap.get(t)) { if (error != null) { errors.add(t + " at " + error.sourceName + ":" + error.lineNumber); } @@ -821,6 +794,11 @@ Multimap> getRenamedTypesForTesting() { return ret; } + private void addInvalidatingType(JSType type) { + checkState(!type.isUnionType()); + invalidatingTypes.add(type); + } + private JSType getType(Node node) { if (node == null || node.getJSType() == null) { return registry.getNativeType(JSTypeNative.UNKNOWN_TYPE); @@ -1021,7 +999,7 @@ private JSType getInstanceFromPrototype(JSType type) { * recordInterface, and there was no speed-up. * And it made the code harder to understand, so we don't do it. */ - void recordInterfaces(FunctionType constructor, JSType relatedType, Property p) { + private void recordInterfaces(FunctionType constructor, JSType relatedType, Property p) { Iterable interfaces = ancestorInterfaces.get(constructor); if (interfaces == null) { interfaces = constructor.isConstructor()