Skip to content

Add annotation reader cache #818

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

Closed
wants to merge 11 commits into from
Closed

Add annotation reader cache #818

wants to merge 11 commits into from

Conversation

Vincz
Copy link
Collaborator

@Vincz Vincz commented Feb 1, 2021

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Documented? no
Fixed tickets #...
License MIT

Add an cache for annotations reader to improve performances.

@ferrastas
Copy link
Contributor

What about allowing the user to use any implementation of Doctrine\Common\Cache\Cache? Even having apcu available on the system, the user might want to rely on the Doctrine\Common\Cache\PhpFileCache and take advantage of OPcache.

@Vincz Vincz requested review from murtukov and mcg-web April 23, 2021 07:29
@Vincz
Copy link
Collaborator Author

Vincz commented Apr 23, 2021

What about allowing the user to use any implementation of Doctrine\Common\Cache\Cache? Even having apcu available on the system, the user might want to rely on the Doctrine\Common\Cache\PhpFileCache and take advantage of OPcache.

Hi Ferrastas, we are currently working on the next version and in the next version, the reader will be a service, so it will be easier to override it.

* Import functions
* Make property non-nullable
* Edit exception message
@murtukov
Copy link
Contributor

@Vincz I pushed some changes into your branch. Don't know what is the problem with Ocular. Didn't you already have this problem with Ocular?

@Vincz
Copy link
Collaborator Author

Vincz commented Apr 23, 2021

@Vincz I pushed some changes into your branch. Don't know what is the problem with Ocular. Didn't you already have this problem with Ocular?

Hi @murtukov :-)
I had a problem with it before, yes. I think, it was because it was not running from the main repository if I remember correctly. But curiously, others PR seem to work just fine.
Seems to be a GIT problem, but my skills with it are pretty limited...

@murtukov
Copy link
Contributor

@Vincz do you remember which PR was it, where we fixed the issue with Ocular?

@Vincz
Copy link
Collaborator Author

Vincz commented Nov 11, 2021

Handle by #867

@Vincz Vincz closed this Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants