Skip to content

Commit 0d655ba

Browse files
fabpotfabpot
committed
minor symfony#25568 [Debug] Skip DebugClassLoader checks for already parsed files (nicolas-grekas)
This PR was merged into the 3.4 branch. Discussion ---------- [Debug] Skip DebugClassLoader checks for already parsed files | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Perf. fix for the dev mode only: I propose to run the checks done by DebugClassLoader only once, when the file is changed/parsed. This should improve DX much, by making the dev env faster. Here is a bench on the hello world of the standard edition (class inlining by the DIC disabled, but still): ![image](https://user-images.githubusercontent.com/243674/34224242-8852ee9a-e5c2-11e7-94ae-6edc6ab41de5.png) https://blackfire.io/profiles/compare/31ff0792-9992-4658-9707-cac87a320d1f/graph Commits ------- f0c1223 [Debug] Skip DebugClassLoader checks for already parsed files
2 parents f0b1dc2 + f0c1223 commit 0d655ba

File tree

2 files changed

+70
-19
lines changed

2 files changed

+70
-19
lines changed

‎src/Symfony/Component/Debug/DebugClassLoader.php

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,13 @@ class DebugClassLoader
2828
private $isFinder;
2929
private $loaded = array();
3030
private static $caseCheck;
31+
private static $checkedClasses = array();
3132
private static $final = array();
3233
private static $finalMethods = array();
3334
private static $deprecated = array();
3435
private static $internal = array();
3536
private static $internalMethods = array();
36-
private static $php7Reserved = array('int', 'float', 'bool', 'string', 'true', 'false', 'null');
37+
private static $php7Reserved = array('int' => 1, 'float' => 1, 'bool' => 1, 'string' => 1, 'true' => 1, 'false' => 1, 'null' => 1);
3738
private static $darwinCache = array('/' => array('/', array()));
3839

3940
public function __construct(callable $classLoader)
@@ -139,8 +140,14 @@ public function loadClass($class)
139140
try {
140141
if ($this->isFinder && !isset($this->loaded[$class])) {
141142
$this->loaded[$class] = true;
142-
if ($file = $this->classLoader[0]->findFile($class)) {
143+
if ($file = $this->classLoader[0]->findFile($class) ?: false) {
144+
$wasCached = \function_exists('opcache_is_script_cached') && opcache_is_script_cached($file);
145+
143146
require $file;
147+
148+
if ($wasCached) {
149+
return;
150+
}
144151
}
145152
} else {
146153
call_user_func($this->classLoader, $class);
@@ -150,55 +157,75 @@ public function loadClass($class)
150157
error_reporting($e);
151158
}
152159

153-
$exists = class_exists($class, false) || interface_exists($class, false) || trait_exists($class, false);
160+
$this->checkClass($class, $file);
161+
}
154162

155-
if ($class && '\\' === $class[0]) {
163+
private function checkClass($class, $file = null)
164+
{
165+
$exists = null === $file || \class_exists($class, false) || \interface_exists($class, false) || \trait_exists($class, false);
166+
167+
if (null !== $file && $class && '\\' === $class[0]) {
156168
$class = substr($class, 1);
157169
}
158170

159171
if ($exists) {
172+
if (isset(self::$checkedClasses[$class])) {
173+
return;
174+
}
175+
self::$checkedClasses[$class] = true;
176+
160177
$refl = new \ReflectionClass($class);
178+
if (null === $file && $refl->isInternal()) {
179+
return;
180+
}
161181
$name = $refl->getName();
162182

163-
if ($name !== $class && 0 === strcasecmp($name, $class)) {
183+
if ($name !== $class && 0 === \strcasecmp($name, $class)) {
164184
throw new \RuntimeException(sprintf('Case mismatch between loaded and declared class names: "%s" vs "%s".', $class, $name));
165185
}
166186

167187
// Don't trigger deprecations for classes in the same vendor
168-
if (2 > $len = 1 + (strpos($name, '\\') ?: strpos($name, '_'))) {
188+
if (2 > $len = 1 + (\strpos($name, '\\') ?: \strpos($name, '_'))) {
169189
$len = 0;
170190
$ns = '';
171191
} else {
172-
$ns = substr($name, 0, $len);
192+
$ns = \substr($name, 0, $len);
173193
}
174194

175195
// Detect annotations on the class
176196
if (false !== $doc = $refl->getDocComment()) {
177197
foreach (array('final', 'deprecated', 'internal') as $annotation) {
178-
if (false !== strpos($doc, '@'.$annotation) && preg_match('#\n \* @'.$annotation.'(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) {
198+
if (false !== \strpos($doc, $annotation) && preg_match('#\n \* @'.$annotation.'(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) {
179199
self::${$annotation}[$name] = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
180200
}
181201
}
182202
}
183203

184-
$parentAndTraits = class_uses($name, false);
185-
if ($parent = get_parent_class($class)) {
204+
$parentAndTraits = \class_uses($name, false);
205+
if ($parent = \get_parent_class($class)) {
186206
$parentAndTraits[] = $parent;
187207

208+
if (!isset(self::$checkedClasses[$parent])) {
209+
$this->checkClass($parent);
210+
}
211+
188212
if (isset(self::$final[$parent])) {
189213
@trigger_error(sprintf('The "%s" class is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $parent, self::$final[$parent], $name), E_USER_DEPRECATED);
190214
}
191215
}
192216

193217
// Detect if the parent is annotated
194218
foreach ($parentAndTraits + $this->getOwnInterfaces($name, $parent) as $use) {
195-
if (isset(self::$deprecated[$use]) && strncmp($ns, $use, $len)) {
219+
if (!isset(self::$checkedClasses[$use])) {
220+
$this->checkClass($use);
221+
}
222+
if (isset(self::$deprecated[$use]) && \strncmp($ns, $use, $len)) {
196223
$type = class_exists($name, false) ? 'class' : (interface_exists($name, false) ? 'interface' : 'trait');
197224
$verb = class_exists($use, false) || interface_exists($name, false) ? 'extends' : (interface_exists($use, false) ? 'implements' : 'uses');
198225

199226
@trigger_error(sprintf('The "%s" %s %s "%s" that is deprecated%s.', $name, $type, $verb, $use, self::$deprecated[$use]), E_USER_DEPRECATED);
200227
}
201-
if (isset(self::$internal[$use]) && strncmp($ns, $use, $len)) {
228+
if (isset(self::$internal[$use]) && \strncmp($ns, $use, $len)) {
202229
@trigger_error(sprintf('The "%s" %s is considered internal%s. It may change without further notice. You should not use it from "%s".', $use, class_exists($use, false) ? 'class' : (interface_exists($use, false) ? 'interface' : 'trait'), self::$internal[$use], $name), E_USER_DEPRECATED);
203230
}
204231
}
@@ -209,12 +236,12 @@ public function loadClass($class)
209236
foreach ($parentAndTraits as $use) {
210237
foreach (array('finalMethods', 'internalMethods') as $property) {
211238
if (isset(self::${$property}[$use])) {
212-
self::${$property}[$name] = array_merge(self::${$property}[$name], self::${$property}[$use]);
239+
self::${$property}[$name] = self::${$property}[$name] ? self::${$property}[$use] + self::${$property}[$name] : self::${$property}[$use];
213240
}
214241
}
215242
}
216243

217-
$isClass = class_exists($name, false);
244+
$isClass = \class_exists($name, false);
218245
foreach ($refl->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $method) {
219246
if ($method->class !== $name) {
220247
continue;
@@ -233,7 +260,7 @@ public function loadClass($class)
233260
foreach ($parentAndTraits as $use) {
234261
if (isset(self::$internalMethods[$use][$method->name])) {
235262
list($declaringClass, $message) = self::$internalMethods[$use][$method->name];
236-
if (strncmp($ns, $declaringClass, $len)) {
263+
if (\strncmp($ns, $declaringClass, $len)) {
237264
@trigger_error(sprintf('The "%s::%s()" method is considered internal%s. It may change without further notice. You should not extend it from "%s".', $declaringClass, $method->name, $message, $name), E_USER_DEPRECATED);
238265
}
239266
}
@@ -245,14 +272,14 @@ public function loadClass($class)
245272
}
246273

247274
foreach (array('final', 'internal') as $annotation) {
248-
if (false !== strpos($doc, '@'.$annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
275+
if (false !== \strpos($doc, $annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
249276
$message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
250277
self::${$annotation.'Methods'}[$name][$method->name] = array($name, $message);
251278
}
252279
}
253280
}
254281

255-
if (in_array(strtolower($refl->getShortName()), self::$php7Reserved)) {
282+
if (isset(self::$php7Reserved[\strtolower($refl->getShortName())])) {
256283
@trigger_error(sprintf('The "%s" class uses the reserved name "%s", it will break on PHP 7 and higher', $name, $refl->getShortName()), E_USER_DEPRECATED);
257284
}
258285
}
@@ -350,8 +377,6 @@ public function loadClass($class)
350377
throw new \RuntimeException(sprintf('Case mismatch between class and real file names: "%s" vs "%s" in "%s".', substr($tail, -$tailLen + 1), substr($real, -$tailLen + 1), substr($real, 0, -$tailLen + 1)));
351378
}
352379
}
353-
354-
return true;
355380
}
356381
}
357382

‎src/Symfony/Component/Debug/Tests/phpt/debug_class_loader.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
Test DebugClassLoader with previoulsy loaded parents
3+
--FILE--
4+
<?php
5+
6+
namespace Symfony\Component\Debug\Tests\Fixtures;
7+
8+
use Symfony\Component\Debug\DebugClassLoader;
9+
10+
$vendor = __DIR__;
11+
while (!file_exists($vendor.'/vendor')) {
12+
$vendor = dirname($vendor);
13+
}
14+
require $vendor.'/vendor/autoload.php';
15+
16+
class_exists(FinalMethod::class);
17+
18+
set_error_handler(function ($type, $msg) { echo $msg, "\n"; });
19+
20+
DebugClassLoader::enable();
21+
22+
class_exists(ExtendedFinalMethod::class);
23+
24+
?>
25+
--EXPECTF--
26+
The "Symfony\Component\Debug\Tests\Fixtures\FinalMethod::finalMethod()" method is considered final since version 3.3. It may change without further notice as of its next major version. You should not extend it from "Symfony\Component\Debug\Tests\Fixtures\ExtendedFinalMethod".

0 commit comments

Comments
 (0)