Bug 151972 – we should emit op_watchdog after op_enter

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

This will take care of loops that implemented using tail calls.

[Saam Barati]

Created attachment 266926 [details]
patch

[WebKit Commit Bot]

Attachment 266926 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:182:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:183:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:184:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Comment on attachment 266926 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=266926&action=review

r=me

> Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:200
> +                printf("PASS: %s script timed out as expected saamy.\n", tierOptions.tier);

typo: I think you didn't mean to have " saamy" in this line.

I also suggest adding a bit more details to this line to distinguish test from the one before e.g. "PASS: %s script with infinite tail calls timed out as expected.\n"

> Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:203
> +                    printf("FAIL: %s script did not time out as expected.\n", tierOptions.tier);

I suggest "FAIL: %s script with infinite tail calls did not time out as expected.\n".

> Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp:205
> +                    printf("FAIL: %s script timeout callback was not called.\n", tierOptions.tier);

I suggest "FAIL: %s script with infinite tail calls' timeout callback was not called.\n".

[Mark Lam]

*** Bug 147998 has been marked as a duplicate of this bug. ***

[Geoffrey Garen]

Maybe it would be better to emit at op_tail_call instead. That way you don’t pay the cost for simple function calls.

[Saam Barati]

We've decided to do it at op_enter just to make life easier for the sampling profiler.

[Saam Barati]

landed in:
http://trac.webkit.org/changeset/193842

[Saam Barati]

forgot to mark this as fixed.