Skip to content

Minify boolean and numeric literals in obfuscated mode #10291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zbynek wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Minify boolean and numeric literals in obfuscated mode #10291

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f50f053
Minify boolean and numeric literals in obfuscated mode
zbynek Mar 5, 2026
edd9445
Avoid String to char[] conversion, remove log message
zbynek Mar 6, 2026
271e29c
Avoid double serialization, consistent naming
zbynek Mar 6, 2026
7adbee5
Fix failing test
zbynek Mar 10, 2026
cca743f
Inline short bool values
zbynek Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
import com.google.gwt.dev.js.JsStackEmulator;
import com.google.gwt.dev.js.JsStaticEval;
import com.google.gwt.dev.js.JsSymbolResolver;
import com.google.gwt.dev.js.JsToStringGenerationVisitor;
import com.google.gwt.dev.js.JsUnusedFunctionRemover;
import com.google.gwt.dev.js.JsVerboseNamer;
import com.google.gwt.dev.js.SizeBreakdown;
Expand Down Expand Up @@ -772,7 +773,9 @@
DefaultTextOutput out = new DefaultTextOutput(!options.isIncrementalCompileEnabled() &&
options.getOutput().shouldMinimize());
JsReportGenerationVisitor v = new JsReportGenerationVisitor(out, jjsMap,
options.isJsonSoycEnabled());
options.isJsonSoycEnabled(),
new JsToStringGenerationVisitor.PrintOptions(false,
options.getOutput() == JsOutputOption.OBFUSCATED));
v.accept(jsProgram.getFragmentBlock(i));

StatementRanges statementRanges = v.getStatementRanges();
Expand Down Expand Up @@ -1479,6 +1482,7 @@

nodeCount = jprogram.getNodeCount();
mods = stats.getNumMods();
logger.log(TreeLogger.Type.TRACE, "Optimization pass " + passCount + " of " + passLimit + "\n" + stats);

Check warning on line 1485 in dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java

View workflow job for this annotation

GitHub Actions / build (21) 

[checkstyle] reported by reviewdog 🐶
Line is longer than 100 characters (found 112).

Raw Output:
/home/runner/work/gwt/gwt/gwt/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1485:0: warning: Line is longer than 100 characters (found 112). (com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck)
Comment thread
zbynek marked this conversation as resolved.
Outdated
}

float nodeChangeRate = mods / (float) lastNodeCount;
Expand Down
4 changes: 2 additions & 2 deletions dev/core/src/com/google/gwt/dev/js/JsReportGenerationVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ public class JsReportGenerationVisitor extends
private List<JsNode> parentStack = Lists.newArrayList();

public JsReportGenerationVisitor(TextOutput out, JavaToJavaScriptMap map,
boolean needSourcemapNames) {
super(out, map);
boolean needSourcemapNames, PrintOptions options) {
super(out, map, options);
this.out = out;
this.needSourcemapNames = needSourcemapNames;
}
Expand Down
6 changes: 3 additions & 3 deletions dev/core/src/com/google/gwt/dev/js/JsSourceGenerationVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ public JsSourceGenerationVisitor(TextOutput out) {
/**
* Generate the output source code using short or long identifiers.
*
* @param useLongIdents if true, emit all identifiers in long form
* @param options minification options
*/
public JsSourceGenerationVisitor(TextOutput out, boolean useLongIdents) {
super(out, useLongIdents);
public JsSourceGenerationVisitor(TextOutput out, PrintOptions options) {
super(out, options);
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions dev/core/src/com/google/gwt/dev/js/JsSourceGenerationVisitorWithSizeBreakdown.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public class JsSourceGenerationVisitorWithSizeBreakdown extends JsSourceGenerati
private final Map<JsName, Integer> sizeMap = new HashMap<JsName, Integer>();

public JsSourceGenerationVisitorWithSizeBreakdown(TextOutput out,
JavaToJavaScriptMap javaToJavaScriptMap) {
super(out);
JavaToJavaScriptMap javaToJavaScriptMap, PrintOptions options) {
super(out, options);
this.out = out;
this.map = javaToJavaScriptMap;
}
Expand Down
111 changes: 86 additions & 25 deletions dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ public class JsToStringGenerationVisitor extends JsVisitor {
* How many lines of code to print inside of a JsBlock when printing terse.
*/
private static final int JSBLOCK_LINES_TO_PRINT = 3;
private static final long MAX_DECIMAL_VALUE = 999_999_999_999L;

protected boolean needSemi = true;
private List<NamedRange> classRanges = new ArrayList<NamedRange>();
private final List<NamedRange> classRanges = new ArrayList<>();
private NamedRange currentClassRange;
private NamedRange programClassRange;

Expand All @@ -127,27 +128,39 @@ public class JsToStringGenerationVisitor extends JsVisitor {
* because the statements designated by statementEnds and statementStarts are
* those that appear directly within these global blocks.
*/
private Set<JsBlock> globalBlocks = new HashSet<JsBlock>();
private final Set<JsBlock> globalBlocks = new HashSet<>();
private final TextOutput p;
private ArrayList<Integer> statementEnds = new ArrayList<Integer>();
private ArrayList<Integer> statementStarts = new ArrayList<Integer>();
private final ArrayList<Integer> statementEnds = new ArrayList<>();
private final ArrayList<Integer> statementStarts = new ArrayList<>();
private final boolean useLongIdents;
private final boolean minifyLiterals;

public static class PrintOptions {
public final boolean useLongIdents;
public final boolean minifyLiterals;

public PrintOptions(boolean useLongIdents, boolean minifyLiterals) {
this.useLongIdents = useLongIdents;
this.minifyLiterals = minifyLiterals;
}
}

/**
* Generate the output string using short identifiers.
*/
public JsToStringGenerationVisitor(TextOutput out) {
this(out, false);
this(out, new PrintOptions(false, false));
}

/**
* Generate the output string using short or long identifiers.
*
* @param useLongIdents if true, emit all identifiers in long form
* @param options settings for minification
*/
JsToStringGenerationVisitor(TextOutput out, boolean useLongIdents) {
JsToStringGenerationVisitor(TextOutput out, PrintOptions options) {
this.p = out;
this.useLongIdents = useLongIdents;
this.useLongIdents = options.useLongIdents;
this.minifyLiterals = options.minifyLiterals;
}

public List<NamedRange> getClassRanges() {
Expand Down Expand Up @@ -183,8 +196,7 @@ public boolean visit(JsArrayAccess x, JsContext ctx) {
public boolean visit(JsArrayLiteral x, JsContext ctx) {
_lsquare();
boolean sep = false;
for (Object element : x.getExpressions()) {
JsExpression arg = (JsExpression) element;
for (JsExpression arg : x.getExpressions()) {
sep = _sepCommaOptSpace(sep);
_parenPushIfCommaExpr(arg);
accept(arg);
Expand Down Expand Up @@ -227,6 +239,10 @@ public boolean visit(JsBlock x, JsContext ctx) {

@Override
public boolean visit(JsBooleanLiteral x, JsContext ctx) {
if (minifyLiterals) {
p.print(x.getValue() ? "!0" : "!1");
return false;
}
if (x.getValue()) {
_true();
} else {
Expand Down Expand Up @@ -257,8 +273,7 @@ public boolean visit(JsCase x, JsContext ctx) {
_newlineOpt();

indent();
for (Object element : x.getStmts()) {
JsStatement stmt = (JsStatement) element;
for (JsStatement stmt : x.getStmts()) {
needSemi = true;
accept(stmt);
if (needSemi) {
Expand Down Expand Up @@ -387,8 +402,7 @@ public boolean visit(JsDefault x, JsContext ctx) {
_colon();

indent();
for (Object element : x.getStmts()) {
JsStatement stmt = (JsStatement) element;
for (JsStatement stmt : x.getStmts()) {
needSemi = true;
accept(stmt);
if (needSemi) {
Expand Down Expand Up @@ -532,8 +546,7 @@ public boolean visit(JsFunction x, JsContext ctx) {

_lparen();
boolean sep = false;
for (Object element : x.getParameters()) {
JsParameter param = (JsParameter) element;
for (JsParameter param : x.getParameters()) {
sep = _sepCommaOptSpace(sep);
accept(param);
}
Expand Down Expand Up @@ -588,8 +601,7 @@ public boolean visit(JsInvocation x, JsContext ctx) {

_lparen();
boolean sep = false;
for (Object element : x.getArguments()) {
JsExpression arg = (JsExpression) element;
for (JsExpression arg : x.getArguments()) {
sep = _sepCommaOptSpace(sep);
_parenPushIfCommaExpr(arg);
accept(arg);
Expand Down Expand Up @@ -626,7 +638,7 @@ public boolean visit(JsNameRef x, JsContext ctx) {
_parenPush(x, q, false);
accept(q);
if (q instanceof JsNumberLiteral) {
/**
/*
* Fix for Issue #3796. "42.foo" is not allowed, but "42 .foo" is.
*/
_space();
Expand Down Expand Up @@ -681,20 +693,51 @@ public boolean visit(JsNullLiteral x, JsContext ctx) {

@Override
public boolean visit(JsNumberLiteral x, JsContext ctx) {
String val = stringifyNumber(x);
p.print(val);
return false;
}

private String stringifyNumber(JsNumberLiteral x) {
double dvalue = x.getValue();
if (dvalue == 0.0 && 1.0 / dvalue == Double.NEGATIVE_INFINITY) {
// Negative zero is distinct from 0.0 and (integer) 0
p.print("-0.");
return false;
return "-0";
}

long lvalue = (long) dvalue;
if (lvalue == dvalue) {
p.print(Long.toString(lvalue));
String longVal = Long.toString(lvalue);
if (minifyLiterals && lvalue != 0) {
int trailingZeros = numberOfTrailingDecZeros(longVal);
if (trailingZeros > 2) {
// print 1000 as 1e3, keep 100 as is
longVal = longVal.substring(0, longVal.length() - trailingZeros) + "e" + trailingZeros;
} else if (Math.abs(lvalue) > MAX_DECIMAL_VALUE) {
// from 1e12 we may save 1 or 2 bytes by using the hex code
longVal = (lvalue < 0 ? "-0x" : "0x") + Long.toString(Math.abs(lvalue), 16);
}
}
return longVal;
} else {
p.print(Double.toString(dvalue));
String doubleVal = Double.toString(dvalue);
if (minifyLiterals) {
if (doubleVal.startsWith("0.")) {
doubleVal = doubleVal.substring(1);
} else if (doubleVal.startsWith("-0.")) {
doubleVal = "-" + doubleVal.substring(2);
}
}
return doubleVal;
}
return false;
}

private int numberOfTrailingDecZeros(String longVal) {
int idx = longVal.length() - 1;
while (longVal.charAt(idx) == '0') {
idx--;
}
return longVal.length() - idx - 1;
}

@Override
Expand Down Expand Up @@ -790,7 +833,11 @@ public boolean visit(JsReturn x, JsContext ctx) {
_return();
JsExpression expr = x.getExpr();
if (expr != null) {
_space();
if (spaceReturn(expr)) {
_space();
} else {
_spaceOpt();
}
Copy link
Copy Markdown
Member

@niloc132 niloc132 Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a great solution for this, but I just wanted to call out briefly that this seems gross - we have to toString the child node twice, so that we know how to concat them together. Perf-wise (though probably not logic-wise) it would seem better to just go ahead and not accept(expr) if we know it is a literal, but append directly.

Better still would be to signal to the TextOutput "hey you already know if space is optional or not... I just finished writing a keyword, make sure the next token gets a space if it needs it". Then, when the next token is written, TextOutput decides if it needs a space?

One missing case in the current impl is to do with parens - in the terser sample you showed me, there are some cases of extra parens being required in order to maintain operator precedence:

return(t=h.h>>19)!=(u=n.h>>19)?...

Copy link
Copy Markdown
Collaborator Author

@zbynek zbynek Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perf-wise (though probably not logic-wise) it would seem better to just go ahead and not accept(expr) if we know it is a literal

That's implemented in my last commit.

One missing case in the current impl is to do with parens

I don't have a good solution for that -- would it be OK to leave that for another time?

Copy link
Copy Markdown
Member

@niloc132 niloc132 Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think we wait, rethink that as we implement other size enhancements.

Copy link
Copy Markdown
Collaborator Author

@zbynek zbynek Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, so we need to accept the literal node so that subclasses can handle it (for billing). For now I reverted to the duplicated serialization -- we could introduce a flag that lets us accept the node without printing, but it's not obvious if that can save time.

accept(expr);
}
return false;
Expand Down Expand Up @@ -1275,6 +1322,20 @@ private boolean _spaceCalc(JsOperator op, JsExpression arg) {
return false;
}

private boolean spaceReturn(JsExpression arg) {
Comment thread
zbynek marked this conversation as resolved.
Outdated
if (arg instanceof JsBooleanLiteral) {
return !minifyLiterals;
}
if (arg instanceof JsPrefixOperation) {
return ((JsPrefixOperation) arg).getOperator().isKeyword();
}
if (arg instanceof JsNumberLiteral) {
String value = stringifyNumber((JsNumberLiteral) arg);
return value.charAt(0) != '-' && value.charAt(0) != '.';
}
return true;
}

private void _spaceOpt() {
p.printOpt(' ');
}
Expand Down
3 changes: 2 additions & 1 deletion dev/core/src/com/google/gwt/dev/js/ast/JsNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ public final String toSource() {
*/
public final String toSource(boolean useLongIdents) {
DefaultTextOutput out = new DefaultTextOutput(false);
JsSourceGenerationVisitor v = new JsSourceGenerationVisitor(out, useLongIdents);
JsSourceGenerationVisitor v = new JsSourceGenerationVisitor(out,
new JsToStringGenerationVisitor.PrintOptions(useLongIdents, false));
v.accept(this);
return out.toString();
}
Expand Down
6 changes: 1 addition & 5 deletions dev/core/src/com/google/gwt/dev/util/AbstractTextOutput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,7 @@ public void indentOut() {

@Override
public void newline() {
if (compact) {
out.print('\n');
} else {
out.print('\n');
}
out.print('\n');
Comment thread
zbynek marked this conversation as resolved.
position++;
line++;
column = 0;
Expand Down
3 changes: 2 additions & 1 deletion dev/core/test/com/google/gwt/dev/js/JsReportGenerationVisitorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ private void checkMappings(String ...expectedLines)
throws IOException, JsParserException {
DefaultTextOutput text = new DefaultTextOutput(compact);
JsReportGenerationVisitor generator = new JsReportGenerationVisitor(text,
JavaToJavaScriptMap.EMPTY, false) {
JavaToJavaScriptMap.EMPTY, false,
new JsToStringGenerationVisitor.PrintOptions(false, false)) {
@Override
boolean surroundsInJavaSource(SourceInfo parent, SourceInfo child) {
// The Rhino-based JavaScript parser doesn't provide character ranges
Expand Down
Loading
Loading