Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove doctrine annotation reader #203

Merged
merged 24 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
"tests": ["@cs", "@test", "@sa"],
"coverage": ["php -dzend_extension=xdebug.so -dxdebug.mode=coverage phpunit --coverage-text --coverage-html=build/coverage"],
"pcov": ["php -dextension=pcov.so -d pcov.enabled=1 phpunit --coverage-text --coverage-html=build/coverage --coverage-clover=coverage.xml"],
"cs": ["phpcs --standard=./phpcs.xml src tests"],
"cs-fix": ["phpcbf src tests"],
"cs": ["phpcs --standard=./phpcs.xml sl-src src tests"],
"cs-fix": ["phpcbf sl-src src tests"],
"clean": ["phpstan clear-result-cache", "psalm --clear-cache", "rm -rf tests/tmp/*.php"],
"sa": ["psalm --monochrome --show-info=true", "phpstan --memory-limit=-1 analyse -c phpstan.neon"],
"metrics": ["phpmetrics --report-html=build/metrics --exclude=Exception src"],
Expand Down
101 changes: 101 additions & 0 deletions sl-src/Cache.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<?php

declare(strict_types=1);

namespace Ray\ServiceLocator;

use Ray\ServiceLocator\Exception\DirectoryNotWritableException;

use function file_exists;
use function file_put_contents;
use function hash;
use function is_writable;
use function mkdir;
use function pathinfo;
use function rename;
use function serialize;
use function substr;
use function tempnam;
use function unlink;
use function unserialize;

use const DIRECTORY_SEPARATOR;
use const PATHINFO_DIRNAME;

/**
* Minimal cache
*/
final class Cache
{
/** @var string */
private $tmpDir;

public function __construct(string $tmpDir)
{
$this->tmpDir = $tmpDir;
}

/**
* @psalm-param callable():array<object> $callback
*
* @return array<object>
*
* @psalm-suppress MixedInferredReturnType
*/
public function get(string $key, callable $callback): array
{
$filename = $this->getFilename($key);
if (! file_exists($filename)) {
$value = $callback();
$this->writeFile($this->getFilename($key), serialize($value));

return $value;
}

/** @psalm-suppress MixedAssignment, MixedArgument, MixedReturnStatement */
return unserialize(require $filename); // @phpstan-ignore-line
}

private function getFilename(string $id): string
{
$hash = hash('crc32', $id);

$dir = $this->tmpDir
. DIRECTORY_SEPARATOR
. substr($hash, 0, 2);
if (! is_writable($dir)) {
mkdir($dir, 0777, true);
}

return $dir
. DIRECTORY_SEPARATOR
. $hash
. '.php';
}

private function writeFile(string $filename, string $value): void
{
$filepath = pathinfo($filename, PATHINFO_DIRNAME);
if (! is_writable($filepath)) {
// @codeCoverageIgnoreStart
throw new DirectoryNotWritableException($filepath);
// @codeCoverageIgnoreEnd
}

$tmpFile = (string) tempnam($filepath, 'swap');
$valueWithSingileQuote = "'{$value}'";

$content = '<?php return ' . $valueWithSingileQuote . ';';
if (file_put_contents($tmpFile, $content) !== false) {
if (@rename($tmpFile, $filename)) {
return;
}

// @codeCoverageIgnoreStart
@unlink($tmpFile);
}

throw new DirectoryNotWritableException($filepath);
// @codeCoverageIgnoreEnd
}
}
106 changes: 35 additions & 71 deletions sl-src/PsrCachedReader.php → sl-src/CacheReader.php
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
<?php

namespace Doctrine\Common\Annotations;
declare(strict_types=1);

use Psr\Cache\CacheItemPoolInterface;
namespace Ray\ServiceLocator;

use Doctrine\Common\Annotations\Reader;
use LogicException;
use ReflectionClass;
use ReflectionMethod;
use ReflectionProperty;
Expand All @@ -14,39 +17,38 @@
use function filemtime;
use function max;
use function rawurlencode;
use function time;

/**
* A cache aware annotation reader.
* Minimal cache aware annotation reader
*
* This code is taken from original PsrCachedReader.php in doctrine/annotation and modified.
*
* @see https://github.com/doctrine/annotations/commits/2.0.x/lib/Doctrine/Common/Annotations/PsrCachedReader.php
*/
final class PsrCachedReader implements Reader
final class CacheReader implements Reader
{
/** @var Reader */
private $delegate;

/** @var CacheItemPoolInterface */
/** @var Cache */
private $cache;

/** @var bool */
private $debug;

/** @var array<string, array<object>> */
private $loadedAnnotations = [];

/** @var int[] */
private $loadedFilemtimes = [];

public function __construct(Reader $reader, CacheItemPoolInterface $cache, bool $debug = false)
public function __construct(Reader $reader, Cache $cache)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache も interface があれば、例えば APCu に変更したいときに APCuCache だけを実装すればOKになるかなと思いましたがどうですかね。。?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

アノテーションリーダーからアトリビュートの過渡期で、より高速を望むなら、

A) rectorなどでアトリビュートにするか または B)オリジナルのキャッシュリーダー PsrCachedReader を使うか

という選択肢があり、ここでB)を選択しなかったのは「他のライブラリに依存するより最小で自己完結的にした方が良い」と考えたからです。ここで新しいインターフェイスを導入するのは"自己完結"に背くような気もして入れてない感じです。いつもは可能な限り汎用品を使って租結合にしてますが、ここは違うところでは?と考えました。どうでしょうか?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"シナリオ"をパフォーマンスに含めました。

