Skip to content

Commit

Permalink
Make J2clConstantHoisterPass optimize stub static field initializations
Browse files Browse the repository at this point in the history
e.g.:

function clinit() {
  foo = true;
}
var foo;

Also optimize the case where the initialization comes before the clinit.
e.g.:

var foo;
function clinit() {
  foo = true;
}

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=175738294
  • Loading branch information
Dominator008 authored and Tyler Breisacher committed Nov 14, 2017
1 parent 9dc0353 commit 5e4850c
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 25 deletions.
58 changes: 33 additions & 25 deletions src/com/google/javascript/jscomp/J2clConstantHoisterPass.java
Expand Up @@ -87,65 +87,73 @@ private void maybeHoistClassField(
return;
}

Node firstAssignment = Iterables.get(assignments, 0);
Node secondAssignment = Iterables.get(assignments, 1);

// One of them is for field initialization in declaration phase:
if (!isClassFieldInitialization(secondAssignment)) {
Node first = Iterables.get(assignments, 0);
Node second = Iterables.get(assignments, 1);

// One of them is the top level declaration and the other is the assignment in clinit.
Node topLevelDeclaration = isClassFieldDeclaration(first) ? first : second;
Node clinitAssignment = isClinitFieldAssignment(first) ? first : second;
if (!isClassFieldDeclaration(topLevelDeclaration)
|| !isClinitFieldAssignment(clinitAssignment)) {
return;
}

// The other one is the clinit initialization:
if (!isClinitFieldAssignment(firstAssignment)) {
return;
}
// And it is assigned to a literal value; hence could be used in static eval and safe to move:
Node firstAssignmentRhs = firstAssignment.getSecondChild();
if (!NodeUtil.isLiteralValue(firstAssignmentRhs, true /* includeFunctions */)
|| (firstAssignmentRhs.isFunction() && !hoistableFunctions.contains(firstAssignmentRhs))) {
Node assignmentRhs = clinitAssignment.getSecondChild();
if (!NodeUtil.isLiteralValue(assignmentRhs, true /* includeFunctions */)
|| (assignmentRhs.isFunction() && !hoistableFunctions.contains(assignmentRhs))) {
return;
}

// And the assignment are in the same script:
if (NodeUtil.getEnclosingScript(firstAssignment)
!= NodeUtil.getEnclosingScript(secondAssignment)) {
if (NodeUtil.getEnclosingScript(clinitAssignment)
!= NodeUtil.getEnclosingScript(topLevelDeclaration)) {
return;
}

// At this point the only case some could observe the declaration value is the when you have
// cycle between clinits and the field is accessed before initialization; which is almost always
// a bug and GWT never assumed this state is observable in its optimization, yet nobody
// complained. So it is safe to upgrade it to a constant.
hoistConstantLikeField(firstAssignment, secondAssignment);
hoistConstantLikeField(clinitAssignment, topLevelDeclaration);
}

private void hoistConstantLikeField(Node clinitAssignment, Node declarationAssignment) {
private void hoistConstantLikeField(Node clinitAssignment, Node topLevelDeclaration) {
Node clinitAssignedValue = clinitAssignment.getSecondChild();
Node declarationInClass = declarationAssignment.getFirstChild();
Node declarationInClass = topLevelDeclaration.getFirstChild();
Node declarationAssignedValue = declarationInClass.getFirstChild();

Node clinitChangeScope = NodeUtil.getEnclosingChangeScopeRoot(clinitAssignment);
// Remove the clinit initialization
NodeUtil.removeChild(clinitAssignment.getParent(), clinitAssignment);


// Replace the assignment in declaration with the value from clinit
clinitAssignedValue.detach();
compiler.reportChangeToChangeScope(clinitChangeScope);
if (!declarationAssignedValue.isEquivalentTo(clinitAssignedValue)) {
if (declarationAssignedValue == null) {
// Add the value from clinit to the stub declaration
declarationInClass.addChildToFront(clinitAssignedValue);
compiler.reportChangeToEnclosingScope(topLevelDeclaration);
} else if (!declarationAssignedValue.isEquivalentTo(clinitAssignedValue)) {
checkState(NodeUtil.isLiteralValue(declarationAssignedValue, false /* includeFunctions */));
// Replace the assignment in declaration with the value from clinit
declarationInClass.replaceChild(declarationAssignedValue, clinitAssignedValue);
compiler.reportChangeToEnclosingScope(declarationAssignment);
compiler.reportChangeToEnclosingScope(topLevelDeclaration);
}
declarationInClass.putBooleanProp(Node.IS_CONSTANT_VAR, true);

checkState(NodeUtil.isLiteralValue(declarationAssignedValue, false /* includeFunctions */));
}

private static boolean isClassFieldInitialization(Node node) {
/**
* Matches literal value declarations {@code var foo = 3;} or stub declarations like
* {@code var bar;}.
*/
private static boolean isClassFieldDeclaration(Node node) {
return node.getParent().isScript()
&& node.isVar()
&& node.getFirstFirstChild() != null
&& NodeUtil.isLiteralValue(node.getFirstFirstChild(), false /* includeFunctions */);
&& (!node.getFirstChild().hasChildren()
|| (node.getFirstFirstChild() != null
&& NodeUtil.isLiteralValue(
node.getFirstFirstChild(), false /* includeFunctions */)));
}

private static boolean isClinitFieldAssignment(Node node) {
Expand Down
46 changes: 46 additions & 0 deletions test/com/google/javascript/jscomp/J2clConstantHoisterPassTest.java
Expand Up @@ -52,6 +52,52 @@ public void testHoistClinitConstantAssignments() {
"var someClass$buzz = function(){ return someClass$bar; };"));
}

public void testHoistClinitConstantAssignments_assignmentAfterInit() {
test(
LINE_JOINER.join(
"var someClass = /** @constructor */ function() {};",
"var someClass$foo = false;",
"var someClass$bar = null;",
"var someClass$buzz = null;",
"someClass.$clinit = function() {",
" someClass.$clinit = function() {};",
" someClass$foo = true;",
" someClass$bar = 'hey';",
" someClass$buzz = function() { return someClass$bar; };",
"};"),
LINE_JOINER.join(
"var someClass = /** @constructor */ function() {};",
"var someClass$foo = true;",
"var someClass$bar = 'hey';",
"var someClass$buzz = function(){ return someClass$bar; };",
"someClass.$clinit = function() {",
" someClass.$clinit = function() {};",
"};"));
}

public void testHoistClinitConstantAssignments_stubInit() {
test(
LINE_JOINER.join(
"var someClass = /** @constructor */ function() {};",
"someClass.$clinit = function() {",
" someClass.$clinit = function() {};",
" someClass$foo = true;",
" someClass$bar = 'hey';",
" someClass$buzz = function() { return someClass$bar; };",
"};",
"var someClass$foo = false;",
"var someClass$bar;",
"var someClass$buzz;"),
LINE_JOINER.join(
"var someClass = /** @constructor */ function() {};",
"someClass.$clinit = function() {",
" someClass.$clinit = function() {};",
"};",
"var someClass$foo = true;",
"var someClass$bar = 'hey';",
"var someClass$buzz = function(){ return someClass$bar; };"));
}

public void testHoistClinitConstantAssignments_avoidUnsafe() {
testSame(
lines(
Expand Down

0 comments on commit 5e4850c

Please sign in to comment.