Skip to content
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

Jump to bottom

dart2js: Almost all throws are expressions #10633

Open
peter-ahe-google opened this issue May 13, 2013 · 10 comments
Open

dart2js: Almost all throws are expressions #10633

peter-ahe-google opened this issue May 13, 2013 · 10 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js

Comments

@peter-ahe-google
Copy link
Contributor

We know that some JS engines have a hard time optimizing the output from dart2js if we don't explicitly throw. However, it looks like after throw as an expression was implemented, that we always generate an expression without throw.

That is, we generate:

      if ($length !== this.get$length(this))
        $.throwExpression($.ConcurrentModificationError$(this));

But should have generated:

      if ($length !== this.get$length(this))
        throw $.throwExpression($.ConcurrentModificationError$(this));
@kasperl
Copy link

kasperl commented May 23, 2013

Added this to the Later milestone.

@kasperl
Copy link

kasperl commented May 23, 2013

Added TriageForM5 label.

@kasperl
Copy link

kasperl commented May 28, 2013

Removed TriageForM5 label.

@kasperl
Copy link

kasperl commented Jun 26, 2013

Correction. We should have generated:

   if ($length !== this.get$length(this))
     throw $.ConcurrentModificationError$(this);

Set owner to @mkustermann.
Added Accepted label.
Changed the title to: "dart2js: Almost all throws are expressions".

@peter-ahe-google
Copy link
Contributor Author

Kasper, I don't think your correction is correct. You must call wrapException.

@kasperl
Copy link

kasperl commented Jun 26, 2013

Good point, Peter! ... but clearly we don't have to call throwExpression.

@mkustermann
Copy link
Member

cc @floitschG.
Removed the owner.
Added label.

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@peter-ahe-google peter-ahe-google mentioned this issue Jun 3, 2015
Closed
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed Priority-Medium labels Mar 1, 2016
@vsmenon vsmenon added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Jul 20, 2019
@rakudrama
Copy link
Member

A large app has 1395 calls to H.throwExpression(e) and 1571 occurrences of throw H.wrapExpression(e).
Often the throwExpression variant comes from inlining.

I actually think we should use throwExpression more often - it is smaller (minified). Most code is not performance critical, and the explicit throw is probably only useful for code that is executed enough to be JIT-ed by JavaScript - <10% of real apps but 100% of benchmarks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) web-dart2js
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@kevmoo @rakudrama @vsmenon @kasperl @peter-ahe-google @mkustermann