eb89d47

{
$this->delegate = $reader;
$this->cache = $cache;
$this->debug = (bool) $debug;
}

/**
* {@inheritDoc}
*/
public function getClassAnnotations(ReflectionClass $class)
public function getClassAnnotations(ReflectionClass $class) // @phpstan-ignore-line
{
$cacheKey = $class->getName();

Expand All @@ -62,7 +64,7 @@ public function getClassAnnotations(ReflectionClass $class)
/**
* {@inheritDoc}
*/
public function getClassAnnotation(ReflectionClass $class, $annotationName)
public function getClassAnnotation(ReflectionClass $class, $annotationName) // @phpstan-ignore-line
{
foreach ($this->getClassAnnotations($class) as $annot) {
if ($annot instanceof $annotationName) {
Expand All @@ -78,30 +80,15 @@ public function getClassAnnotation(ReflectionClass $class, $annotationName)
*/
public function getPropertyAnnotations(ReflectionProperty $property)
{
$class = $property->getDeclaringClass();
$cacheKey = $class->getName() . '$' . $property->getName();

if (isset($this->loadedAnnotations[$cacheKey])) {
return $this->loadedAnnotations[$cacheKey];
}

$annots = $this->fetchFromCache($cacheKey, $class, 'getPropertyAnnotations', $property);

return $this->loadedAnnotations[$cacheKey] = $annots;
throw new LogicException(__FUNCTION__ . ' Not Supported');
}

/**
* {@inheritDoc}
*/
public function getPropertyAnnotation(ReflectionProperty $property, $annotationName)
{
foreach ($this->getPropertyAnnotations($property) as $annot) {
if ($annot instanceof $annotationName) {
return $annot;
}
}

return null;
throw new LogicException(__FUNCTION__ . ' Not Supported');
}

/**
Expand Down Expand Up @@ -135,56 +122,33 @@ public function getMethodAnnotation(ReflectionMethod $method, $annotationName)
return null;
}

public function clearLoadedAnnotations(): void
{
$this->loadedAnnotations = [];
$this->loadedFilemtimes = [];
}

/** @return mixed[] */
private function fetchFromCache(
/**
* @return array<object>
*
* @psalm-suppress MixedInferredReturnType
*/
private function fetchFromCache( // @phpstan-ignore-line
string $cacheKey,
ReflectionClass $class,
string $method,
Reflector $reflector
): array {
$cacheKey = rawurlencode($cacheKey);

$item = $this->cache->getItem($cacheKey);
if (($this->debug && ! $this->refresh($cacheKey, $class)) || ! $item->isHit()) {
$this->cache->save($item->set($this->delegate->{$method}($reflector)));
}

return $item->get();
}

/**
* Used in debug mode to check if the cache is fresh.
*
* @return bool Returns true if the cache was fresh, or false if the class
* being read was modified since writing to the cache.
*/
private function refresh(string $cacheKey, ReflectionClass $class): bool
{
$lastModification = $this->getLastModification($class);
if ($lastModification === 0) {
return true;
}

$item = $this->cache->getItem('[C]' . $cacheKey);
if ($item->isHit() && $item->get() >= $lastModification) {
return true;
}

$this->cache->save($item->set(time()));

return false;
$cacheKey = rawurlencode($cacheKey) . $this->getLastModification($class);

return $this->cache->get(
$cacheKey,
/** @return array<object> */
function () use ($method, $reflector): array {
/** @psalm-suppress MixedReturnStatement */
return $this->delegate->{$method}($reflector);
}
);
}

/**
* Returns the time the class was last modified, testing traits and parents
*/
private function getLastModification(ReflectionClass $class): int
private function getLastModification(ReflectionClass $class): int // @phpstan-ignore-line
{
$filename = $class->getFileName();

Expand All @@ -210,7 +174,7 @@ private function getLastModification(ReflectionClass $class): int
return $this->loadedFilemtimes[$filename] = $lastModification;
}

private function getTraitLastModificationTime(ReflectionClass $reflectionTrait): int
private function getTraitLastModificationTime(ReflectionClass $reflectionTrait): int // @phpstan-ignore-line
{
$fileName = $reflectionTrait->getFileName();

Expand Down
11 changes: 11 additions & 0 deletions sl-src/Exception/DirectoryNotWritableException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Ray\ServiceLocator\Exception;

use RuntimeException;

final class DirectoryNotWritableException extends RuntimeException
{
}
13 changes: 9 additions & 4 deletions sl-src/ServiceLocator.php
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
<?php

declare(strict_types=1);

namespace Ray\ServiceLocator;

use Doctrine\Common\Annotations\AnnotationReader;
use Doctrine\Common\Annotations\Reader;
use Koriym\Attributes\AttributeReader;
use Koriym\Attributes\DualReader;

use function sys_get_temp_dir;

final class ServiceLocator
{
/**
* @var ?Reader
*/
/** @var ?Reader */
private static $reader;

public static function setReader(Reader $reader): void
Expand All @@ -22,7 +24,10 @@ public static function setReader(Reader $reader): void
public static function getReader(): Reader
{
if (! self::$reader) {
self::$reader = new DualReader(new AnnotationReader(), new AttributeReader());
self::$reader = new CacheReader(
new DualReader(new AnnotationReader(), new AttributeReader()),
new Cache(sys_get_temp_dir())
);
}

return self::$reader;
Expand Down