Skip to content

Angular's Parser test regressed #17073

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

Closed
blois opened this issue Feb 24, 2014 · 5 comments
Closed

Angular's Parser test regressed #17073

blois opened this issue Feb 24, 2014 · 5 comments
Assignees
Labels
closed-as-intended Closed as the reported issue is expected behavior web-dart2js

Comments

@blois
Copy link

blois commented Feb 24, 2014

Angular's Parser test appears to have regressed in:
http://chromegw.corp.google.com/i/client.dart/builders/dart2js-drt-linux-1-2-be/builds/1715

Repro command is:
python tools/test.py -mrelease -cdart2js -rdrt --use-sdk --write-debug-log --write-test-outcome-log --checked -t120 pkg/third_party/angular_tests/browser_test/core/parser/parser

CL which introduced the regression appears to be:
https://codereview.chromium.org//140643006

Reverting this CL appears to fix the issue.

@kasperl
Copy link

kasperl commented Feb 25, 2014

Pretty sure I know exactly how to fix this on the Angular side. Stay tuned!


cc @lrhn.
Set owner to @kasperl.

@lrhn
Copy link
Member

lrhn commented Feb 25, 2014

Still, if you regress on performance, it means that the Symbol.validated constructor (the only thing that changed) is now slower.
It's not surprising, since we did increase the complexity of the validation in order to detect more/all invalid inputs, but I would not expect it to be very significant.
Especially with dart2js code, the RegExp should be fast enough.

How much did you regress?

@kasperl
Copy link

kasperl commented Feb 25, 2014

It's not a performance regression.

@kasperl
Copy link

kasperl commented Feb 25, 2014

Solved on the Angular side with this PR: dart-archive/angular.dart#614.


Added Started label.

@kasperl
Copy link

kasperl commented Feb 25, 2014

This really isn't a dart2js issue, so I'm marking this 'as designed'.


Removed Priority-Critical label.
Added Priority-Medium, AsDesigned labels.

@blois blois added Type-Defect web-dart2js closed-as-intended Closed as the reported issue is expected behavior labels Feb 25, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior web-dart2js
Projects
None yet
Development

No branches or pull requests

3 participants