Repository navigation
OptimizedOnlyCompilerSuite is never run, fails #10253
Description
Activity
Reverting cfb4e79 fixes the failing test, which suggests it is a matter of the MethodInliner racing against the "this call is clearly something the compiler can statically evaluate" check in DeadCodeElimination. The new implementation is probably "too easy" to inline, so the test started failing.
The order of the passes in optimizeJavaToFixedPoint and MethodInliner running in a loop with a DeadCodeElimination pass after each attempt appears to be an effort to avoid exactly this case - before every chance for MethodInliner to rewrite any method, static eval should get a chance to spot the
"1234".contains("1")and transform statically totrue. Watching the compiler pass by pass it appears this is what is happening:- initial state
- DCE can't statically evaluate
contains("1234", "1") - MethodInliner moves String.contains contents into the tests's NativeType.contains, so it now looks like
final boolean contains(String s1, String s2) { return s1.asNativeString().includes(s2); }
- DCE can't do anything to this
- MethodInliner can again inline this into alwaysCallNative(), which now can never be transformed away from a ternary expression
Previously however, the inliner rewrote the old indexof implementation instead, resulting in something like
final boolean contains(String s1, String s2) { return s1.indexOf(s2) != -1; }
and that in turn was inlined into alwaysCallNative() rather than having indexOf() itself be inlined - in short, we got lucky.
We should stick our thumbs on the scale a little more, and avoid this kind of luck where it prevents DCE from kicking in - String is a special class as far as DCE is concerned, and we should take greater pains to avoid inlining away real java.lang.String methods until we're quite confident that all other inlining and optimization has been performed. Something like a "@DoNotInlineQuiteSoAggressively" on all String methods that just delegate to NativeString methods. This might look a little like how method specialization works, where we remove all specialization markers and ask the compiler to inline everything one more time.
I'm hesitant to land a fix for this right before a release for fear that it might have some other side effect, but it might make for a good point release fix if we find it improves compiled size.
Given the context above, I'm going to limit this ticket to merely ensuring that this suite is run and disabling the failing test. The details of improving DCE will be moved to #10147.
- added a commit that references this issue
on Jan 24, 2026 I was wrong above - it wasn't (just) about the order of the inline operations, it was that contains(CharSequence) just can't be constant folded using the old implementation at all, as CharSequence isnt one of the supported types that could be passed in. But, when it was rewritten to be "1234".indexOf("1".toString()), the
"1".toString()could easily be rewritten to just a constant, and then "indexOf" could be constant folded.We won't be able to constant fold an arbitrary CharSequence param, only string literals can be supported - but once we're sure its a string literal, there's nothing wrong with supporting CharSequence.
Still should be part of 10147, just wanted to call out the mistake in case someone else tries to follow my notes here (like future me).
- added a commit that references this issue
on Jun 16, 2026 - added a commit that references this issue
on Aug 6, 2026
As far as I can tell, OptimizedOnlyCompilerSuite has never run as part of the ant build since it was introduced, it has always been in the
gwt.junit.testcase.web.excludeslist, which prevents it from running not only in draft mode but also prod tests. It was first introduced in 24de83e, and was in the excludes list from then on forward.Only one test fails:
gwt/user/test/com/google/gwt/dev/jjs/optimized/JsOverlayMethodOptimizationTest.java
Lines 29 to 65 in a433a55
The
contains()method is not statically evaluated as expected and the rest ofalwaysCallNativeends up not being inlined - instead we keep the overlap method (despite only having one invocation, which should make it an obvious candidate for js inlining).In the short term we could disable the test and enable the suite for non-draft "web" runs. Should also bisect to see when it started failing, if this points to something else being broken.