From 68319d3861e8f40268c6028df03ecbe158633dfb Mon Sep 17 00:00:00 2001 From: PabloKowalczyk <11366345+PabloKowalczyk@users.noreply.github.com> Date: Sun, 10 Jul 2022 14:09:46 +0000 Subject: [PATCH] Microoptimizations (#14) --- composer.json | 11 +++- perf/src/CitySchema.php | 15 +++++ phpstan-baseline.neon | 35 ----------- src/Parser/IdentifierAndResource.php | 10 ++-- src/Parser/ParsedDocumentData.php | 40 +++++++++++++ src/Parser/Parser.php | 84 +++++++++------------------ src/Representation/FieldSetFilter.php | 13 ++--- src/Schema/SchemaContainer.php | 41 ++----------- 8 files changed, 105 insertions(+), 144 deletions(-) create mode 100644 src/Parser/ParsedDocumentData.php diff --git a/composer.json b/composer.json index 6a59bb6..205e3cf 100644 --- a/composer.json +++ b/composer.json @@ -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": [ @@ -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" } } diff --git a/perf/src/CitySchema.php b/perf/src/CitySchema.php index f8dc21d..7484a0e 100644 --- a/perf/src/CitySchema.php +++ b/perf/src/CitySchema.php @@ -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 []; + } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 62b5608..6fb2be5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -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 @@ -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 @@ -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 diff --git a/src/Parser/IdentifierAndResource.php b/src/Parser/IdentifierAndResource.php index 353b015..7e9f61f 100644 --- a/src/Parser/IdentifierAndResource.php +++ b/src/Parser/IdentifierAndResource.php @@ -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); } /** @@ -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, diff --git a/src/Parser/ParsedDocumentData.php b/src/Parser/ParsedDocumentData.php new file mode 100644 index 0000000..be215ce --- /dev/null +++ b/src/Parser/ParsedDocumentData.php @@ -0,0 +1,40 @@ +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; + } +} diff --git a/src/Parser/Parser.php b/src/Parser/Parser.php index 14d2709..4463aba 100644 --- a/src/Parser/Parser.php +++ b/src/Parser/Parser.php @@ -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; @@ -72,7 +72,6 @@ public function __construct( SchemaContainerInterface $container, EditableContextInterface $context ) { - $this->resourcesTracker = []; $this->factory = $factory; $this->schemaContainer = $container; $this->context = $context; @@ -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 @@ -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)); @@ -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); @@ -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 diff --git a/src/Representation/FieldSetFilter.php b/src/Representation/FieldSetFilter.php index b1e5999..7600ee9 100644 --- a/src/Representation/FieldSetFilter.php +++ b/src/Representation/FieldSetFilter.php @@ -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)); @@ -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; @@ -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; } } diff --git a/src/Schema/SchemaContainer.php b/src/Schema/SchemaContainer.php index b0e800b..e039818 100644 --- a/src/Schema/SchemaContainer.php +++ b/src/Schema/SchemaContainer.php @@ -51,8 +51,6 @@ class SchemaContainer implements SchemaContainerInterface */ private array $createdProviders = []; - private array $resType2JsonType = []; - private \Neomerx\JsonApi\Contracts\Factories\FactoryInterface $factory; public function __construct(FactoryInterface $factory, iterable $schemas) @@ -96,7 +94,6 @@ public function register(string $type, $schema): void if ($schema instanceof SchemaInterface) { $this->setProviderMapping($type, \get_class($schema)); - $this->setResourceToJsonTypeMapping($schema->getType(), $type); $this->setCreatedProvider($type, $schema); } else { $this->setProviderMapping($type, $schema); @@ -120,7 +117,7 @@ public function getSchema($resource): SchemaInterface { \assert($this->hasSchema($resource)); - $resourceType = $this->getResourceType($resource); + $resourceType = \get_class($resource); return $this->getSchemaByType($resourceType); } @@ -131,7 +128,7 @@ public function getSchema($resource): SchemaInterface public function hasSchema($resourceObject): bool { return true === \is_object($resourceObject) && - true === $this->hasProviderMapping($this->getResourceType($resourceObject)); + true === $this->hasProviderMapping(\get_class($resourceObject)); } /** @@ -142,8 +139,8 @@ public function hasSchema($resourceObject): bool */ protected function getSchemaByType(string $type): SchemaInterface { - if (true === $this->hasCreatedProvider($type)) { - return $this->getCreatedProvider($type); + if (true === isset($this->createdProviders[$type])) { + return $this->createdProviders[$type]; } $classNameOrCallable = $this->getProviderMapping($type); @@ -157,8 +154,6 @@ protected function getSchemaByType(string $type): SchemaInterface /* @var SchemaInterface $schema */ - $this->setResourceToJsonTypeMapping($schema->getType(), $type); - return $schema; } @@ -183,39 +178,11 @@ protected function setProviderMapping(string $type, $schema): void $this->providerMapping[$type] = $schema; } - protected function hasCreatedProvider(string $type): bool - { - return isset($this->createdProviders[$type]); - } - - protected function getCreatedProvider(string $type): SchemaInterface - { - return $this->createdProviders[$type]; - } - protected function setCreatedProvider(string $type, SchemaInterface $provider): void { $this->createdProviders[$type] = $provider; } - protected function setResourceToJsonTypeMapping(string $resourceType, string $jsonType): void - { - $this->resType2JsonType[$resourceType] = $jsonType; - } - - /** - * @param object $resource - */ - protected function getResourceType($resource): string - { - \assert( - true === \is_object($resource), - 'Unable to get a type of the resource as it is not an object.' - ); - - return \get_class($resource); - } - protected function createSchemaFromCallable(callable $callable): SchemaInterface { $schema = \call_user_func($callable, $this->factory);