Add static cache to DatabaseConnection ::escapeDatabase(), ::escapeTable(), ::escapeField() and ::escapeAlias()

Looking over some cachegrind's of mine and I see right around 10ms for 3k calls for each of the escape methods (~30ms total). Is this similar to what you're experiencing?

Shouldn't this be against 8.x first?

This sounds like a very good idea to me.

+++ b/includes/database/database.inc
@@ -919,11 +919,15 @@ abstract class DatabaseConnection extends PDO {
+    static $tables = array();

@@ -933,11 +937,15 @@ abstract class DatabaseConnection extends PDO {
+    static $fields = array();

@@ -948,11 +956,15 @@ abstract class DatabaseConnection extends PDO {
+    static $fields = array();

This doesn't have to be a static one. It would be totally enough to have a property on the object, caching the result.

I think that input from Crell would be wise.

The patch look good, but I have two remarks:

1. +++ b/core/lib/Drupal/Core/Database/Connection.php
   @@ -925,7 +925,11 @@ public function schema() {
   +    static $databases = [];
   
   @@ -942,7 +946,11 @@ public function escapeDatabase($database) {
   +    static $tables = [];
   
   @@ -959,7 +967,11 @@ public function escapeTable($table) {
   +    static $fields = [];
   
   @@ -977,7 +989,11 @@ public function escapeField($field) {
   +    static $fields = [];
   

   @dawehner stated in comment #13 that the variables do not have to be static ones. Can you explain why you keep them static.

2. +++ b/core/lib/Drupal/Core/Database/Connection.php
   @@ -977,7 +989,11 @@ public function escapeField($field) {
      public function escapeAlias($field) {
   -    return preg_replace('/[^A-Za-z0-9_]+/', '', $field);
   +    static $fields = [];
   +    if (!isset($fields[$field])) {
   +      $fields[$field] = preg_replace('/[^A-Za-z0-9_]+/', '', $field);
   +    }
   +    return $fields[$field];
   

   Can we rename the variable fields to aliases, because we already have a variable fields and its is holding an array of aliases.

@joelpittet: If you change the variable name $fields to $aliases in the method escapeAlias(), the patch will for me be good enough for RTBC. Thank you for your explanation in the previous comment.

I all looks good to me. But I am not 100% sure this is the right solution and I would like to ask one of our framework managers to be the committer for this issue.

Thanks @daffie.

Adding the rc deadline tag because I think this improvement could go into 8.2.x during its beta but not RC, if we do choose to commit it with its current implementation. (After the beta it can go straight into 8.3.x of course.)

Since these are normal methods, we could use a regular protected property on the class - no need for an actual static here. I see above this was avoided because it would be three properties, that doesn't seem like a reason not to do it?

It would also be good to see xhprof profiling with a standard install - I imagine we'll see a small improvement because we hit the same cache tables often.

@joelpittet one per regex difference seems best - also if a database/table/column happened to be named the same (albeit unlikely) we could re-use the sanitization.

I see the reasoning for the static, but the disadvantage outweighs the advantage for me.

Thanks! That looks a lot better. Moving back to RTBC.

A nice little performance improvement. I was going to assign it to @catch for signoff but he RTBCed the issue, so I've committed 4295f59 and pushed to 8.3.x. Thanks!

And now we can add this to 7.x as well :)

Marking for commit.

Very nice change.

Re-reviewed with Fabianx, and committed and pushed to 7.x, thanks!