DoctrineEncryptBundle icon indicating copy to clipboard operation
DoctrineEncryptBundle copied to clipboard

Upgrading to 5.1 from 5.0.3 huge performance decrease when using annotations

Open virtualize opened this issue 2 years ago • 14 comments

In 5.1 support for PHP attributes was added and the default doctrine annotations reader was replaced. This change slows down our systems by almost 100%, i.e. rendering a page takes almost twice as long.

If we replace in services.yaml the new annotation reader arguments ("@ambta_doctrine_annotation_reader") with the default one from doctrine "@annotation_reader" the system is fast again.

@flavou45 do you have any idea why this is? Could this be annotation caching issue?

virtualize avatar May 31 '22 21:05 virtualize

I had problems with CI on CircleCI. (https://github.com/absolute-quantum/DoctrineEncryptBundle/issues/67) Tests did failed with 5.1, kill signal from the container.

I specified @annotation_reader on the services as you specified. It did pass so the problem was related. Thank you, you saved me a big headache.

encreinformatique avatar Jun 01 '22 11:06 encreinformatique

I noticed the same problem here. When I upgrade from 5.0.3 to 5.1 then some pages (specifically the ones with a lot of doctrine actions) become a lot slower. For me it is even 3 to 4 times slower!

joostcapsearch avatar Jul 06 '22 13:07 joostcapsearch

+1 hudge bug

charliefancelli avatar Aug 19 '22 09:08 charliefancelli

I've had the same issue with massive performance loss after upgrading, with your workaround it works again, thanks.

klausi85 avatar Nov 07 '22 08:11 klausi85

This issue caused a 300%+ increase to our loading times.

Following OP's advice, the issue was resolved by adding the following to the project's services.yaml:


    # This is a performance fix for michaeldegroot/doctrine-encrypt-bundle v5.1
    # See https://github.com/absolute-quantum/DoctrineEncryptBundle/issues/70 for context
    ambta_doctrine_annotation_reader:
        alias: annotation_reader

Note that this will only work if you use annotations - if you use PHP attributes, this "fix" will not apply to you.

EmilePerron avatar Dec 13 '22 13:12 EmilePerron

Due to various Doctrine ORM deprecations including:

  • getEntity() is deprecated
  • getEntityManager() is deprecated
  • Using Doctrine subscribers as services is deprecated, declare listeners instead

Discussions in this pull https://github.com/absolute-quantum/DoctrineEncryptBundle/pull/65 request.

These 2 comments specifically:

  • https://github.com/absolute-quantum/DoctrineEncryptBundle/pull/65#issuecomment-1476301156
  • https://github.com/absolute-quantum/DoctrineEncryptBundle/pull/65#issuecomment-1178882618

I need to ensure that my projects will be able to upgrade to new versions of packages and not fall behind, so I created the DoctrineEncryptBundle organisation forked the pull request's repository from integr8rs:master-fixes and created the following package https://packagist.org/packages/doctrineencryptbundle/doctrine-encrypt-bundle

I know about some issues in the version I released (5.3.0) Example: issue (https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle/issues/1) that have already been closed. It will be in a release in the next couple days as I do plan on looking fixing a few issues that I know about.

I do need to still add another Owner and probably more members to the organisation. Volunteers would be appreciated.

I do think that I might have fixed the annotation reader issue, I unfortunately do not have a project that uses annotations anymore. Would some of you be able to test it for me?

r3hp1c avatar Jul 17 '23 12:07 r3hp1c

@r3hp1c I dropped annotations for attributes a couple of months ago but I might be able to find it. (lots of commits and PR in between) As I recall, annotations were faster in this case. Did you fix the annotation reader for attributes? (attribute reader)

Just to be sure, the right repo for PHP ^8.1 is now https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle or the current one ? I do not see a mention on absolute-quantum/DoctrineEncryptBundle hence the question.

encreinformatique avatar Jul 24 '23 22:07 encreinformatique

@encreinformatique Yes, I created the DoctrineEncryptBundle as a GitHub organization so that the bundle will hopefully not become stale again (still looking for co-owners BTW).

For the annotations vs the attributes, I had to try and keep backwards compatibility so I made it that the AttributeReader could be used directly still keeping the AttributeAnnotationReader as the default: https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle/blob/master/src/Resources/doc/configuration.md

I also made the annotations cache in this commit: https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle/commit/f83dee413590e8cf291bb54ffe1e14fd261e5059 The slowness was caused due to the annotations being read every single time for every entity. Caching the annotations I believe should make it faster.

I still don't suggest using the AttributeAnnotationReader really as I have noticed that if it does not find an attribute for a property it still tries to read annotations. You should be able to get a little more speed by only using the attribute reader.

I have fixed some other fixes in that repository as well, including some that still need to make it into a release like this one: https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle/commit/1e7f8c58299ab305615c0d88a16d997ab61611ef I linked all old issues that I did not have time to look at initially in this issue: https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle/issues/7

r3hp1c avatar Jul 25 '23 06:07 r3hp1c

@r3hp1c I am interested in co-maintaining the project. will be more than happy to contribute.

cs-akash-jarad avatar Jul 25 '23 08:07 cs-akash-jarad

@cs-akash-jarad Thank you. Sent you an invite to be an owner in the organization.

r3hp1c avatar Jul 25 '23 08:07 r3hp1c

I also made the annotations cache in this commit: DoctrineEncryptBundle@f83dee4 The slowness was caused due to the annotations being read every single time for every entity. Caching the annotations I believe should make it faster.

This is a nice one. That would explain part of my lack of performance on an entity with 200ish records. What I did then was store some of the decrypted values in the cache system of Symfony. (in memory ram in fact) Not the most secure thing, but those fields did not require extreme security precautions either.

I'll try it this week and see if I see improvements. I am interested in the bundle as well. I reduced the exposure due to performance so if it has been fixed, usage would be greater in my case.

encreinformatique avatar Jul 25 '23 11:07 encreinformatique

@r3hp1c I made some tests with both attributes reader and annotation reader. I can't say it is very conclusive. It was in docker on my computer, prod environment and cache flushed between the change of reader. Entity had both annotation and attribute Encrypted.

4 properties in one entity and 85 records. Navigating between is fast but when it touch that entity, it remains slow. Maybe I missed something.

-- edit: It does not read the attributes in the app if I remove the annotation so I have something wrong in the config. The service has been added so I need to narrow it down to the cause of error. It seems the problem is on my side and note the bundle. Speed is slow though.

  ambta_doctrine_encrypt.orm_subscriber:
    class: Ambta\DoctrineEncryptBundle\Subscribers\DoctrineEncryptSubscriber
    arguments: [ "@ambta_doctrine_attribute_reader", "@ambta_doctrine_encrypt.encryptor" ]
    tags:
      - { name: doctrine.event_subscriber }

encreinformatique avatar Jul 26 '23 13:07 encreinformatique

@encreinformatique which configuration file and under which tag did you put the configuration and what timings are you looking at when you state it remains slow?

In version 5.3.1 of https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle I have the configuration in services.yaml like below and the attributes read fine for me in both the browser and the commands as I do not use annotations either.

services:
    # Skip trying to read annotations. Only read attributes
    ambta_doctrine_encrypt.orm_subscriber:
        class: Ambta\DoctrineEncryptBundle\Subscribers\DoctrineEncryptSubscriber
        arguments: ["@ambta_doctrine_attribute_reader", "@ambta_doctrine_encrypt.encryptor"]
        tags:
            -  { name: doctrine.event_subscriber }

    ambta_doctrine_encrypt.command.decrypt.database:
        class: Ambta\DoctrineEncryptBundle\Command\DoctrineDecryptDatabaseCommand
        tags: ['console.command']
        arguments:
            - "@doctrine.orm.entity_manager"
            - "@ambta_doctrine_attribute_reader"
            - "@ambta_doctrine_encrypt.subscriber"

    ambta_doctrine_encrypt.command.encrypt.database:
        class: Ambta\DoctrineEncryptBundle\Command\DoctrineEncryptDatabaseCommand
        tags: ['console.command']
        arguments:
            - "@doctrine.orm.entity_manager"
            - "@ambta_doctrine_attribute_reader"
            - "@ambta_doctrine_encrypt.subscriber"

    ambta_doctrine_encrypt.command.encrypt.status:
        class: Ambta\DoctrineEncryptBundle\Command\DoctrineEncryptStatusCommand
        tags: ['console.command']
        arguments:
            - "@doctrine.orm.entity_manager"
            - "@ambta_doctrine_attribute_reader"
            - "@ambta_doctrine_encrypt.subscriber"

In my project using the bundle I have a list of staff members displaying with my development environment having the list at more that 800 staff members. Believe that is more that 800 calls for the postLoad event and the page loads under 2 seconds.

Some additional details:

$ php bin/console doctrine:encrypt:status
..
199 entities found which are containing 40 encrypted properties.

Think the decrypted value is incorrect below:

$ time php bin/console doctrine:decrypt:database
..
Decryption finished values found: 23774, decrypted: 0.
All values are now decrypted.

real	0m9.928s
user	0m5.703s
sys	0m0.549s

Think the encrypted value is incorrect below as well:

$ time php bin/console doctrine:encrypt:database
..
Encryption finished. Values encrypted: 20 values.
All values are now encrypted.

real	0m19.090s
user	0m13.051s
sys	0m0.744s

This seems to be working fine for me and hopes this helps to figure out your slowness.

Logged issue for the incorrect counts here: https://github.com/DoctrineEncryptBundle/DoctrineEncryptBundle/issues/25

r3hp1c avatar Jul 27 '23 07:07 r3hp1c

@r3hp1c I have the following in the config and, for context, my use case is the following :

  • list of contacts with email, skype, first name, and last name, all encrypted.
  • I need to have them encrypted in the database (EU GDPR stuff etc...) at cold state
  • the user of the app must be able to see the contacts or the app is unusable

I display the list of contacts, (150 at most) paginated in a readable format (ie. not encrypted) only if the user has the right to access the page. I have a cache layer storing the decrypted info in RAM for speed. Not the best solution (if someone could access the server) but still better than having everything not encrypted. Filtering is another beast but can be dealt into the browser or on the cached data. That EntityCacheSubscriber layer is subscribed to \Doctrine\ORM\Events::preUpdate and \Doctrine\ORM\Events::postLoad (to update a contact if needed and when loading them) It is dealing only with the entities implementing a CachableInterface. (from the app, not the bundle) Sensitive data would not be "cachable".

Maybe I got it wrong or am implementing an incorrect solution, I cannot exclude that possibility. :)

