Skip to content

Commit 3dbeb51

Browse files
shickscopybara-github
authored andcommitted
Change AsyncContext transpilation to use separate exit/reenter functions
This works around a [suspected JIT bug in Safari](https://bugs.webkit.org/show_bug.cgi?id=291386). PiperOrigin-RevId: 758343560
1 parent 8fd25b6 commit 3dbeb51

2 files changed

Lines changed: 388 additions & 300 deletions

File tree

src/com/google/javascript/jscomp/InstrumentAsyncContext.java

Lines changed: 78 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,17 @@
3333
/** Instruments {@code await} and {@code yield} for the {@code AsyncContext} polyfill. */
3434
public class InstrumentAsyncContext implements CompilerPass, NodeTraversal.Callback {
3535

36-
private static final String SWAP = "$jscomp$swapContext";
37-
private static final String REENTER_SWAP = "$jscomp$swapContextReenter";
3836
private static final String JSCOMP = "$jscomp";
3937
private static final String ENTER = "asyncContextEnter";
40-
private static final String CREATE_REENTER_SWAP = "asyncContextCreateReenterSwap";
38+
39+
// NOTE: we prefix all internal symbols with a characteristic "ᵃᶜ" (U+1D43, U+1D9C) to
40+
// significantly reduce the chance of conflicts with existing names in the code. This isn't
41+
// perfect, but it results in much more readable transpiled code compared to always generating a
42+
// longer name, keeps tests simple and deterministic compared to using an ID generator, and is
43+
// more performant than scanning for conflicting symbols to only disambiguate when necessary.
44+
private static final String FACTORY = "ᵃᶜfactory";
45+
private static final String EXIT = "ᵃᶜexit";
46+
private static final String REENTER = "ᵃᶜreenter";
4147

4248
private final AbstractCompiler compiler;
4349
private final AstFactory astFactory;
@@ -47,11 +53,6 @@ public class InstrumentAsyncContext implements CompilerPass, NodeTraversal.Callb
4753
// can rely on `Promise.then` to take care of it).
4854
private final boolean shouldInstrumentAwait;
4955

50-
// Whether to emit a more verbose transpilation that uses separate top-level functions for "exit"
51-
// and "reenter", so that we can get a better idea of what's happening.
52-
// TODO: b/379881163, go/tracing-safari-crash - Remove this once we've diagnosed it.
53-
private final boolean diagnoseSafari;
54-
5556
// TODO(sdh): Investigate whether async generator instrumentation can be skipped in ES2017
5657
// output. They are transpiled to ordinary generators with Promise.then, so we may be able
5758
// to rely on runtime patching in that case as well. If so, we'll add an additional boolean
@@ -68,11 +69,10 @@ public InstrumentAsyncContext(AbstractCompiler compiler, boolean shouldInstrumen
6869
}
6970

7071
public InstrumentAsyncContext(
71-
AbstractCompiler compiler, boolean shouldInstrumentAwait, boolean diagnoseSafari) {
72+
AbstractCompiler compiler, boolean shouldInstrumentAwait, boolean unusedDiagnoseSafari) {
7273
this.compiler = compiler;
7374
this.astFactory = compiler.createAstFactory();
7475
this.shouldInstrumentAwait = shouldInstrumentAwait;
75-
this.diagnoseSafari = diagnoseSafari;
7676
}
7777

7878
@Override
@@ -106,7 +106,7 @@ private boolean isReentrance(Node n) {
106106

107107
@Override
108108
public void visit(NodeTraversal t, Node n, @Nullable Node parent) {
109-
if (n.isName() && n.getString().equals(SWAP)) {
109+
if (n.isName() && n.getString().equals(FACTORY)) {
110110
alreadyInstrumented.add(t.getEnclosingFunction());
111111
} else if (n.isSuper()) {
112112
hasSuper.add(t.getEnclosingFunction());
@@ -189,15 +189,15 @@ void instrumentForAwaitOf(NodeTraversal t, Node n, Node parent) {
189189
// use(x);
190190
// }
191191
// becomes
192-
// for await (const x of swap(expr())) { // exit
193-
// swap(0, 1); // reenter
192+
// for await (const x of exit(expr())) {
193+
// reenter();
194194
// try {
195195
// use(x);
196196
// } finally {
197-
// swap(); // exit
197+
// exit();
198198
// }
199199
// }
200-
// swap(0, 1); // reenter
200+
// reenter();
201201

202202
// First wrap the iterator argument
203203
Node placeholder = IR.empty();
@@ -278,8 +278,9 @@ void instrumentGeneratorFunction(NodeTraversal t, Node f) {
278278
generatorBody = addFinallyExit(generatorBody);
279279
generatorBody.addChildToFront(IR.exprResult(createReenter()));
280280
Node newOuterBody =
281-
IR.block(createEnterExit(), IR.returnNode(astFactory.createCallWithUnknownType(inner)));
282-
addReenterSwapAfter(newOuterBody.getFirstChild());
281+
IR.block(
282+
createFactoryExitCall(), IR.returnNode(astFactory.createCallWithUnknownType(inner)));
283+
addExitReenterVarsAfter(newOuterBody.getFirstChild());
283284
f.addChildToBack(newOuterBody.srcrefTreeIfMissing(generatorBody));
284285

285286
// NOTE: if the IIFE generator refers to `this` or `arguments` then we need to extract these
@@ -324,10 +325,10 @@ void instrumentGeneratorMethod(NodeTraversal t, Node f) {
324325
Node outerParams = f.getSecondChild();
325326
Node innerParams = outerParams.cloneTree();
326327

327-
// Add the $jscomp$swapContext parameter to the front of the inner parameter list
328-
Node contextParam = astFactory.createNameWithUnknownType(SWAP);
329-
innerParams.addChildToFront(contextParam);
330-
AstFactory.Type unknownType = AstFactory.type(contextParam);
328+
// Add the $jscomp$swapFactory parameter to the front of the inner parameter list
329+
Node swapFactoryParam = createFactoryName();
330+
innerParams.addChildToFront(swapFactoryParam);
331+
AstFactory.Type unknownType = AstFactory.type(swapFactoryParam);
331332

332333
// Add a finally-exit wrapper and an initial reenter call around the original generator body
333334
// (which has already had its yields/awaits instrumented).
@@ -345,7 +346,7 @@ void instrumentGeneratorMethod(NodeTraversal t, Node f) {
345346
argumentsReference = astFactory.createArgumentsReference();
346347
argumentsParam =
347348
astFactory.createName(renamer.argumentsName, AstFactory.type(argumentsReference));
348-
argumentsParam.insertAfter(contextParam);
349+
argumentsParam.insertAfter(swapFactoryParam);
349350
}
350351

351352
// Detach the generator body and replace it with a new empty block. We'll fill the empty block
@@ -406,8 +407,8 @@ void instrumentGeneratorMethod(NodeTraversal t, Node f) {
406407
}
407408

408409
// Populate the outer (non-generator) method body with an enter() and call to the inner method.
409-
outerBody.addChildToBack(createEnterExit());
410-
addReenterSwapAfter(outerBody.getLastChild());
410+
outerBody.addChildToBack(createFactoryExitCall());
411+
addExitReenterVarsAtStartOf(generatorBody);
411412
outerBody.addChildToBack(IR.returnNode(innerCall));
412413

413414
// Various cleanups
@@ -489,8 +490,8 @@ void instrumentAsyncFunction(Node f) {
489490
// Add a variable to the front of the body
490491
Node block = f.getLastChild();
491492
Node newBlock = addFinallyExit(block);
492-
newBlock.addChildToFront(createEnter().srcrefTreeIfMissing(newBlock));
493-
addReenterSwapAfter(newBlock.getFirstChild());
493+
newBlock.addChildToFront(createFactoryCall().srcrefTreeIfMissing(newBlock));
494+
addExitReenterVarsAfter(newBlock.getFirstChild());
494495
compiler.reportChangeToChangeScope(f);
495496

496497
// TODO(sdh): Need to instrument async functions for unhandled rejections.
@@ -514,71 +515,88 @@ Node addFinallyExit(Node block) {
514515
return newBlock;
515516
}
516517

517-
/** Creates a {@code $jscomp$swapContext} identifier node. */
518-
private Node createSwap() {
519-
return astFactory.createQNameWithUnknownType(SWAP);
518+
/** Creates an {@code ᕦᕤexit} identifier node. */
519+
private Node createExitName() {
520+
return astFactory.createQNameWithUnknownType(EXIT);
520521
}
521522

522-
// Alternative version for the "diagnose safari" case.
523-
private Node createReenterSwap() {
524-
return astFactory.createQNameWithUnknownType(diagnoseSafari ? REENTER_SWAP : SWAP);
523+
/** Creates a {@code ᕦᕤreenter} identifier node. */
524+
private Node createReenterName() {
525+
return astFactory.createQNameWithUnknownType(REENTER);
525526
}
526527

527-
private void addReenterSwapAfter(Node node) {
528-
if (diagnoseSafari) {
529-
// var $jscomp$swapContextReenter = $jscomp.asyncContextCreateReenterSwap($jscomp$swapContext)
530-
Node swap =
531-
astFactory.createCallWithUnknownType(
532-
astFactory.createQNameWithUnknownType(JSCOMP, ImmutableList.of(CREATE_REENTER_SWAP)),
533-
createSwap());
534-
IR.var(astFactory.createConstantName(REENTER_SWAP, AstFactory.type(swap)), swap)
535-
.srcrefTreeIfMissing(node)
536-
.insertAfter(node);
537-
}
528+
/** Creates a {@code ᕦᕤfactory} identifier node. */
529+
private Node createFactoryName() {
530+
return astFactory.createQNameWithUnknownType(FACTORY);
538531
}
539532

540-
/** Creates a statement {@code const $jscomp$swapContext = $jscomp$asyncContextEnter();}. */
541-
private Node createEnter(Node... arg) {
533+
private void addExitReenterVarsAfter(Node node) {
534+
// var ᕦᕤexit = ᕦᕤfactory();
535+
// var ᕦᕤreenter = ᕦᕤfactory(1);
536+
Node reenterCall =
537+
astFactory.createCallWithUnknownType(createFactoryName(), astFactory.createNumber(1));
538+
IR.var(astFactory.createConstantName(REENTER, AstFactory.type(reenterCall)), reenterCall)
539+
.srcrefTreeIfMissing(node)
540+
.insertAfter(node);
541+
Node exitCall = astFactory.createCallWithUnknownType(createFactoryName());
542+
IR.var(astFactory.createConstantName(EXIT, AstFactory.type(exitCall)), exitCall)
543+
.srcrefTreeIfMissing(node)
544+
.insertAfter(node);
545+
}
546+
547+
private void addExitReenterVarsAtStartOf(Node node) {
548+
// var ᕦᕤexit = ᕦᕤfactory();
549+
// var ᕦᕤreenter = ᕦᕤfactory(1);
550+
Node reenterCall =
551+
astFactory.createCallWithUnknownType(createFactoryName(), astFactory.createNumber(1));
552+
node.addChildToFront(
553+
IR.var(astFactory.createConstantName(REENTER, AstFactory.type(reenterCall)), reenterCall)
554+
.srcrefTreeIfMissing(node));
555+
Node exitCall = astFactory.createCallWithUnknownType(createFactoryName());
556+
node.addChildToFront(
557+
IR.var(astFactory.createConstantName(EXIT, AstFactory.type(exitCall)), exitCall)
558+
.srcrefTreeIfMissing(node));
559+
}
560+
561+
/** Creates a statement {@code const ᕦᕤfactory = $jscomp$asyncContextEnter();}. */
562+
private Node createFactoryCall(Node... arg) {
542563
Node call =
543564
astFactory.createCallWithUnknownType(
544565
astFactory.createQNameWithUnknownType(JSCOMP, ImmutableList.of(ENTER)), arg);
545-
return createConst(astFactory.createConstantName(SWAP, AstFactory.type(call)), call);
566+
return createVar(astFactory.createConstantName(FACTORY, AstFactory.type(call)), call);
546567
}
547568

548-
private Node createConst(Node name, Node value) {
549-
if (compiler.getOptions().getOutputFeatureSet().contains(Feature.CONST_DECLARATIONS)) {
550-
return IR.constNode(name, value);
551-
}
569+
private Node createVar(Node name, Node value) {
552570
return IR.var(name, value);
553571
}
554572

555-
/** Creates a statement {@code const $jscomp$swapContext = $jscomp$asyncContextEnter(1);}. */
556-
private Node createEnterExit() {
557-
return createEnter(astFactory.createNumber(1));
573+
/** Creates a statement {@code const ᕦᕤfactory = $jscomp$asyncContextEnter(1);}. */
574+
private Node createFactoryExitCall() {
575+
return createFactoryCall(astFactory.createNumber(1));
558576
}
559577

560-
/** Wraps the given node in an exit call: {@code $jscomp$swapContext(NODE)}. */
578+
/** Wraps the given node in an exit call: {@code ᕦᕤexit(NODE)}. */
561579
private Node createExit(Node inner) {
562-
return astFactory.createCall(createSwap(), AstFactory.type(inner), inner);
580+
return astFactory.createCall(createExitName(), AstFactory.type(inner), inner);
563581
}
564582

565583
/** Creates an empty exit call: {@code $jscomp$swapContext()}. */
566584
private Node createExit() {
567-
Node swap = createSwap();
568-
return astFactory.createCall(swap, AstFactory.type(swap));
585+
Node exit = createExitName();
586+
return astFactory.createCall(exit, AstFactory.type(exit));
569587
}
570588

571589
/** Wraps the given node in a reenter call: {@code $jscomp$swapContext(NODE, 1)}. */
572590
private Node createReenter(Node inner) {
573-
return astFactory.createCall(
574-
createReenterSwap(), AstFactory.type(inner), inner, astFactory.createNumber(1));
591+
return astFactory.createCall(createReenterName(), AstFactory.type(inner), inner);
575592
}
576593

577594
/**
578595
* Creates an empty reenter call. The re-returned first argument is irrelevant, so we just pass a
579596
* literal zero node, since it should compress well.
580597
*/
581598
private Node createReenter() {
582-
return createReenter(astFactory.createNumber(0));
599+
Node reenter = createReenterName();
600+
return astFactory.createCall(reenter, AstFactory.type(reenter));
583601
}
584602
}

0 commit comments

Comments
 (0)