RR : Add a -draftCompile flag to trade output quality for reduced compile time

[BobV]

The attached patch adds a "-draftCompile" flag to the compiler.
Setting this flag will reduce Java optimizations to a single pass and
totally disable JS optimizations. A single-permutation compile of
Showcase gets about 25% faster on my MBP. A full 25-permutation
compile takes less than half of the time. The resulting JS is
intended to be good-enough for compile/test/fix cycles, but is
unsuitable for deployment. I ran the GWT unit tests with draft mode
enabled; tests that are unrelated to specific optimizations pass.

diffstat:
HostedModeBase.java | 2 2 + 0 - 0 !
Precompile.java | 14 12 + 2 - 0 !
jjs/JJSOptions.java | 5 3 + 2 - 0 !
jjs/JJSOptionsImpl.java | 10 10 + 0 - 0 !
jjs/JavaToJavaScriptCompiler.java | 13 11 + 2 - 0 !
util/arg/ArgHandlerDraftCompile.java | 49 49 + 0 - 0 !
util/arg/OptionDraftCompile.java | 26 26 + 0 - 0 !
7 files changed, 113 insertions(+), 6 deletions(-)

--
Bob Vawter
Google Web Toolkit Team

[Bruce Johnson]

Can we add a separate suite of automated test invocations for -draftCompile mode? It would be a nightmare if we had unrecognized breakages in -draftCompile mode.

[BobV]

On Fri, Jan 16, 2009 at 4:09 PM, Bruce Johnson <br...@google.com> wrote:
> Can we add a separate suite of automated test invocations for -draftCompile
> mode? It would be a nightmare if we had unrecognized breakages in
> -draftCompile mode.

Yes, I'll double-up the web mode tests, excluding those tests test
specific optimizations that don't happen in draftCompile.

[Ray Cromwell]

Awesome, I've been wanting to ask for this, as I'm already using single permutation compiles, but they still take 3 minutes.  I wonder how much further this can be taken to speed up the compile. If you look at how fast Javac or JDT can compile a few hundred classes, it seems there's alot of room left for improvement for GWT 3.0 :)

[Lex Spoon]

On Fri, Jan 16, 2009 at 4:09 PM, Bruce Johnson <br...@google.com> wrote:

> Can we add a separate suite of automated test invocations for -draftCompile
> mode? It would be a nightmare if we had unrecognized breakages in
> -draftCompile mode.

I've been thinking about this a few days, and I thought of one JS
"optimization" that we might want to leave on in draftMode:
JsStaticEval.EvalFunctionsAtTopScope. This visitor massages legal but
questionable JavaScript into something that will run in all browsers.

Despite this visitor being within JsStaticEval, I don't really think
it's an optimization. An optimization should transform legal code
into better legal code. This visitor translates not-so-legal code
into legal code. It's more of a normalizer.

Bob, do you have the list of tests handy that fail in draft mode? If
you don't, no worries. I can patch it in and run it myself.

-Lex

[Lex Spoon]

Okay, I have looked through all the tests that -draftCompile affects.
I'll write a full report, but there is one case where opinions would
really help.

The difficult case is that two test cases test that null+null
evaluates to "nullnull". With optimization on, this translation is
done within the compiler and it works. In -draftCompile, the
null+null expression goes all the way out to the browser, where it
evaluates to the number 0 (i.e., not even a string!).

In my opinion, it would be a great enhancement to GWT to actually
support this test case. However, we currently don't, and everyone
seems to live with it okay. So, I sugest that we disable it for now.
We could change the test to ""+null+null and add a big comment about
why it's done that way.

Once nullness tracking is in, we could revisit this case and add a ''+
in front of any string append that might need it. Without nullness
tracking, though, I fear quite a lot of useless ''+ would end up in
the output.

Thoughts?

-Lex

[Scott Blum]

That seems bad.... is it only null literals that evaluate to a number?

Code like this:

String x = staticallyUnevalableNull();

String y = staticallyUnevalableNull();

Window.alert(x + y);

Should produce:

"nullnull"

Not a number.  Sounds like we're making bad assumptions.

[Lex Spoon]

Here is an updated patch for -draftCompile. Everything it leaves alone LGTM.
That includes adding the new flag and updating the compiler to skip
optimizations when that flag is on. The handling of dead clinits was
a good catch, and also looks good.

Bob, can you review the new code in the updated patch, and/or make
the next round of changes? John and Scott, there are some changes
described below you might want to eyeball.

The one change to the compiler is that EvalFunctionsAtTopScope always
runs once, even with -draftCompile. It no longer runs in a loop,
because once it has run once no optimizer should recreate such code.

The patch also tweaks several test cases.

The most unclear test-case change is the one for testNull(). This
patch effectively disables it, based on the reasoning from my previous
email on this thread. I'm not sure what would be better.

The other test case changes look more clearly good to
me, but please do weigh in if anyone disagrees. They all
involve confusion between different numeric types,
especially float versus double. In all cases, I don't
think we really claim to support as good of discrimination as
the test case originally implied. I changed the test cases to still
test the intended feature while not being sensitive to these
numeric-type confusions.

Specifically:

StringBufferTest.testStringBuilder tests append of 1.0 and 1.0f. It's
a known quirk that this appends "1" instead of "1.0" in some contexts,
but optimization causes it to reliably emit "1.0" as in Java. I
changed the 1.0's to 1.5's.

I18NTest.testTypedMessages includes a double->float cast. These are
known not to be accurately supported in web mode, so the resulting
value can be sensitive to whether an optimizer fires. In this test, I
changed the test numbers 1.2 and 2.3 to 1.5 and 2.25, because the
latter are exactly represented in floating-point formats and so are
insensitive to float versus double weirdness.

Likewise, NumberFormat_en and NumberFormat_fr format the number
789.12345e-9. I changed it to 789.12346e-9 to get more reliable
rounding.

Likewise, CompilerTest.testStringOptimizations uses 3.3; I changed it to 3.5.

-Lex

[Lex Spoon]

On Wed, Jan 28, 2009 at 4:46 PM, Scott Blum <sco...@google.com> wrote:
> That seems bad.... is it only null literals that evaluate to a number?
> Code like this:
> String x = staticallyUnevalableNull();
> String y = staticallyUnevalableNull();
> Window.alert(x + y);
> Should produce:
> "nullnull"
> Not a number. Sounds like we're making bad assumptions.

Unfortunately, that is the situation. GenerateJavaScriptAST turns a
Java x+y to a JavaScript x+y, which does the wrong thing if x and y
are both null.

I've made a bug report, including a small repro test case, so that we
can schedule any work to be done on it.

http://code.google.com/p/google-web-toolkit/issues/detail?id=3320

I don't see an easy way to improve this. Do you? It seems like,
until we have nullness tracking, the compiler would have to add
useless ""+ to the output all over the place. Nullness tracking is
very close, but it needs a few free hours to push it over the hill.

Unless there's a really easy fix to this problem, I would argue that
we don't block -draftCompile waiting for it.

-Lex

[Scott Blum]

Agreed, especially since it's a general problem.