Implemented a memory-reduction step for non-day flat-first hierarchy rebuild

Author: sgiehl

core/ArchiveProcessor/RecordBuilder.php

@@ -382,7 +382,7 @@ protected function aggregateBuiltFromFlatRecordForNonDay(
         $archiveProcessor->insertBlobRecord($flatRecordName, $flatSerialized);
         $processedFlatRecords[$flatRecordName] = true;
 
-        $hierarchicalTable = $this->buildHierarchicalTableFromFlatTable(
+        $hierarchicalTable = $this->buildHierarchicalTableFromFlatTableAndConsumeRows(
             $flatTable,
             $columnAggregationOps,
             function (Row $flatRow) use ($flatToHierarchyPathCallback, $archiveProcessor, $hierarchicalRecord) {
@@ -410,7 +410,8 @@ function (Row $flatRow) use ($flatToHierarchyPathCallback, $archiveProcessor, $h
      * the hierarchical blob record is serialized and inserted.
      *
      * Intended for plugin-specific finalization (for example, metadata or column cleanup) when
-     * using setBuiltFromFlatRecord().
+     * using setBuiltFromFlatRecord(). The flat table has already been serialized at this point
+     * and may have been fully consumed while rebuilding the hierarchy.
      */
     protected function beforeInsertBuiltFromFlatHierarchyRecord(
         ArchiveProcessor $archiveProcessor,
@@ -465,6 +466,47 @@ protected function buildHierarchicalTableFromFlatTable(
         return $hierarchicalTable;
     }
 
+    protected function buildHierarchicalTableFromFlatTableAndConsumeRows(
+        DataTable $flatTable,
+        ?array $columnAggregationOps,
+        callable $flatToHierarchyPathCallback,
+        array $defaultHierarchyRowColumns = []
+    ): DataTable {
+        $hierarchicalTable = new DataTable();
+        if (!empty($columnAggregationOps)) {
+            $hierarchicalTable->setMetadata(DataTable::COLUMN_AGGREGATION_OPS_METADATA_NAME, $columnAggregationOps);
+        }
+
+        while (($flatRow = $flatTable->shiftRow()) instanceof Row) {
+            $path = call_user_func($flatToHierarchyPathCallback, $flatRow);
+            if (is_array($path) && !empty($path)) {
+                [$destinationRow, $level] = $hierarchicalTable->walkPath($path, $defaultHierarchyRowColumns, 0);
+                if ($destinationRow instanceof Row) {
+                    $this->sumRowIntoDestination($flatRow, $destinationRow, $columnAggregationOps);
+                }
+            }
+
+            Common::destroy($flatRow);
+        }
+
+        $summaryRow = $flatTable->getSummaryRow();
+        if ($summaryRow instanceof Row && !$this->isSummaryRowEmpty($summaryRow)) {
+            $destinationSummaryRow = $hierarchicalTable->getRowFromId(DataTable::ID_SUMMARY_ROW);
+            if ($destinationSummaryRow === false) {
+                $destinationSummaryRow = clone $summaryRow;
+                $destinationSummaryRow->setIsSummaryRow();
+                $hierarchicalTable->addSummaryRow($destinationSummaryRow);
+            } else {
+                $this->sumRowIntoDestination($summaryRow, $destinationSummaryRow, $columnAggregationOps);
+            }
+        }
+
+        $flatTable->deleteRow(DataTable::ID_SUMMARY_ROW);
+        Common::destroy($summaryRow);
+
+        return $hierarchicalTable;
+    }
+
     protected function sumRowIntoDestination(Row $source, Row $destination, ?array $columnAggregationOps): void
     {
         $sourceCopy = clone $source;

core/DataTable.php

@@ -965,6 +965,30 @@ public function getRowsWithoutSummaryRow()
         return $this->rows;
     }
 
+    /**
+     * Removes and returns the first non-summary row in this table.
+     *
+     * @return Row|null
+     */
+    public function shiftRow()
+    {
+        if (empty($this->rows)) {
+            return null;
+        }
+
+        reset($this->rows);
+        $rowId = key($this->rows);
+
+        if (!isset($this->rows[$rowId])) {
+            return null;
+        }
+
+        $row = $this->rows[$rowId];
+        unset($this->rows[$rowId]);
+
+        return $row;
+    }
+
     /**
      * @return int
      * @ignore

tests/PHPUnit/Unit/ArchiveProcessor/RecordBuilderTest.php

@@ -721,6 +721,84 @@ protected function getAllSubperiodKeys(ArchiveProcessor $archiveProcessor): arra
         $this->assertSame(['/flat-path', '/legacy-path'], $hierarchyLabels);
     }
 
+    public function testBuildForNonDayPeriodConsumesFlatTableBeforePreInsertHook(): void
+    {
+        $hookState = (object) ['flatRowsAtHook' => null, 'hierarchyLabelsAtHook' => []];
+
+        $recordBuilder = new class ($hookState) extends ArchiveProcessor\RecordBuilder {
+            private $hookState;
+
+            public function __construct(object $hookState)
+            {
+                parent::__construct();
+                $this->hookState = $hookState;
+            }
+
+            public function getRecordMetadata(ArchiveProcessor $archiveProcessor): array
+            {
+                return [
+                    Record::make(Record::TYPE_BLOB, 'TestPlugin_hierarchy')
+                        ->setBuiltFromFlatRecord('TestPlugin_flat', function (Row $flatRow): ?array {
+                            $label = $flatRow->getColumn('label');
+                            if (!is_string($label) || $label === '') {
+                                return null;
+                            }
+
+                            return [$label];
+                        }),
+                    Record::make(Record::TYPE_BLOB, 'TestPlugin_flat'),
+                ];
+            }
+
+            protected function aggregate(ArchiveProcessor $archiveProcessor): array
+            {
+                return [];
+            }
+
+            protected function aggregateRootDataTableFromBlobs(
+                ArchiveProcessor $archiveProcessor,
+                string $recordName,
+                ?array $columnsAggregationOperation,
+                ?array $columnsToRenameAfterAggregation
+            ): array {
+                $table = new DataTable();
+                if ($recordName === 'TestPlugin_flat') {
+                    $table->addRowFromSimpleArray(['label' => '/flat-path-a', 'nb_visits' => 5]);
+                    $table->addRowFromSimpleArray(['label' => '/flat-path-b', 'nb_visits' => 3]);
+                    $table->addSummaryRow(new Row([Row::COLUMNS => ['label' => '-1', 'nb_visits' => 2]]));
+
+                    return [$table, true, ['2020-03-04,2020-03-04' => true]];
+                }
+
+                return [$table, false, []];
+            }
+
+            protected function getAllSubperiodKeys(ArchiveProcessor $archiveProcessor): array
+            {
+                return ['2020-03-04,2020-03-04' => true];
+            }
+
+            protected function beforeInsertBuiltFromFlatHierarchyRecord(
+                ArchiveProcessor $archiveProcessor,
+                Record $hierarchicalRecord,
+                DataTable $hierarchicalTable,
+                DataTable $flatTable
+            ): void {
+                $this->hookState->flatRowsAtHook = $flatTable->getRowsCount();
+                $this->hookState->hierarchyLabelsAtHook = $hierarchicalTable->getColumn('label');
+            }
+        };
+
+        $mockArchiveProcessor = $this->getMockArchiveProcessor('week', ['TestPlugin_flat']);
+        $recordBuilder->buildForNonDayPeriod($mockArchiveProcessor);
+
+        $this->assertSame(0, $hookState->flatRowsAtHook);
+        $this->assertSame(['/flat-path-a', '/flat-path-b', '-1'], $hookState->hierarchyLabelsAtHook);
+
+        $this->assertSame(['/flat-path-a', '/flat-path-b', '-1'], $this->getTopLevelLabelsOfInsertedBlobRecord('TestPlugin_flat'));
+        $this->assertSame(['/flat-path-a', '/flat-path-b', '-1'], $this->getTopLevelLabelsOfInsertedBlobRecord('TestPlugin_hierarchy'));
+    }
+
     public function testBuildForNonDayPeriodCorrectlyAggregatesMetricsForMetricsThatAreRowCountsOfRecords()
     {
         $recordBuilder = new class () extends ArchiveProcessor\RecordBuilder {