Render/process everythign recursively - turn XHP tree into an async tree

- all tests pass, including tests added that failed against previous PR - every element renders itself and it's children - all child rendering is async - so, the XHP tree becomes a corresponding async tree This means that asyncRender() at different depths in the tree can be executing simultaneously. There shouldn't be much of a performance cost, as HHVM should convert things things to a StaticWaitHandle when XHPAsync isn't being used. fixes #136 fixes #85 fixes #139
hhvm · May 14, 2015 · 45e4542 · 45e4542
1 parent f08d919
commit 45e4542
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 135 deletions.
diff --git a/src/core/ComposableElement.php b/src/core/ComposableElement.php
@@ -18,15 +18,6 @@ abstract class :x:composable-element extends :xhp {
   private Vector<XHPChild> $children = Vector {};
   private Map<string, mixed> $context = Map {};
 
-  // Helper to put all the UNSAFE in one place until facebook/hhvm#4830 is
-  // addressed
-  protected static async function __xhpAsyncRender(
-    XHPAwaitable $child,
-  ): Awaitable<XHPRoot> {
-    // UNSAFE
-    return await $child->asyncRender();
-  }
-
   protected function init(): void {}
 
   /**
@@ -439,73 +430,7 @@ final protected function __transferContext(
     }
   }
 
-  final protected async function __flushElementChildren(): Awaitable<void> {
-    // Flush all :xhp elements to x:primitive's
-
-    foreach ($this->children as $child) {
-      if ($child instanceof :x:composable-element) {
-        $child->__transferContext($this->context);
-      }
-    }
-
-    $childWaitHandles = Map{};
-    do {
-      if ($childWaitHandles) {
-        $awaitedChildren = await GenMapWaitHandle::create($childWaitHandles);
-        if ($awaitedChildren) {
-          foreach ($awaitedChildren as $i => $awaitedChild) {
-            $this->children->set($i, $awaitedChild);
-          }
-          // Convert <x:frag>s
-          $this->replaceChildren(<x:frag>{$this->children}</x:frag>);
-          $childWaitHandles = Map{};
-        }
-      }
-
-      $ln = count($this->children);
-      for ($i = 0; $i < $ln; ++$i) {
-        $child = $this->children->get($i);
-        if ($child instanceof :x:element) {
-          do {
-            assert($child instanceof :x:element);
-            if ($child instanceof XHPAwaitable) {
-              $child = static::__xhpAsyncRender($child)->getWaitHandle();
-            } else {
-              $child = $child->render();
-            }
-            if ($child instanceof WaitHandle) {
-              $childWaitHandles[$i] = $child;
-            } else if ($child instanceof :x:element) {
-              continue;
-            } else if ($child instanceof :x:frag) {
-              $children = $this->children->toValuesArray();
-              array_splice($children, $i, 1, $child->getChildren());
-              $this->children = new Vector($children);
-              $ln = count($this->children);
-              --$i;
-            } else if ($child === null) {
-              $this->children->removeKey($i);
-              $i--;
-            } else {
-              assert($child instanceof XHPChild);
-              $this->children[$i] = $child;
-            }
-          } while ($child instanceof :x:element);
-        }
-      }
-    } while ($childWaitHandles);
-
-    $flushWaitHandles = Vector{};
-    foreach ($this->children as $child) {
-      if ($child instanceof :x:primitive) {
-        $flushWaitHandles[] = $child->__flushElementChildren()->getWaitHandle();
-      }
-    }
-
-    if ($flushWaitHandles) {
-      await GenVectorWaitHandle::create($flushWaitHandles);
-    }
-  }
+  abstract protected function __flushSubtree(): Awaitable<:x:primitive>;
 
   /**
    * Defined in elements by the `attribute` keyword. The declaration is simple.

diff --git a/src/core/Element.php b/src/core/Element.php
@@ -24,47 +24,49 @@ final public function toString(): string {
   }
 
   final public async function asyncToString(): Awaitable<string> {
-    if (:xhp::$ENABLE_VALIDATION) {
-      $this->validateChildren();
-    }
     $that = await $this->__flushRenderedRootElement();
     $ret = await $that->asyncToString();
     return $ret;
   }
 
+  final protected async function __flushSubtree(): Awaitable<:x:primitive> {
+    $that = await $this->__flushRenderedRootElement();
+    return await $that->__flushSubtree();
+  }
+
+  protected async function __renderAndProcess(): Awaitable<XHPRoot> {
+    if (:xhp::$ENABLE_VALIDATION) {
+      $this->validateChildren();
+    }
+
+    if ($this instanceof XHPAwaitable) {
+      // UNSAFE - interfaces don't support 'protected': facebook/hhvm#4830
+      $composed = await $this->asyncRender();
+    } else {
+      $composed = $this->render();
+    }
+
+    $composed->__transferContext($this->getAllContexts());
+    if ($this instanceof XHPHasTransferAttributes) {
+      $this->transferAttributesToRenderedRoot($composed);
+    }
+
+    return $composed;
+  }
+
   final protected async function __flushRenderedRootElement(
   ): Awaitable<:x:primitive> {
     $that = $this;
     // Flush root elements returned from render() to an :x:primitive
-    do {
-      if (:xhp::$ENABLE_VALIDATION) {
-        $that->validateChildren();
-      }
-      if ($that instanceof XHPAwaitable) {
-        $composed = await static::__xhpAsyncRender($that);
-      } else {
-        invariant(
-          $that instanceof :x:element,
-          "Trying to render something that isn't an element",
-        );
-        $composed = $that->render();
-      }
-      invariant(
-        $composed instanceof :x:composable-element,
-        'Did not get an :x:element from render()',
-      );
-      $composed->__transferContext($that->getAllContexts());
-      if ($that instanceof XHPHasTransferAttributes) {
-        $that->transferAttributesToRenderedRoot($composed);
-      }
-      $that = $composed;
-    } while ($composed instanceof :x:element);
+    while ($that instanceof :x:element) {
+      $that = await $that->__renderAndProcess();
+    }
 
-    if (!($composed instanceof :x:primitive)) {
-      // render() must always (eventually) return :x:primitive
-      throw new XHPCoreRenderException($this, $that);
+    if ($that instanceof :x:primitive) {
+      return $that;
     }
 
-    return $composed;
+    // render() must always (eventually) return :x:primitive
+    throw new XHPCoreRenderException($this, $that);
   }
 }
diff --git a/src/core/Primitive.php b/src/core/Primitive.php
@@ -23,10 +23,33 @@ final public function toString(): string {
   }
 
   final public async function asyncToString(): Awaitable<string> {
+    $that = await $this->__flushSubtree();
+    return $that->stringify();
+  }
+
+  final private async function __flushElementChildren(): Awaitable<void> {
+    $children = $this->getChildren();
+    $awaitables = Map { };
+    foreach ($children as $idx => $child) {
+      if ($child instanceof :x:composable-element) {
+        $child->__transferContext($this->getAllContexts());
+        $awaitables[$idx] = $child->__flushSubtree();
+      }
+    }
+    if ($awaitables) {
+      $awaited = await HH\Asio\m($awaitables);
+      foreach ($awaited as $idx => $child) {
+        $children[$idx] = $child;
+      }
+    }
+    $this->replaceChildren($children);
+  }
+
+  final protected async function __flushSubtree(): Awaitable<:x:primitive> {
     await $this->__flushElementChildren();
     if (:xhp::$ENABLE_VALIDATION) {
       $this->validateChildren();
     }
-    return $this->stringify();
+    return $this;
   }
 }
diff --git a/src/core/XHPRoot.php b/src/core/XHPRoot.php
@@ -10,4 +10,5 @@
  */
 
 interface XHPRoot extends XHPChild {
+  require extends :x:composable-element;
 }
diff --git a/tests/AsyncTest.php b/tests/AsyncTest.php
@@ -84,40 +84,26 @@ public function testInstanceOfInterface() {
     $this->assertInstanceOf(XHPAwaitable::class, $xhp);
   }
 
+  public function parallelizationContainersProvider() {
+    return [
+      [<test:xfrag-wrap />],
+      [<test:async-xfrag-wrap />],
+    ];
+  }
 
-  public function testParallelization() {
+  /**
+   * @dataProvider parallelizationContainersProvider
+   */
+  public function testParallelization(:x:element $container) {
     :async:par-test::$log = Vector { };
 
     $a = <async:par-test label="a" />;
     $b = <async:par-test label="b" />;
     $c = <async:par-test label="c" />;
-    $tree = <async:test>{$a}<test:xfrag-wrap>{$b}{$c}</test:xfrag-wrap></async:test>;
-    $this->assertSame('<div><div>a</div><div>b</div><div>c</div></div>', $tree->toString());
-
-    $log = :async:par-test::$log;
-    $by_node = Map { 'a' => Map { }, 'b' => Map { }, 'c' => Map { } };
 
-    foreach ($log as $idx => $data) {
-      list($label, $action) = $data;
-      $by_node[$label][$action] = $idx;
-    }
+    $container->replaceChildren([$b, $c]);
 
-    $max_start = max($by_node->map($x ==> $x['start']));
-    $min_mid = min($by_node->map($x ==> $x['mid']));
-    $max_mid = max($by_node->map($x ==> $x['mid']));
-    $min_finish = min($by_node->map($x ==> $x['finish']));
-
-    $this->assertGreaterThan($max_start, $min_mid);
-    $this->assertGreaterThan($max_mid, $min_finish);
-  }
-
-  public function testParallelizationWithAsyncFragWrap() {
-    :async:par-test::$log = Vector { };
-
-    $a = <async:par-test label="a" />;
-    $b = <async:par-test label="b" />;
-    $c = <async:par-test label="c" />;
-    $tree = <async:test>{$a}<test:async-xfrag-wrap>{$b}{$c}</test:async-xfrag-wrap></async:test>;
+    $tree = <async:test>{$a}{$container}</async:test>;
     $this->assertSame('<div><div>a</div><div>b</div><div>c</div></div>', $tree->toString());
 
     $log = :async:par-test::$log;
@@ -129,8 +115,12 @@ public function testParallelizationWithAsyncFragWrap() {
     }
 
     $max_start = max($by_node->map($x ==> $x['start']));
+    $min_mid = min($by_node->map($x ==> $x['mid']));
+    $max_mid = max($by_node->map($x ==> $x['mid']));
     $min_finish = min($by_node->map($x ==> $x['finish']));
 
-    $this->assertGreaterThan($max_start, $min_finish);
+    $this->assertGreaterThan($max_start, $min_mid, 'all should be started before any get continued');
+    $this->assertGreaterThan($max_mid, $min_finish, 'all should have reached stage two before any finish');
+    $this->assertGreaterThan($max_start, $min_finish, 'sanity check: all have started before any finish');
   }
 }
diff --git a/tests/BasicsTest.php b/tests/BasicsTest.php
@@ -15,6 +15,11 @@ public function testDivWithString() {
     $this->assertEquals('<div> Hello, world. </div>', $xhp->toString());
   }
 
+  public function testFragWithString() {
+    $xhp = <x:frag>Derp</x:frag>;
+    $this->assertSame('Derp', $xhp->toString());
+  }
+
   public function testDivWithChild() {
     $xhp = <div><div>Herp</div></div>;
     $this->assertEquals('<div><div>Herp</div></div>', $xhp->toString());

diff --git a/tests/XHPHelpersTest.php b/tests/XHPHelpersTest.php
@@ -14,7 +14,7 @@ class :test:xhphelpers extends :x:element {
   attribute :xhp:html-element;
 
   protected function render(): XHPRoot {
-    return <div>{$this->getChildren() }</div>;
+    return <div>{$this->getChildren()}</div>;
   }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,4 +10,5 @@ @@
      */
     interface XHPRoot extends XHPChild {
+      require extends :x:composable-element;
     }