dart2js: use type information to simplify type checks (guards and is checks) #9836
Comments
|
I think what you're seeing here is that you check for any List, but in order for us to use fruits[i] in the generated code we have to make sure that it is a builtin List (represented as a JS array). Clearly the code could be much nicer if we realized that we know something about fruits after the 'is List' check passes. We know it's non-null and we know it's an object that implements List (I think we actually know that in the compiler). It seems like all we need to do to generate vastly better code here is to improve the is checks by using information about the type of the checked instruction in visitTypeGuard, handleNumberOrStringSupertypeCheck, handleStringSupertypeCheck, and handleListOrSupertypeCheck in codegen.dart. Added this to the M5 milestone. |
|
We just chatted a bit more about this at the office and it would also be possible for us to realize when all Lists in a program implement JavaScriptIndexingBehavior and optimize the second check away in that case. |
|
Awesome, thanks for the great update. Playing with dart2js is a lot of fun. Keep up the good work! |
|
Added TriageForM5 label. |
|
Have this been fixed already? |
|
Removed TriageForM5 label. |
|
Removed this from the Later milestone. |
|
Removed Oldschool-Milestone-Later label. |
|
Verified fixed in 79a8654. |
Hi, hope this is a useful report.
Consider this code:
main() {
var map = {};
map['fruits'] = ['apples', 'oranges'];
var fruits = map['fruits'];
if (fruits is! List) return;
for (var i = 0; i < fruits.length; i++) {
print(fruits[i]);
}
}
My expectation is that if fruits is not a list, the for loop is never encountered, and that my manual type guard is sufficient.
However, here's the dart2js compiled code:
$.main = function() {
var map, fruits, i;
map = $.makeLiteralMap([]);
map.$indexSet(map, "fruits", ["apples", "oranges"]);
fruits = map.$index(map, "fruits");
if (!(typeof fruits === "object" && fruits !== null && (fruits.constructor === Array || !!$.getInterceptor(fruits).$isList)))
return;
else if (typeof fruits !== "object" || fruits === null || fruits.constructor !== Array && !$.getInterceptor(fruits).$isJavaScriptIndexingBehavior)
return $.main$bailout(1, fruits);
for (i = 0; i < fruits.length; ++i)
$.Primitives_printString($.toString$0(fruits[i]));
};
Shouldn't my manual type guard be sufficient? Can we drop the "else if" guard?
Thanks for taking a look!
The text was updated successfully, but these errors were encountered: