Eager evaluation fails for non-CSA builtins (Array.prototype.toString, Array.prototype.join)
Reported by
thejames...@gmail.com,
May 22 2018
|
|||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36 Steps to reproduce the problem: I've been trying out eager evaluation in Devtools and it's great, but something I noticed is that `[1,2,3,4].toString()` doesn't show any eager evaluation. What is the expected behavior? What went wrong? The result of the above call is not displayed as an eager evaluation. This is surprising as I didn't think `.toString` would have any side effects. Did this work before? N/A Chrome version: 68.0.3432.3 Channel: dev OS Version: Ubuntu 18.04 Flash Version: This isn't particularly important mostly just interesting that the side-effect detection thinks that `Array.prototype.toString` causes a side-effect.
,
May 23 2018
TE@, eager evaluation is tracked in issue 810176 . Inspecting devtools' _requestPreview() function in devtools-on-devtools reveals "EvalError: Possible side-effect in debug-evaluate" exception: https://cs.chromium.org/chromium/src/third_party/blink/renderer/devtools/front_end/console/ConsolePrompt.js?l=107&rcl=dd9d6fa34e49353c59a5e335d44b9b3ebb5680dc FWIW, [].toString() is successfully evaluated to ""
,
May 23 2018
As per issue id: 810176, ccing dev luoe@ for further inputs on this issue. Thanks...!!
,
May 25 2018
,
May 25 2018
Thank you for the report. When EagerEval cannot confirm that a method is safe, it just bails out. Some builtin methods such as Array.p.join do not use V8's code stub assembler, which we require to mark it as safe. Array.p.toString() is not considered safe, only because it may use Array.p.join(), which hasn't been ported to CSA yet. I believe there is an existing effort port all V8 builtins, and I'll check the status.
,
Jan 14
It looks like this has been fixed already on the V8 side due to this CL [1]. Now that Array.prototype.toString, Array.prototype.join have been added, these seem to work on M72 and M73! [1] https://chromium.googlesource.com/v8/v8.git/+/952c097679c5e16ae214595ad3b01381483eab7b |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by krajshree@chromium.org
, May 23 2018