From 8fcb686470e1d765c7f85185bc9f54eb4c0ccc58 Mon Sep 17 00:00:00 2001 From: Nikolai Tillmann Date: Mon, 11 Jun 2018 17:33:27 -0700 Subject: [PATCH] Batch effects applications in each fixed point visiting round. (#2105) Summary: Release notes: Speeding up visiting phase. While there was already some batching in place, lumping together all actions associated with a particular final generator, the batching did not take into account the inherent tree structure of the generator effects. This is now being fixed. Also added logging when in React verbose mode. This change dramatically speeds up internal benchmarks. However, the fixed point computation is still an intolerable bottleness. Future changes can or should include some dependency tracking during the fixed point computation. Right now, each item is a block box, and it is not clear whether it needs to be re-run, so it is re-run. (Created issue #2111 to track this opportunity.) This speeds up an internal React benchmark from 261s to 193s. Closes https://github.com/facebook/prepack/pull/2105 Differential Revision: D8370641 Pulled By: NTillmann fbshipit-source-id: 8fcc499c6f56f2a63fc05417f53d5ebdc746347e --- src/serializer/ResidualHeapVisitor.js | 65 +++++++++++++++++++-------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/src/serializer/ResidualHeapVisitor.js b/src/serializer/ResidualHeapVisitor.js index 0c37b1f53..00ac00b84 100644 --- a/src/serializer/ResidualHeapVisitor.js +++ b/src/serializer/ResidualHeapVisitor.js @@ -11,7 +11,7 @@ import { GlobalEnvironmentRecord, DeclarativeEnvironmentRecord, EnvironmentRecord } from "../environment.js"; import { CompilerDiagnostic, FatalError } from "../errors.js"; -import { Realm } from "../realm.js"; +import { type Effects, Realm } from "../realm.js"; import { Path } from "../singletons.js"; import type { Descriptor, PropertyBinding, ObjectKind } from "../types.js"; import type { Binding } from "../environment.js"; @@ -1225,6 +1225,9 @@ export class ResidualHeapVisitor { } _visitUntilFixpoint() { + if (this.realm.react.verbose) { + this.logger.logInformation(`Computing fixed point...`); + } // Do a fixpoint over all pure generator entries to make sure that we visit // arguments of only BodyEntries that are required by some other residual value let progress = true; @@ -1233,7 +1236,8 @@ export class ResidualHeapVisitor { // as applying effects is expensive, and so we don't want to do it // more often than necessary. let actionsByGenerator = new Map(); - for (let { scope, action } of this.delayedActions.reverse()) { + let expected = 0; + for (let { scope, action } of this.delayedActions) { let generator; if (scope instanceof FunctionValue) generator = this.generatorDAG.getCreator(scope) || this.globalGenerator; else if (scope === "GLOBAL") generator = this.globalGenerator; @@ -1244,26 +1248,50 @@ export class ResidualHeapVisitor { let a = actionsByGenerator.get(generator); if (a === undefined) actionsByGenerator.set(generator, (a = [])); a.push({ action, scope }); + expected++; } this.delayedActions = []; progress = false; + // We build up a tree of effects runner that mirror the nesting of Generator effects. + // This way, we only have to apply any given effects once, regardless of how many actions we have associated with whatever generators. + let effectsInfos: Map void, nestedEffectsRunners: Array<() => void> }> = new Map(); + let topEffectsRunners: Array<() => void> = []; + let actual = 0; for (let [generator, scopedActions] of actionsByGenerator) { - let withEffectsAppliedInGlobalEnv: (() => void) => void = f => f(); + let runGeneratorAction = () => { + for (let { action, scope } of scopedActions) { + actual++; + this._withScope(scope, () => { + if (action() !== false) progress = true; + }); + } + }; let s = generator; let visited = new Set(); + let newNestedRunner; while (s !== "GLOBAL") { invariant(!visited.has(s)); visited.add(s); if (s instanceof Generator) { let effectsToApply = s.effectsToApply; if (effectsToApply) { - let inner = withEffectsAppliedInGlobalEnv; - withEffectsAppliedInGlobalEnv = f => { - this.realm.withEffectsAppliedInGlobalEnv(() => { - inner(f); - return null; - }, effectsToApply); - }; + let info = effectsInfos.get(effectsToApply); + let runner; + if (info === undefined) { + runner = () => { + this.realm.withEffectsAppliedInGlobalEnv(() => { + invariant(info !== undefined); + for (let nestedEffectsRunner of info.nestedEffectsRunners) nestedEffectsRunner(); + return null; + }, effectsToApply); + }; + effectsInfos.set(effectsToApply, (info = { runner, nestedEffectsRunners: [] })); + } + if (newNestedRunner !== undefined) info.nestedEffectsRunners.push(newNestedRunner); + newNestedRunner = runner; + if (runGeneratorAction === undefined) break; + info.nestedEffectsRunners.push(runGeneratorAction); + runGeneratorAction = undefined; } s = this.generatorDAG.getParent(s); } else if (s instanceof FunctionValue) { @@ -1272,14 +1300,15 @@ export class ResidualHeapVisitor { } invariant(s instanceof Generator || s instanceof FunctionValue || s === "GLOBAL"); } - - withEffectsAppliedInGlobalEnv(() => { - for (let { action, scope } of scopedActions) { - this._withScope(scope, () => { - if (action() !== false) progress = true; - }); - } - }); + if (runGeneratorAction !== undefined) { + invariant(newNestedRunner === undefined); + runGeneratorAction(); + } else if (newNestedRunner !== undefined) topEffectsRunners.push(newNestedRunner); + } + for (let topEffectsRunner of topEffectsRunners) topEffectsRunner(); + invariant(expected === actual); + if (this.realm.react.verbose) { + this.logger.logInformation(` (${actual} items processed)`); } } }