Bug 79040

Created attachment 127922 [details] Patch

(In reply to comment #1) > Created an attachment (id=127922) [details] > Patch It is a workaround to make our bots happier. Without fixing this bug or without this workaround our bots can't be green anymore. (because of stricter rule in master.cfg - http://trac.webkit.org/changeset/108246)

Filip, it seems that LLint doesn't do the timeout checks for loops, like it's done in op_loop in Interpreter.cpp. Is that a missing feature in llint?

Comment on attachment 127922 [details] Patch rs=me. Let's get this workaround in until we figure out why the timeout don't seem to be getting called with llint.

I made a little digging, and checked if TimeoutChecker::didTimeOut(ExecState* exec) is called or not. It is called from tst_QWebPage::infiniteLoopJS()if DFG JIT is disabled. But it isn't called if DFG JIT is enabled. And it isn't called with same infinite loop from DRT and QtTestBrowser irrespectively of DFG JIT is enabled or not.

Comment on attachment 127922 [details] Patch Clearing flags on attachment: 127922 Committed r108341: <http://trac.webkit.org/changeset/108341>

Hey Filip, I think I tracked down this issue. It seems the DFG JIT does not call TimeoutChecker at all, so nothing checks the timeout in case of a simple (function () { while(true) { } })() Which essentially stops the browser forever. Any idea how to fix this?

We don't currently support a mechanism to asynchronously interrupt JavaScript execution. To stop a runaway script in WebKit2, you can kill the web process - this ensures all resources are released.

We discussed this issue with Philip before, and we decided that we refactor the code. This work is currently in progress.

Created attachment 166022 [details] Proposed patch Proposed patch without changelog. It depends on https://bugs.webkit.org/show_bug.cgi?id=96916

Mark Lam is working on a more efficient fix for this.

Thanks for letting us know!

(In reply to comment #11) > Mark Lam is working on a more efficient fix for this. :(( I don't think if ignoring review at all for a month is the best / most friendlier way to let us know that you don't want Gábor's fix and you prefer fixing by yourself.

> I don't think if ignoring review at all for a month is the best / > most friendlier way to let us know that you don't want Gábor's fix > and you prefer fixing by yourself. To clarify, Mark wasn't working on this a month ago. If you want this patch reviewed, the best thing to do is mark the review flag r? and post performance numbers for a reviewer to evaluate.

Sunspider benchmark results on x86_64 Linux: sunspider: TEST COMPARISON r134387 Proposed patch DETAILS ============================================================================= ** TOTAL **: *1.014x as slow* 208.5ms +/- 0.1% 211.4ms +/- 0.2% significant ============================================================================= 3d: *1.019x as slow* 32.6ms +/- 0.4% 33.2ms +/- 0.4% significant cube: *1.050x as slow* 11.5ms +/- 1.1% 12.1ms +/- 0.5% significant morph: ?? 9.1ms +/- 0.9% 9.3ms +/- 1.1% not conclusive: might be *1.012x as slow* raytrace: - 11.9ms +/- 0.5% 11.9ms +/- 0.5% access: ?? 18.0ms +/- 0.0% 18.0ms +/- 0.2% not conclusive: might be *1.001x as slow* binary-trees: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% fannkuch: ?? 8.0ms +/- 0.0% 8.0ms +/- 0.3% not conclusive: might be *1.002x as slow* nbody: - 4.0ms +/- 0.0% 4.0ms +/- 0.0% nsieve: - 4.0ms +/- 0.0% 4.0ms +/- 0.0% bitops: 1.025x as fast 12.5ms +/- 0.8% 12.2ms +/- 0.7% significant 3bit-bits-in-byte: - 1.0ms +/- 0.0% 1.0ms +/- 0.0% bits-in-byte: 1.057x as fast 5.5ms +/- 1.8% 5.2ms +/- 1.6% significant bitwise-and: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% nsieve-bits: - 4.0ms +/- 0.0% 4.0ms +/- 0.0% controlflow: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% recursive: - 2.0ms +/- 0.0% 2.0ms +/- 0.0% crypto: - 17.0ms +/- 0.0% 17.0ms +/- 0.1% aes: - 10.0ms +/- 0.0% 10.0ms +/- 0.0% md5: ?? 4.0ms +/- 0.0% 4.0ms +/- 0.5% not conclusive: might be *1.002x as slow* sha1: - 3.0ms +/- 0.0% 3.0ms +/- 0.0% date: ?? 31.0ms +/- 0.1% 31.1ms +/- 0.5% not conclusive: might be *1.002x as slow* format-tofte: ?? 16.0ms +/- 0.2% 16.1ms +/- 0.8% not conclusive: might be *1.006x as slow* format-xparb: - 15.0ms +/- 0.2% 15.0ms +/- 0.3% math: *1.073x as slow* 18.6ms +/- 0.5% 20.0ms +/- 0.0% significant cordic: *1.33x as slow* 3.0ms +/- 0.0% 4.0ms +/- 0.0% significant partial-sums: - 13.0ms +/- 0.2% 13.0ms +/- 0.0% spectral-norm: *1.141x as slow* 2.6ms +/- 3.7% 3.0ms +/- 0.0% significant regexp: - 11.0ms +/- 0.2% 11.0ms +/- 0.0% dna: - 11.0ms +/- 0.2% 11.0ms +/- 0.0% string: *1.017x as slow* 65.8ms +/- 0.2% 66.9ms +/- 0.4% significant base64: ?? 5.0ms +/- 0.0% 5.0ms +/- 0.4% not conclusive: might be *1.002x as slow* fasta: - 10.0ms +/- 0.0% 10.0ms +/- 0.0% tagcloud: *1.008x as slow* 16.2ms +/- 0.5% 16.4ms +/- 0.6% significant unpack-code: *1.025x as slow* 26.5ms +/- 0.4% 27.1ms +/- 0.6% significant validate-input: *1.042x as slow* 8.1ms +/- 0.6% 8.4ms +/- 1.5% significant V8: TEST COMPARISON r134387 Proposed patch DETAILS ============================================================================= ** TOTAL **: *1.016x as slow* 748.9ms +/- 0.0% 760.7ms +/- 0.1% significant ============================================================================= v8: *1.016x as slow* 748.9ms +/- 0.0% 760.7ms +/- 0.1% significant crypto: *1.107x as slow* 86.8ms +/- 0.1% 96.1ms +/- 0.1% significant deltablue: *1.009x as slow* 135.4ms +/- 0.1% 136.7ms +/- 0.1% significant earley-boyer: *1.006x as slow* 97.8ms +/- 0.1% 98.4ms +/- 0.2% significant raytrace: *1.002x as slow* 70.8ms +/- 0.1% 71.0ms +/- 0.1% significant regexp: 1.010x as fast 107.2ms +/- 0.1% 106.1ms +/- 0.1% significant richards: *1.014x as slow* 113.5ms +/- 0.2% 115.0ms +/- 0.2% significant splay: - 137.3ms +/- 0.2% 137.4ms +/- 0.3%

As far as I know, Gábor doesn't work on it anymore. Could you confirm it?

(In reply to comment #16) > As far as I know, Gábor doesn't work on it anymore. Could you confirm it? Yes, that's true. As far as I know Mark Lam is working on it now.