doctrine / lexer

Base library for a lexer that can be used in Top-Down, Recursive Descent Parsers.
https://www.doctrine-project.org/projects/lexer.html
MIT License
11.07k stars 60 forks source link

Memoize object construction and method call result #68

Closed ghostwriter closed 1 year ago

ghostwriter commented 2 years ago

Description

The method getLiteral($token) currently creates a new ReflectionClass every time it's invoked, to get the constants defined on the static class.

This PR attempts to cache the first call and reuse it for subsequent calls.

Diff

--- a/lib/Doctrine/Common/Lexer/AbstractLexer.php
+++ b/lib/Doctrine/Common/Lexer/AbstractLexer.php
     public function getLiteral($token)
     {
+        static $constants = null;
         $className = static::class;
-        $reflClass = new ReflectionClass($className);
-        $constants = $reflClass->getConstants();
+        $constants = $constants ?? (new ReflectionClass($className))->getConstants();

once this package min support version is bumped to PHP 7.4

$constants = $constants ?? (new ReflectionClass($className))->getConstants();

can later be optimized/shortened to

$constants ??= (new ReflectionClass($className))->getConstants();

derrabus commented 2 years ago

Thank you. Please have a look at the CS failure.

SenseException commented 2 years ago

What does this PR introduce or is trying to fix? Can you please write a description?

ghostwriter commented 2 years ago

@SenseException done, ready for review.

SenseException commented 1 year ago

Thank you for clearing up the PR. It's now more understandable what you try to accomplish with the changes. Now it would help to understand "why". Did you run into memory issues or something different because of the repeated creation of the ReflectionClass instance?

ghostwriter commented 1 year ago

No!

anything else you want to know?

greg0ire commented 1 year ago

Feel free to reopen a similar PR when you're ready to be respectful to maintainers. In the meantime, we'll make do without your contribution.