Skip to content

Commit

Permalink
Automated g4 rollback of changelist 141926849.
Browse files Browse the repository at this point in the history 
*** 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
  • Loading branch information
@blickly
dbulner authored and blickly committed Dec 15, 2016
1 parent c584457 commit e3102c5
Showing 1 changed file with 56 additions and 78 deletions.
134 changes: 56 additions & 78 deletions src/com/google/javascript/jscomp/DisambiguateProperties.java
View file
Expand Up @@ -118,7 +118,7 @@ static class Warnings {
* Map of a type to all the related errors that invalidated the type
* for disambiguation.
*/
private Multimap<JSType, JSError> invalidationMap;
private final Multimap<JSType, JSError> invalidationMap;

/**
* In practice any large code base will have thousands and thousands of
Expand Down Expand Up @@ -326,7 +326,8 @@ boolean scheduleRenaming(Node node, JSType type) {
AbstractCompiler compiler, Map<String, CheckLevel> 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),
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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<JSError> 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<JSType, JSError> 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<JSError> 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<JSType, JSError> 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) {
Expand Down Expand Up @@ -673,7 +646,7 @@ private void printErrorLocations(List<String> 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);
}
Expand Down Expand Up @@ -821,6 +794,11 @@ Multimap<String, Collection<JSType>> 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);
Expand Down Expand Up @@ -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<ObjectType> interfaces = ancestorInterfaces.get(constructor);
if (interfaces == null) {
interfaces = constructor.isConstructor()
Expand Down

0 comments on commit e3102c5

Please sign in to comment.