Skip to content

Commit

Permalink
Microoptimizations (#14)
Browse files Browse the repository at this point in the history
  • Loading branch information
PabloKowalczyk authored Jul 10, 2022
1 parent dc40934 commit 68319d3
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 144 deletions.
11 changes: 8 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@
}
},
"scripts": {
"bench-01": "docker compose -f ./perf/docker-compose.yml run --rm cli_php blackfire --samples 40 run php -d zend.assertions=-1 01-encode-simple-single-resource.php",
"bench-02": "docker compose -f ./perf/docker-compose.yml run --rm cli_php blackfire --samples 40 run php -d zend.assertions=-1 02-encode-long-list-of-simple-resources.php",
"checks": [
"@test-cs-fixer",
"@phpstan:check",
"@php vendor/bin/phpunit"
],
"cs-fixer": "./vendor/bin/php-cs-fixer fix --diff -v --ansi",
"phpstan:check": "@php vendor/bin/phpstan analyse -c phpstan.neon src tests",
"test": [
Expand All @@ -75,8 +82,6 @@
"test-cs-fixer": "./vendor/bin/php-cs-fixer fix --diff --dry-run -v",
"test-md": "./vendor/bin/phpmd ./src text codesize,controversial,cleancode,design,unusedcode,naming",
"test-unit": "./vendor/phpunit/phpunit/phpunit --coverage-text",
"test-unit-phpdbg": "phpdbg -qrr ./vendor/bin/phpunit --coverage-text",
"bench-01": "docker compose -f ./perf/docker-compose.yml run --rm cli_php blackfire --samples 10 run php -d zend.assertions=-1 01-encode-simple-single-resource.php",
"bench-02": "docker compose -f ./perf/docker-compose.yml run --rm cli_php blackfire --samples 10 run php -d zend.assertions=-1 02-encode-long-list-of-simple-resources.php"
"test-unit-phpdbg": "phpdbg -qrr ./vendor/bin/phpunit --coverage-text"
}
}
15 changes: 15 additions & 0 deletions perf/src/CitySchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,19 @@ public function getRelationships($resource, ContextInterface $context): iterable
{
return [];
}

public function isAddSelfLinkInRelationshipByDefault(string $relationshipName): bool
{
return false;
}

public function isAddRelatedLinkInRelationshipByDefault(string $relationshipName): bool
{
return false;
}

public function getLinks($resource): iterable
{
return [];
}
}
35 changes: 0 additions & 35 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1230,16 +1230,6 @@ parameters:
count: 1
path: src/Representation/FieldSetFilter.php

-
message: "#^Parameter \\#1 \\$type of method Neomerx\\\\JsonApi\\\\Representation\\\\FieldSetFilter\\:\\:getAllowedFields\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/Representation/FieldSetFilter.php

-
message: "#^Parameter \\#1 \\$type of method Neomerx\\\\JsonApi\\\\Representation\\\\FieldSetFilter\\:\\:hasFilter\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/Representation/FieldSetFilter.php

-
message: "#^Property Neomerx\\\\JsonApi\\\\Representation\\\\FieldSetFilter\\:\\:\\$fieldSets type has no value type specified in iterable type array\\.$#"
count: 1
Expand Down Expand Up @@ -1455,16 +1445,6 @@ parameters:
count: 1
path: src/Schema/LinkWithAliases.php

-
message: "#^Call to function assert\\(\\) with true and 'Unable to get a…' will always evaluate to true\\.$#"
count: 1
path: src/Schema/SchemaContainer.php

-
message: "#^Call to function is_object\\(\\) with object will always evaluate to true\\.$#"
count: 1
path: src/Schema/SchemaContainer.php

-
message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
count: 2
Expand Down Expand Up @@ -1515,21 +1495,6 @@ parameters:
count: 1
path: src/Schema/SchemaContainer.php

-
message: "#^Property Neomerx\\\\JsonApi\\\\Schema\\\\SchemaContainer\\:\\:\\$resType2JsonType is never read, only written\\.$#"
count: 1
path: src/Schema/SchemaContainer.php

