[DoctrineBundle] Enable collect-backtrace for dev env

[ottaviano]

Q	A
License	MIT
Doc issue/PR

Enable collect backtrace of each query for Dev env

[Pierstoval]

This has a really big impact on performances and memory usage, I wouldn't recommend to add it in dev at all.

A compromise would be to comment it and add an explanation, but I'm not sure of the added value.

[nicolas-grekas]

This has been added to the main doctrine.yaml, commented.

[stof]

The performance impact is precisely the reason why it is disabled by default in DoctrineBundle. So I agree about keeping it commented.

[ottaviano]

yes, no problem for keeping it commented, the goal of this PR is to show this option. And I think the right place for it is in dev/doctrine.yaml. This PR adds it for 3 versions: 1.12, 1.6 and 2.0 (not only for 1.12)

[nicolas-grekas]

then please send another PR that moves the comment (but doesn't add it to prod/test config files)

[ottaviano]

Why another PR? This PR already adds it only for dev env and for 3 versions. I can update it for moving the comment, no?

[ottaviano]

updated, status: need review

[lyrixx]

1. This Pr does not work

Unrecognized option "profiling_collect_backtrace" under "doctrine.dbal". Available options are "connections", "default_connection", "types".

It should be under the connections keys

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                profiling_collect_backtrace: true

2. About the performance impact, I tested it why a real application, in easy admin. I did not notice any big performance impact as state by @Pierstoval and @stof. Could you share your numbers?

Mine:

Without the collector enabled

With the collector enabled

[Pierstoval]

@lyrixx When I tested this, the cost was at least of twice the time, but maybe some internal improvements have been in Doctrine in the meantime, I'll retry it with two of my projects to see if there's anything new in perfs 👍

[lyrixx]

If you look at the code, the diff is about debug_backtrace.

So here a simple bench script that simulate 100K SQL requests, at a depth of 50 nested calls (to bench the CPU)

function empty_function_call()
{
}

function a(int $i = 50) {
    if ($i === 0) {
        $s = microtime(true);
        for ($i=0; $i < 100000; $i++) {
            empty_function_call(DEBUG_BACKTRACE_IGNORE_ARGS);
        }
        printf("void %.2fs\n", microtime(true) - $s);
        printf("void %.2fMb\n", memory_get_peak_usage() / 1024 / 1024);


        $s = microtime(true);
        for ($i=0; $i < 100000; $i++) {
            debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
        }
        printf("backtrace %.2fs\n", microtime(true) - $s);
        printf("backtrace %.2fMb\n", memory_get_peak_usage() / 1024 / 1024);

        return;
    }
    a(--$i);
}

a();

Results:

void 0.00s
void 0.61Mb
backtrace 0.20s
backtrace 0.61Mb

So in term of CPU it's almost the same. I recall we talk of 100K req here! And in term of memory it's the same. BUT, here I don't store the trace, so this should be taken in account. A trace weight highly varies (depth, path length, etc) but it's about 25Kb

If I ran again the previous script with 1K SQL request (which is huge!!!) and I store the trace, I got the following results

void 0.00s
void 0.61Mb
backtrace 0.01s
backtrace 21.23Mb

So, IMHO, the impact is not so much.

TL;DR: With 1K SQL requests: CPU +10ms ; Memory +20Mb(the values depends on each setup)

[ottaviano]

@lyrixx thank you for doing this bench! @stof, @nicolas-grekas, @Pierstoval should we reopen the discussion about leaving this param commented?

For your first point, I have two projects with exactly: doctrine: dbal: profiling_collect_backtrace: true in dev/doctrine.yaml and it works fine (doctrine/doctrine-bundle: 1.11.1 and 2.2.0) 🤔

[Pierstoval]

Wow, I don't know what changed, but I checked on a project where an operation has to run more than 900 selects and runs 972 inserts (yep, a bit complex project, rare case but can happen every once in a while), the whole operation takes approx 2 seconds of query time, and there's almost no difference.

And with such amount of queries, I expected the web profiler's Doctrine page to take longer to load, but it's kinda the same time for both.

Great news, LGTM then 👍

Using doctrine/dbal 2.11.1 and doctrine/doctrine-bundle 2.1.2

[nicolas-grekas]

(please rebase so that the new checks can run)

[github-actions[bot]]

Diff between recipe versions

Thanks for the PR 😍

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes. I'm going keep this comment up to date with any updates of the attached patch.

doctrine/doctrine-bundle

1.6 vs 1.12

diff --git a/doctrine/doctrine-bundle/1.12/config/packages/dev/doctrine.yaml b/doctrine/doctrine-bundle/1.12/config/packages/dev/doctrine.yaml
new file mode 100644
index 0000000..23606fc
--- /dev/null
+++ b/doctrine/doctrine-bundle/1.12/config/packages/dev/doctrine.yaml
@@ -0,0 +1,4 @@
+doctrine:
+    dbal:
+        # backtrace queries in profiler (increases memory usage per request)
+        #profiling_collect_backtrace: '%kernel.debug%'
diff --git a/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
index bea3648..13c8428 100644
--- a/doctrine/doctrine-bundle/1.6/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
@@ -10,9 +10,10 @@ doctrine:
         charset: utf8mb4
         default_table_options:
             collate: utf8mb4_unicode_ci
+
     orm:
         auto_generate_proxy_classes: true
-        naming_strategy: doctrine.orm.naming_strategy.underscore
+        naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
         auto_mapping: true
         mappings:
             App:
diff --git a/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml b/doctrine/doctrine-bundle/1.12/config/packages/prod/doctrine.yaml
index 0a7c53b..084f59a 100644
--- a/doctrine/doctrine-bundle/1.6/config/packages/prod/doctrine.yaml
+++ b/doctrine/doctrine-bundle/1.12/config/packages/prod/doctrine.yaml
@@ -2,26 +2,14 @@ doctrine:
     orm:
         auto_generate_proxy_classes: false
         metadata_cache_driver:
-            type: service
-            id: doctrine.system_cache_provider
+            type: pool
+            pool: doctrine.system_cache_pool
         query_cache_driver:
-            type: service
-            id: doctrine.system_cache_provider
+            type: pool
+            pool: doctrine.system_cache_pool
         result_cache_driver:
-            type: service
-            id: doctrine.result_cache_provider
-
-services:
-    doctrine.result_cache_provider:
-        class: Symfony\Component\Cache\DoctrineProvider
-        public: false
-        arguments:
-            - '@doctrine.result_cache_pool'
-    doctrine.system_cache_provider:
-        class: Symfony\Component\Cache\DoctrineProvider
-        public: false
-        arguments:
-            - '@doctrine.system_cache_pool'
+            type: pool
+            pool: doctrine.result_cache_pool
 
 framework:
     cache:

1.12 vs 2.0

diff --git a/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml b/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
index 13c8428..c319176 100644
--- a/doctrine/doctrine-bundle/1.12/config/packages/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml
@@ -5,12 +5,6 @@ doctrine:
         # IMPORTANT: You MUST configure your server version,
         # either here or in the DATABASE_URL env var (see .env file)
         #server_version: '13'
-
-        # only needed for MySQL
-        charset: utf8mb4
-        default_table_options:
-            collate: utf8mb4_unicode_ci
-
     orm:
         auto_generate_proxy_classes: true
         naming_strategy: doctrine.orm.naming_strategy.underscore_number_aware
diff --git a/doctrine/doctrine-bundle/1.12/post-install.txt b/doctrine/doctrine-bundle/2.0/post-install.txt
index 8dd3c86..54ca138 100644
--- a/doctrine/doctrine-bundle/1.12/post-install.txt
+++ b/doctrine/doctrine-bundle/2.0/post-install.txt
@@ -4,5 +4,5 @@
 
   * Modify your DATABASE_URL config in .env
 
-  * Configure the driver (mysql) and
-    server_version (5.7) in config/packages/doctrine.yaml
+  * Configure the driver (postgresql) and
+    server_version (13) in config/packages/doctrine.yaml

2.0 vs 2.3

diff --git a/doctrine/doctrine-bundle/2.0/config/packages/dev/doctrine.yaml b/doctrine/doctrine-bundle/2.0/config/packages/dev/doctrine.yaml
deleted file mode 100644
index 23606fc..0000000
--- a/doctrine/doctrine-bundle/2.0/config/packages/dev/doctrine.yaml
+++ /dev/null
@@ -1,4 +0,0 @@
-doctrine:
-    dbal:
-        # backtrace queries in profiler (increases memory usage per request)
-        #profiling_collect_backtrace: '%kernel.debug%'
diff --git a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
new file mode 100644
index 0000000..2ace640
--- /dev/null
+++ b/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
@@ -0,0 +1,4 @@
+doctrine:
+    dbal:
+        # "TEST_TOKEN" is typically set by ParaTest
+        dbname: 'main_test%env(default::TEST_TOKEN)%'
diff --git a/doctrine/doctrine-bundle/2.0/manifest.json b/doctrine/doctrine-bundle/2.3/manifest.json
index 062aed0..995a953 100644
--- a/doctrine/doctrine-bundle/2.0/manifest.json
+++ b/doctrine/doctrine-bundle/2.3/manifest.json
@@ -15,9 +15,9 @@
         "DATABASE_URL": "postgresql://db_user:[email protected]:5432/db_name?serverVersion=13&charset=utf8"
     },
     "dockerfile": [
-        "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev && \\",
-        "\tdocker-php-ext-install -j$(nproc) pdo_pgsql && \\",
-        "\tapk add --no-cache --virtual .pgsql-rundeps so:libpq.so.5 && \\",
+        "RUN apk add --no-cache --virtual .pgsql-deps postgresql-dev; \\",
+        "\tdocker-php-ext-install -j$(nproc) pdo_pgsql; \\",
+        "\tapk add --no-cache --virtual .pgsql-rundeps so:libpq.so.5; \\",
         "\tapk del .pgsql-deps"
     ],
     "docker-compose": {

2.3 vs 2.4

diff --git a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml b/doctrine/doctrine-bundle/2.4/config/packages/test/doctrine.yaml
index 2ace640..34c2ebc 100644
--- a/doctrine/doctrine-bundle/2.3/config/packages/test/doctrine.yaml
+++ b/doctrine/doctrine-bundle/2.4/config/packages/test/doctrine.yaml
@@ -1,4 +1,4 @@
 doctrine:
     dbal:
         # "TEST_TOKEN" is typically set by ParaTest
-        dbname: 'main_test%env(default::TEST_TOKEN)%'
+        dbname_suffix: '_test%env(default::TEST_TOKEN)%'

[nicolas-grekas]

Sorry, can you please rebase once again and target only the recipe for the latest version of the bundle (which uses when@ now)