|
Description
Matthew Mirman
2014-12-10 11:58:04 PST
Created attachment 243056 [details]
failing put-by-id resursive test
Created attachment 243308 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
Comment on attachment 243308 [details] Fixes operationPutByIdOptimizes such that they save the structure before the put View in context: https://bugs.webkit.org/attachment.cgi?id=243308&action=review Code looks good, but you should improve this layout test before landing it. > Source/JavaScriptCore/tests/stress/put-by-id-build-list-order-recurse.js:15 > + var str = "for (var i = " + currentCount + "; i < " + (100 + currentCount) + "; ++i)\n" > + + " o.f\n"; > + eval(str); An incorrect read or write can sometimes survive without crashing. This test should explicitly check its result for some sensible value. It looks like this crash is in the JIT. Why does it only happen when the LLInt is enabled? It looks like the major thing that was changing the occurrence of the bug was thresholdForJITAfterWarmUp. Setting it to 100 causes this test specifically to crash, but I have been able to get smaller tests to crash with it set much lower (6). Changing when the JIT is invoked changes when the inline cache gets rewritten. Comment on attachment 243308 [details] Fixes operationPutByIdOptimizes such that they save the structure before the put Committed r177380: <http://trac.webkit.org/changeset/177380> Reviewed patch landed. Closing bug. Re-opened since this is blocked by bug 139707 Created attachment 243702 [details] Fixes operationPutByIdOptimizes such that they save the structure before the put It appears as though there still might be some failing tests due to rdar://problem/19339106 so this isn't to be committed quite yet. Comment on attachment 243702 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
Nvm the last comment. run-javascript-tests works.
Comment on attachment 243702 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
r=me if there is no perf impact
Created attachment 244546 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
Attachment 244546 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/jit/Repatch.cpp:997: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/jit/Repatch.cpp:1003: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/jit/Repatch.cpp:1007: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/jit/Repatch.cpp:1010: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/jit/Repatch.cpp:1012: Multi line control clauses should use braces. [whitespace/braces] [4]
Total errors found: 5 in 10 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 244549 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
fixes some style from the last patch.
Comment on attachment 244549 [details] Fixes operationPutByIdOptimizes such that they save the structure before the put View in context: https://bugs.webkit.org/attachment.cgi?id=244549&action=review > Source/JavaScriptCore/tests/mozilla/mozilla-tests.yaml:793 > +# - path: ecma/LexicalConventions/7.7.3.js > +# cmd: defaultRunMozillaTest :normal, "../shell.js" What's going on here? Created attachment 244550 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
Comment on attachment 244550 [details] Fixes operationPutByIdOptimizes such that they save the structure before the put View in context: https://bugs.webkit.org/attachment.cgi?id=244550&action=review You should rebase this patch. > Source/JavaScriptCore/tests/mozilla/mozilla-tests.yaml:793 > -- path: ecma/LexicalConventions/7.7.3.js > - cmd: defaultRunMozillaTest :normal, "../shell.js" > +# - path: ecma/LexicalConventions/7.7.3.js > +# cmd: defaultRunMozillaTest :normal, "../shell.js" You should put a comment above this referencing the bug. (In reply to comment #15) > Comment on attachment 244549 [details] > Fixes operationPutByIdOptimizes such that they save the structure before the > put > > View in context: > https://bugs.webkit.org/attachment.cgi?id=244549&action=review > > > Source/JavaScriptCore/tests/mozilla/mozilla-tests.yaml:793 > > +# - path: ecma/LexicalConventions/7.7.3.js > > +# cmd: defaultRunMozillaTest :normal, "../shell.js" > > What's going on here? <rdar://problem/19339106> Created attachment 244553 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
Comment on attachment 244553 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
Can you rebase this?
Created attachment 244625 [details]
Fixes operationPutByIdOptimizes such that they save the structure before the put
I accidentally included a change to a non existent file in the last patch. This should be better.
Comment on attachment 244625 [details] Fixes operationPutByIdOptimizes such that they save the structure before the put Committed r177380: <http://trac.webkit.org/changeset/178441> Reviewed patch landed. Closing bug. |