Improve Objects.equal specialization

[niloc132]

The static method Objects.equals(Object,Object) emulation has a non-standard overload equals(String,String), intended to capture via Java's standard compile time polymorphism. This is great when the two arguments are declared to be Strings in the original Java source, but GWT already has a mechanism to pick a more specialized method, and it can kick in after a few rounds of compilation, allowing a variable declared as Object but always assigned to a String value to take advantage of this.

Additionally, this would make it possible for generated record type equals(Object) methods to automatically dispatch to the expected String specialization without having to statically look up the correct implementation.

We can't just rename equals(String,String) to equalsString(String,String) and add the @SpecializeMethod annotation on equals(Object,Object) as SpecializeMethod's javadoc notes:

gwt/user/super/com/google/gwt/emul/javaemul/internal/annotations/SpecializeMethod.java

Lines 26 to 28 in 3caf879

	* The annotated method must call the target method directly, and the compiler
	* must not inline the target method to prevent it from being pruned while it
	* still might be used.

This is a new note that I added recently as part of solving another bug, see #10165 / #10055. Applying this requirement would increase compiled size of equals(Object,Object) by adding an extra unpruneable instanceof - so while this is possible, it probably isn't the best plan.

The requirement exists so that the compiler is able to leave the specialization without pruning it because it still has at least one use. MethodInliner is already prevented from inlining away the original implementation until we're confident that the specialization cannot be applied. Then, we remove all specializations and prune one more time.

DeadCodeElimination performing constant folding through static method calls on constants also has a similar limitation (or at least ought to) - if the method is inlined away before DCE can examining the call to see if it applies to constants and can be computed at compile time, then the optimization cannot take place. #10255 is intended to improve this situation, and the draft fix works by marking certain method calls as capable of being statically evaluated, then uses the RemoveSpecializations pass to also remove that marker as well. This suggests that it might be worth also running MethodInliner again after specializations are removed, something also included in that draft.

[zbynek]

If we implement Objects.equals as

a == null ? a==b || a.equals(b),

do not specialize it, let the compiler inline it and we implement the optimization condition() ? foo() || foo() -> foo(), then for strings (and enums) it could still get simplified to a==b.

[niloc132]

The trick there is that String.equals can't normally be inlined:

gwt/user/super/com/google/gwt/emul/java/lang/String.java

Lines 419 to 426 in 4e6ae10

	// Marked with @DoNotInline because we don't have static eval for "==" yet.
	@DoNotInline
	@Override
	public boolean equals(Object other) {
	// Java equality is translated into triple equality which is a quick to compare strings for
	// equality without any instanceOf checks.
	return checkNotNull(this) == other;
	}

So, the optimization gets stuck at something like a == null ? a == b || string_equals(a, b), and if the two branches aren't the same the optimization can't take place.

I think that in light of #10255 we could probably remove the do-not-inline annotation though, but that would still result in a == null ? a == b : checkNotNull(a) == b.

gwt/user/super/com/google/gwt/emul/javaemul/internal/InternalPreconditions.java

Lines 388 to 400 in 4e6ae10

	public static <T> T checkNotNull(T reference) {
	if (IS_API_CHECKED) {
	checkCriticalNotNull(reference);
	} else if (IS_ASSERTED) {
	try {
	checkCriticalNotNull(reference);
	} catch (Exception e) {
	throw new AssertionError(e);
	}
	}
	return reference;
	}

IS_API_CHECKED defaults to true. So this is normally inlined away to

gwt/user/super/com/google/gwt/emul/javaemul/internal/InternalPreconditions.java

Lines 402 to 407 in 4e6ae10

	public static <T> T checkCriticalNotNull(T reference) {
	if (reference == null) {
	throw new NullPointerException();
	}
	return reference;
	}

If we implemented both local type tightening (so that in the "else" branch we statically know that a cannot possibly be null) and partial inlining (so that checkCriticalNotNull could have its trivial "else" branch inlined), then that ternary optimization can take effect.

All in all, its definitely better than what I had in mind, since it will also automatically deal with Double/Double and Boolean/Boolean comparisons, and if we already have partial inlining, we'll get null/non-null comparisons for free too. And these were all planned optimizations anyway.

• Identify when consecutive branches of if/else have the same contents and merge them #10285
• Missing compile-time constant folding operations #10147
• Branches based on local/param type information can be used for local type tightening #10241
• Explore partial inlining #10321