-
message: "#^Property Neomerx\\\\JsonApi\\\\Schema\\\\SchemaContainer\\:\\:\\$resType2JsonType type has no value type specified in iterable type array\\.$#"
count: 1
path: src/Schema/SchemaContainer.php

-
message: "#^Strict comparison using \\=\\=\\= between true and true will always evaluate to true\\.$#"
count: 1
path: src/Schema/SchemaContainer.php

-
message: "#^Class Neomerx\\\\Tests\\\\JsonApi\\\\Data\\\\Collection implements generic interface ArrayAccess but does not specify its types\\: TKey, TValue$#"
count: 1
Expand Down
10 changes: 5 additions & 5 deletions src/Parser/IdentifierAndResource.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ public function getIdentifierMeta()
*/
public function getAttributes(): iterable
{
$this->getContext()->setPosition($this->getPosition());
$this->context->setPosition($this->position);

return $this->schema->getAttributes($this->data, $this->getContext());
return $this->schema->getAttributes($this->data, $this->context);
}

/**
Expand All @@ -163,14 +163,14 @@ public function getRelationships(): iterable
$currentPath = $this->position->getPath();
$nextLevel = $this->position->getLevel() + 1;
$nextPathPrefix = true === empty($currentPath) ? '' : $currentPath . PositionInterface::PATH_SEPARATOR;
$this->getContext()->setPosition($this->getPosition());
foreach ($this->schema->getRelationships($this->data, $this->getContext()) as $name => $description) {
$this->context->setPosition($this->position);
foreach ($this->schema->getRelationships($this->data, $this->context) as $name => $description) {
\assert(true === $this->assertRelationshipNameAndDescription($name, $description));

[$hasData, $relationshipData, $nextPosition] = $this->parseRelationshipData(
$this->factory,
$this->schemaContainer,
$this->getContext(),
$this->context,
$this->type,
$name,
$description,
Expand Down
40 changes: 40 additions & 0 deletions src/Parser/ParsedDocumentData.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

namespace Neomerx\JsonApi\Parser;

use Neomerx\JsonApi\Contracts\Parser\DocumentDataInterface;
use Neomerx\JsonApi\Contracts\Schema\PositionInterface;

final class ParsedDocumentData implements DocumentDataInterface
{
private PositionInterface $position;
private bool $isCollection;
private bool $isNull;

public function __construct(
PositionInterface $position,
bool $isCollection,
bool $isNull
) {
$this->position = $position;
$this->isCollection = $isCollection;
$this->isNull = $isNull;
}

public function getPosition(): PositionInterface
{
return $this->position;
}

public function isCollection(): bool
{
return $this->isCollection;
}

public function isNull(): bool
{
return $this->isNull;
}
}
84 changes: 28 additions & 56 deletions src/Parser/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Parser implements ParserInterface

private ?array $paths = null;

private array $resourcesTracker;
private array $resourcesTracker = [];

private \Neomerx\JsonApi\Contracts\Parser\EditableContextInterface $context;

Expand All @@ -72,7 +72,6 @@ public function __construct(
SchemaContainerInterface $container,
EditableContextInterface $context
) {
$this->resourcesTracker = [];
$this->factory = $factory;
$this->schemaContainer = $container;
$this->context = $context;
Expand Down Expand Up @@ -179,7 +178,9 @@ private function parseAsResource(
*/
private function parseResource(ResourceInterface $resource): iterable
{
$seenBefore = isset($this->resourcesTracker[$resource->getId()][$resource->getType()]);
$id = $resource->getId();
$type = $resource->getType();
$seenBefore = isset($this->resourcesTracker[$id][$type]);

// top level resources should be yielded in any case as it could be an array of the resources
// for deeper levels it's not needed as they go to `included` section and it must have no more
Expand All @@ -192,7 +193,7 @@ private function parseResource(ResourceInterface $resource): iterable
// parse relationships only for resources not seen before (prevents infinite loop for circular references)
if (false === $seenBefore) {
// remember by id and type
$this->resourcesTracker[$resource->getId()][$resource->getType()] = true;
$this->resourcesTracker[$id][$type] = true;

foreach ($resource->getRelationships() as $name => $relationship) {
\assert(\is_string($name));
Expand All @@ -206,7 +207,9 @@ private function parseResource(ResourceInterface $resource): iterable
yield from $this->parseResource($relData->getResource());

continue;
} elseif (true === $relData->isCollection()) {
}

if (true === $relData->isCollection()) {
foreach ($relData->getResources() as $relResource) {
\assert($relResource instanceof ResourceInterface ||
$relResource instanceof IdentifierInterface);
Expand Down Expand Up @@ -283,69 +286,38 @@ public function getIdentifierMeta()

private function createDocumentDataIsCollection(PositionInterface $position): DocumentDataInterface
{
return $this->createParsedDocumentData($position, true, false);
return new ParsedDocumentData(
$position,
true,
false,
);
}

private function createDocumentDataIsNull(PositionInterface $position): DocumentDataInterface
{
return $this->createParsedDocumentData($position, false, true);
return new ParsedDocumentData(
$position,
false,
true,
);
}

private function createDocumentDataIsResource(PositionInterface $position): DocumentDataInterface
{
return $this->createParsedDocumentData($position, false, false);
return new ParsedDocumentData(
$position,
false,
false,
);
}

private function createDocumentDataIsIdentifier(PositionInterface $position): DocumentDataInterface
{
return $this->createParsedDocumentData($position, false, false);
}

private function createParsedDocumentData(
PositionInterface $position,
bool $isCollection,
bool $isNull
): DocumentDataInterface {
return new class($position, $isCollection, $isNull) implements DocumentDataInterface {
private \Neomerx\JsonApi\Contracts\Schema\PositionInterface $position;
private bool $isCollection;

private bool $isNull;

public function __construct(
PositionInterface $position,
bool $isCollection,
bool $isNull
) {
$this->position = $position;
$this->isCollection = $isCollection;
$this->isNull = $isNull;
}

/**
* {@inheritdoc}
*/
public function getPosition(): PositionInterface
{
return $this->position;
}

/**
* {@inheritdoc}
*/
public function isCollection(): bool
{
return $this->isCollection;
}

/**
* {@inheritdoc}
*/
public function isNull(): bool
{
return $this->isNull;
}
};
return new ParsedDocumentData(
$position,
false,
false,
);
}

private function normalizePaths(iterable $paths): array
Expand Down
13 changes: 5 additions & 8 deletions src/Representation/FieldSetFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@

class FieldSetFilter implements FieldSetFilterInterface
{
private array $fieldSets;
private array $fieldSets = [];

/**
* @param array|null $fieldSets
*/
public function __construct(array $fieldSets)
{
$this->fieldSets = [];

foreach ($fieldSets as $type => $fields) {
\assert(true === \is_string($type) && false === empty($type));
\assert(true === \is_iterable($fields));
Expand Down Expand Up @@ -72,8 +70,8 @@ public function getRelationships(ResourceInterface $resource): iterable
public function shouldOutputRelationship(PositionInterface $position): bool
{
$parentType = $position->getParentType();
if (true === $this->hasFilter($parentType)) {
return isset($this->getAllowedFields($parentType)[$position->getParentRelationship()]);
if (true === isset($this->fieldSets[$parentType])) {
return isset($this->fieldSets[$parentType][$position->getParentRelationship()]);
}

return true;
Expand All @@ -93,15 +91,14 @@ protected function getAllowedFields(string $type): array

protected function filterFields(string $type, iterable $fields): iterable
{
if (false === $this->hasFilter($type)) {
if (false === isset($this->fieldSets[$type])) {
yield from $fields;

return;
}

$allowedFields = $this->getAllowedFields($type);
foreach ($fields as $name => $value) {
if (true === isset($allowedFields[$name])) {
if (true === isset($this->fieldSets[$type][$name])) {
yield $name => $value;
}
}
Expand Down
Loading

0 comments on commit 68319d3

Please sign in to comment.