New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dart2js: find a way not to add all checked mode helpers in resolution queue #13155
Comments
This comment was originally written by ngeoffray@google.com I do not agree with the low priority. Enqueuing those helpers have an effect on compile-times and global optimizations. We've put a lot of effort on making sure only what is necessary is being resolved/compiled, and I believe this bug falls into that category and should be fixed. |
Regarding claim 1): the selection criteria during resolution has to be about the type that is tested, because this is the only reliable information available. The static type of the receiver may not be known at all. It is possible to use instantiated classes as a filter for which receiver types are possible. Regarding claim 2): enqueuing more than needed during resolution can affect the amount of generated code, as the codegen will ask the resolver about global information. So as best as I can tell, we lack motivation for lowering the priority. |
I think your reservations are based on the theoretical implications of adding a few dozen top level definitions to the resolution queue and not the actual implications. I did not see any changes in codegen from before and after my change, but I could have missed something. If I missed something, I agree it should not be Priority-Low. Regarding #2 (1): The correct criteria for quality code generation must include the receiver type: If we can't access the type of "x" then we need to pull in at least three helpers, of which only one will eventually be used. Regarding code bloat: The helpers that pull in more than the common dependencies, I agree that they might pull in more code. (x as TypeParameter) and (x as Type<Parameter>) and (x as Type<TypeParameter>) probably can cause the general subtype code to be pulled in. But I didn't see any extra code generated because the code is dominated by top level functions that are eliminated in codegen. I think we need a general solution to the problem of dependencies during resolution. main() { We might also take a different approach to the 'checked mode helpers'. Removed Priority-Low label. |
Removed this from the Later milestone. |
Removed Oldschool-Milestone-Later label. |
We register all the checked mode helpers in resolution because the check,
emitter.nativeEmitter.requiresNativeIsCheck(element);
does not work property until after resolution.
This work item is to find a way to avoid enqueueing all 22 helpers.
The problem is that selecting the correct helper now depends on whether a
mixin is used on some native class or a subclass of a native class. You can't
tell from the type in your hand (the mixin or a subtype thereof) that a mixin is
used in another class until that other class has been resolved. Resolving that
other class depends on allocations or native calls in methods that have not been
resolved yet. So the selection of the correct helper depends the set of all
resolved types.
Basically, we need to avoid subtype queries until after resolution is complete.
I'm putting this at low priority because:
(1) I believe the selection criteria for helpers should be based of the static type of the receiver, and not the type of the test. This information is not known at resolution time.
(2) No extra code is generated by putting a few more top level methods in the resolution queue.
The text was updated successfully, but these errors were encountered: