Skip to content

Commit

Permalink
Improve performance for a few Rows methods (#671)
Browse files Browse the repository at this point in the history
* Improve performance for a few `Rows` methods
Covered: `find`, `findOne`, `first`, `partitionBy`, `take`, `takeRight`

* Rework benchmark GH action to use artifact for baseline
  • Loading branch information
stloyd committed Oct 30, 2023
1 parent 74e9f52 commit 8af1c62
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 54 deletions.
14 changes: 5 additions & 9 deletions .github/workflows/test-benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,12 @@ jobs:
- name: "Install locked dependencies"
run: "composer install --no-interaction --no-progress"

- name: "Restore PHPBench cache"
uses: "actions/cache@v3"
- name: "Download phpbench benchmarks artifact"
uses: dawidd6/action-download-artifact@v2
with:
path: "var/phpbench"
key: "php-${{ matrix.php-version }}-1.x-phpbench-"
restore-keys: |
php-${{ matrix.php-version }}-1.x-phpbench-
- name: "Clean previous summary"
run: rm -rf ./var/phpbench/summary.txt
workflow: benchmark-baseline.yml
name: phpbench-baseline
path: ./var/phpbench/

- name: "Execute benchmarks"
id: init_comment
Expand Down
54 changes: 10 additions & 44 deletions src/core/etl/src/Flow/ETL/Rows.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ public function filter(callable $callable) : self

public function find(callable $callable) : self
{
$rows = $this->rows;

if (!\count($rows)) {
if (0 === $this->count()) {
return new self();
}

Expand All @@ -209,34 +207,18 @@ public function find(callable $callable) : self

public function findOne(callable $callable) : ?Row
{
$rows = $this->rows;

if (!\count($rows)) {
return null;
}

$rows = [];

foreach ($this->rows as $row) {
if ($callable($row)) {
$rows[] = $row;
return $row;
}
}

if ([] !== $rows) {
return \current($rows);
}

return null;
}

public function first() : Row
{
if (empty($this->rows)) {
throw new RuntimeException('First row does not exist in empty collection');
}

return $this->rows[0];
return $this->rows[0] ?? throw new RuntimeException('First row does not exist in empty collection');
}

/**
Expand Down Expand Up @@ -509,11 +491,11 @@ public function offsetSet(mixed $offset, mixed $value) : void
/**
* @param int $offset
*
* @throws InvalidArgumentException
* @throws RuntimeException
*/
public function offsetUnset(mixed $offset) : void
{
throw new RuntimeException('In order to add new rows use Rows::remove(int $offset) : self');
throw new RuntimeException('In order to remove rows use Rows::remove(int $offset) : self');
}

/**
Expand All @@ -526,9 +508,7 @@ public function offsetUnset(mixed $offset) : void
*/
public function partitionBy(string|Reference $entry, string|Reference ...$entries) : array
{
\array_unshift($entries, $entry);

$refs = References::init(...$entries);
$refs = References::init($entry, ...$entries);

/** @var array<string, array<mixed>> $partitions */
$partitions = [];
Expand All @@ -537,10 +517,10 @@ public function partitionBy(string|Reference $entry, string|Reference ...$entrie
$partitionEntryValues = [];

foreach ($this->rows as $row) {
$partitionEntryValues[] = $row->get($ref)->value();
$partitionEntryValues[$row->get($ref)->value()] = true;
}

$partitions[$ref->name()] = \array_unique($partitionEntryValues);
$partitions[$ref->name()] = \array_keys($partitionEntryValues);
}

/**
Expand Down Expand Up @@ -723,26 +703,12 @@ public function sortEntries() : self

public function take(int $size) : self
{
$rows = $this->rows;
$newRows = [];

for ($i = 0; $i < $size; $i++) {
$newRows[] = \array_shift($rows);
}

return new self(...\array_filter($newRows));
return new self(...\array_slice($this->rows, 0, $size));
}

public function takeRight(int $size) : self
{
$rows = $this->rows;
$newRows = [];

for ($i = 0; $i < $size; $i++) {
$newRows[] = \array_pop($rows);
}

return new self(...\array_filter($newRows));
return new self(...\array_reverse(\array_slice($this->rows, -$size, $size)));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/core/etl/tests/Flow/ETL/Tests/Unit/RowsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public function test_array_access_set() : void
public function test_array_access_unset() : void
{
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('In order to add new rows use Rows::remove(int $offset) : self');
$this->expectExceptionMessage('In order to remove rows use Rows::remove(int $offset) : self');
$rows = new Rows(Row::create(new IntegerEntry('id', 1)));
unset($rows[0]);
}
Expand Down

0 comments on commit 8af1c62

Please sign in to comment.