@chx seems to have added this, but why don't we use it?

CommentFileSizeAuthor
#8 1901390_8.patch940 byteschx
drupal8.annotation-only.0.patch1.8 KBsun

Comments

eclipsegc’s picture

eclipsegc commented 
+++ b/core/vendor/doctrine/common/lib/Doctrine/Common/Reflection/StaticReflectionParser.phpundefined
@@ -119,7 +119,7 @@ protected function parse()
         $this->parsed = true;
         $contents = file_get_contents($fileName);
         if ($this->classAnnotationOptimize) {
-            if (preg_match("/(\A.*)^\s+(abstract|final)?\s+class\s+$className\s+{/sm", $contents, $matches)) {
+            if (preg_match('/(\A.*)^\s+(abstract|final)?\s+class\s+' . preg_quote($this->className, '/') . '\s+{/sm', $contents, $matches)) {
                 $contents = $matches[1];
             }
         }

This has to be done upstream in Doctrine. Do we need this for the change in AnnotatedClassDiscovery?

chx’s picture

chx commented

wat? I ack the bug that $className needs to be $this->className (needs to be fixed upstream) but since when does a php identifier need a preg_quote??

chx’s picture

chx commented

This has been fixed upstream about one day before sun filed this issue.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

The primary and essential change in question is only this anyway:

+++ b/core/lib/Drupal/Component/Plugin/Discovery/AnnotatedClassDiscovery.php
@@ -86,7 +86,7 @@ public function getDefinitions() {
-              $parser = new StaticReflectionParser($class, $finder);
+              $parser = new StaticReflectionParser($class, $finder, TRUE);

Was there any particular reason for not using the option?

chx’s picture

chx commented

None but we need the commit i linked before we can use it cos *blush* it's broken in the current version core has.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

drupal8.annotation-only.0.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal8.annotation-only.0.patch, failed testing.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new 1901390_8.patch940 bytes
chx’s picture

chx commented
Title: Enable StaticReflectionParser performance optimization to speed up Annotation parsing? » Enable StaticReflectionParser performance optimization to speed up Annotation parsing
Status: Needs review » Reviewed & tested by the community

I have only rerolled #0 really w/o the Doctrine change -- and I know this works :)

neclimdul’s picture

neclimdul
He/Him
commented

Seems reasonable. To be clear because I was confused, we already pulled in the other changes so we just need to turn on the optimization. I can't see a problem with that its pretty straightforward and if you're following our standards there won't be a problem.

RTBC+1

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 47ff5ff and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.