services:
  ambta_doctrine_encrypt.orm_subscriber:
    class: Ambta\DoctrineEncryptBundle\Subscribers\DoctrineEncryptSubscriber
    arguments: [ "@ambta_doctrine_attribute_reader", "@ambta_doctrine_encrypt.encryptor" ]
    tags:
      - { name: doctrine.event_subscriber }

commands.yaml

  ambta_doctrine_encrypt.command.decrypt.database:
    class: Ambta\DoctrineEncryptBundle\Command\DoctrineDecryptDatabaseCommand
    tags: [ 'console.command' ]
    arguments:
      - "@doctrine.orm.entity_manager"
      - "@ambta_doctrine_attribute_reader"
      - "@ambta_doctrine_encrypt.subscriber"
  ambta_doctrine_encrypt.command.encrypt.database:
    class: Ambta\DoctrineEncryptBundle\Command\DoctrineEncryptDatabaseCommand
    tags: [ 'console.command' ]
    arguments:
      - "@doctrine.orm.entity_manager"
      - "@ambta_doctrine_attribute_reader"
      - "@ambta_doctrine_encrypt.subscriber"
  ambta_doctrine_encrypt.command.encrypt.status:
    class: Ambta\DoctrineEncryptBundle\Command\DoctrineEncryptStatusCommand
    tags: [ 'console.command' ]
    arguments:
      - "@doctrine.orm.entity_manager"
      - "@ambta_doctrine_attribute_reader"
      - "@ambta_doctrine_encrypt.subscriber"

All this means the commands are barely used in the app. To measure time, I look at the profiler or the time of the console in production environment, for multiple calls without the cache layer (EntityCacheSubscriber) of the app. The app is on local in Docker (OrbStack in fact) as I cannot test it on the production server.

Commands returns those time.

# time bin/console doctrine:encrypt:database --env=prod 
Encryption finished. Values encrypted: 10 values.
All values are now encrypted.
59.74user 0.13system 1:02.45elapsed 95%CPU (0avgtext+0avgdata 68580maxresident)k
0inputs+0outputs (0major+13745minor)pagefaults 0swaps
# time bin/console doctrine:decrypt:database --env=prod 
Decryption finished values found: 510, decrypted: 0.
All values are now decrypted.
30.80user 0.12system 0:32.83elapsed 94%CPU (0avgtext+0avgdata 68452maxresident)k
0inputs+0outputs (0major+13703minor)pagefaults 0swaps

encreinformatique avatar Jul 28 '23 13:07 encreinformatique