-
Notifications
You must be signed in to change notification settings - Fork 382
Description
Depends on #10240 to be done more effectively.
While examining compiler output for AbstractHashMap.put as it relates to method specializing, I noticed that the generated code is usually doing extra pointless work. The put() method is decorated with @SpecializeMethod, which lets the compiler detect when the key is a String and redirect the call to putStringValue:
gwt/user/super/com/google/gwt/emul/java/util/AbstractHashMap.java
Lines 211 to 216 in 58a69dd
| @SpecializeMethod(params = {String.class, Object.class}, target = "putStringValue") | |
| @Override | |
| public V put(K key, V value) { | |
| return key instanceof String | |
| ? putStringValue(JsUtils.uncheckedCast(key), value) : putHashValue(key, value); | |
| } |
gwt/user/super/com/google/gwt/emul/java/util/AbstractHashMap.java
Lines 285 to 292 in 58a69dd
| /** | |
| * Sets the specified key to the specified value in the stringMap. Returns the | |
| * value previously at that key. Returns <code>null</code> if the specified | |
| * key did not exist. | |
| */ | |
| private V putStringValue(String key, V value) { | |
| return key == null ? putHashValue(null, value) : stringMap.put(key, value); | |
| } |
If the compiler doesn't detect it, then at runtime
put() will still do an instanceof and call putStringValue() anyway. This means that put ends up being rewritten through inlining to something like
public V put(K key, V value) {
return key instanceof String
? key == null ? putHashValue(null, value) : stringMap.put((String) key, value)
: putHashValue(key, value);
}This gives us two branches calling putHashValue, but one of them is impossible - if key is a String, then it cannot also be null, and the compiler should be able to remove the putHashValue(null, value) branch. As of today, it cannot.
This ticket is to add a "local type tightening" pass to the GWT AST loop to detect branches that are based on type information and use that information to improve one or both sides of the branch. The idea would be similar to DuplicateClinitRemover, where the local state is copied on every branch, modified to suit the branch, and then applied locally. Each time a local/param reference is assigned, it would also update this information. References to the local/param then would be rewritten if possible, perhaps with an "unchecked cast" to the more accurate information. Alternatively, it could be possible to modify JVariableRef.getType() to be assignable with more local information.
This would apply not only to if/else blocks (and through #10240 to if blocks that throw/return/break/continue), but would also impact short circuiting && and ||, and ternary blocks, in the same way that Java 16's Pattern Matching instanceof works - effectively, as far as compiled output is concerned, every if (foo instanceof Bar) { Bar b = (Bar) foo; ... } would stop paying for that cast, both in generated code and cpu time at runtime.
This optimization could open the door to other future enhancements:
- Beyond type information, value checks could be added as well ("this string isn't the empty string", "this value is always greater than zero"). This might be hard to encode, but one could imagine a for loop through a List that no longer needs range checks because of the bounds of the for's expressions.
- MakeCallsStatic presently supports an option to add null checks and ensure that staticified calls are never made on null instances. Since being added, this has never been enabled, nor has it even been possible to enable. It could be possible to propagate this same type information and always turn that check on, as it may be possible to find that it rarely enough adds an extra check that we even need to worry about a size regression over the release.
If there are risks of size regression from this change itself, they are likely to do with making code simpler than expected and making inlining easier - it seems unlikely to harm more than help.
Compile time could potentially suffer, depending on how much state is being copied - as a compiler pass, this would could not be done only once, but might interact with other passes and progressively enhance as methods are inlined and other type information becomes more accurate.