[webkit-changes] [WebKit/WebKit] 863e4a: [JSC] Array destructuring assignment should close ...

[Commit Queue]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 863e4a5d13af9630b7ff401e3249294100886bc6
      https://github.com/WebKit/WebKit/commit/863e4a5d13af9630b7ff401e3249294100886bc6
  Author: Alexey Shvayka <ashvayka at apple.com>
  Date:   2024-09-10 (Tue, 10 Sep 2024)

  Changed paths:
    A JSTests/microbenchmarks/destructuring-array-default-value-can-not-throw.js
    A JSTests/microbenchmarks/destructuring-array-default-value-can-throw.js
    A JSTests/stress/destructuring-iterator-close-not-called-if-iterator-step-throws.js
    M JSTests/test262/expectations.yaml
    M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
    M Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
    M Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
    M Source/JavaScriptCore/parser/Nodes.h

  Log Message:
  -----------
  [JSC] Array destructuring assignment should close the iterator if PutValue or initializer evaluation throws
https://bugs.webkit.org/show_bug.cgi?id=268271
<rdar://problem/121960887>

Reviewed by Yusuke Suzuki.

Before this change, if PutValue or initializer evaluation threw an exception during array destructuring
assignment [1], IteratorClose() algorithm was not executed at all, depriving the iterator of a chance
to cleanup (e.g. generator function executing the `finally` block).

This patch extracts emitTryWithFinallyThatDoesNotShadowException() from for/of loop so it can be used
in array destructuring to close the iterator if case of an exception, but only conditionally:
if exception can be thrown beyond IteratorStep(), which is determined in bindValueOrDefaultValueCanThrow().

If exception is thrown in IteratorStep() and friends, Iterator Record's [[Done]] will be `true`,
failing the condition to perform IteratorClose() [1].

bindValueCanThrow() is introduced because in case of VarKind::Scope, op_put_to_scope never throws yet
writableDirectBindingIfPossible() returns `nullptr`; the rationale for adding getFromScopeCanThrow() is
similar -- avoid wrapping in try/finally as many assignments as possible.

For most cases of destructuring on the web, this change doesn't add a single bytecode to be emitted,
yet performance hit in the rest of the cases is second to none with FTL:

                                                            ToT                      patch

destructuring-array-default-value-can-not-throw      24.2511+-0.0364     !     24.5875+-0.0325        ! definitely 1.0139x slower
destructuring-array-default-value-can-throw          27.6809+-0.0325           27.6434+-0.0406

<geometric>                                          25.9092+-0.0235     !     26.0706+-0.0303        ! definitely 1.0062x slower

This patch was thoroughly benchmarked to ensure JS3 and SP3 are neutral: the try/finally around
destructuring is emitted only in JS3/coffeescript-wtb, SP3/TodoMVC-Vue, and SP3/Charts-observable-plot;
it was proven these tests are neutral and no function crosses inlining threshold.

Aligns JSC with V8 and SpiderMonkey.

[1]: https://tc39.es/ecma262/#sec-runtime-semantics-destructuringassignmentevaluation (last 4 productions)

* JSTests/microbenchmarks/destructuring-array-default-value-can-not-throw.js: Added.
* JSTests/microbenchmarks/destructuring-array-default-value-can-throw.js: Added.
* JSTests/stress/destructuring-iterator-close-not-called-if-iterator-step-throws.js: Added.
* JSTests/test262/expectations.yaml: Mark 84 tests as passing.
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitTryWithFinallyThatDoesNotShadowException):
(JSC::BytecodeGenerator::emitGenericEnumeration):
(JSC::BytecodeGenerator::emitEnumeration):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:
* Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:
(JSC::ResolveNode::getFromScopeCanThrow const):
(JSC::ArrayPatternNode::bindValue const):
(JSC::BindingNode::bindValueCanThrow const):
(JSC::AssignmentElementNode::bindValueCanThrow const):
* Source/JavaScriptCore/parser/Nodes.h:
(JSC::DestructuringPatternNode::bindValueCanThrow const):

Canonical link: https://commits.webkit.org/283443@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications