| Summary: | Skip no-llint tests that fail due to running out of executable memory after r188969 | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||||||
| Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | fpizlo, msaboff, ossy, ysuzuki | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | Other | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Michael Saboff
2015-08-20 17:50:55 PDT
Created attachment 259564 [details]
Patch
Oops, i need to ios32 restriction. Created attachment 259572 [details]
Patch
Comment on attachment 259572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259572&action=review Thanks for addressing this quickly. Basically looks good. I suggested some changes to one ChangeLog entry. Do change the "if" clause for every test to include 'and $hostOS == "darwin"' > LayoutTests/ChangeLog:11 > + It consumes much memory and when no-llint, some tests failed due to "ran out of executable memory". > + This patch temporary skips failing tests. Since the size of the fixed allocator is defined by the architecture, > + the failure will be caused when the architecture is ARM. this patch applies the configuration when the architecture is ARM. I think it sounds better if you say something like: That change increases JIT memory usage. This is causing the "no-llint" variation of some tests to fail due to "ran out of executable memory". Since the size of the fixed allocator is defined by the architecture and the tests are only failing on iOS ARM devices, skip those tests. > LayoutTests/js/script-tests/dfg-float32array.js:3 > +//@ noNoLLIntRunLayoutTest if $architecture == "arm" Since the failures have only been reported on iOS, change this to 'if $architecture == "arm" and $hostOS == "darwin"' Comment on attachment 259572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259572&action=review Thank you for your review! >> LayoutTests/ChangeLog:11 >> + the failure will be caused when the architecture is ARM. this patch applies the configuration when the architecture is ARM. > > I think it sounds better if you say something like: > That change increases JIT memory usage. This is causing the "no-llint" variation of some tests to fail due to "ran out of executable memory". > Since the size of the fixed allocator is defined by the architecture and the tests are only failing on iOS ARM devices, skip those tests. Sounds nice! I'll fix the description soon. >> LayoutTests/js/script-tests/dfg-float32array.js:3 >> +//@ noNoLLIntRunLayoutTest if $architecture == "arm" > > Since the failures have only been reported on iOS, change this to 'if $architecture == "arm" and $hostOS == "darwin"' Oh, OK. I'll change it to `if $architecture == "arm" and $hostOS == "darwin"` https://bugs.webkit.org/show_bug.cgi?id=148288 will take several time. So I'll continue to do this. Created attachment 259585 [details]
Patch
Comment on attachment 259585 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259585&action=review Looks good, just another ChangeLog comment. > LayoutTests/ChangeLog:11 > + That change increases JIT memory usage. This is causing the "no-llint" variation of some tests to fail due to "ran out of executable memory". > + Since the size of the fixed allocator is defined by the architecture and the tests are only failing on iOS ARM devices, skip those tests. > + Eliminate this text since you have a more specific explanation below. Comment on attachment 259585 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259585&action=review Thank you! I'll upload the revised one soon. >> LayoutTests/ChangeLog:11 >> + > > Eliminate this text since you have a more specific explanation below. Thanks. Dropped. Created attachment 259634 [details]
Patch
Comment on attachment 259634 [details]
Patch
r=me
(In reply to comment #11) > Comment on attachment 259634 [details] > Patch > > r=me Thank you :) Committed r188767: <http://trac.webkit.org/changeset/188767> |