createControllerByID function add cache?

[easydowork]

in yii\base\Module file,the function createControllerByID use preg,can add cache in this function? like this :

public function createControllerByID($id)
    {
        $className = Yii::$app->cache->getOrSet([Yii::$app->id,Yii::getAlias('@app'),'createControllerByID',$id],function () use($id){
            $pos = strrpos($id, '/');
            if ($pos === false) {
                $prefix = '';
                $className = $id;
            } else {
                $prefix = substr($id, 0, $pos + 1);
                $className = substr($id, $pos + 1);
            }

            if ($this->isIncorrectClassNameOrPrefix($className, $prefix)) {
                return null;
            }

            $className = preg_replace_callback('%-([a-z0-9_])%i', function ($matches) {
                    return ucfirst($matches[1]);
                }, ucfirst($className)) . 'Controller';
            $className = ltrim($this->controllerNamespace . '\\' . str_replace('/', '\\', $prefix) . $className, '\\');
            if (strpos($className, '-') !== false || !class_exists($className)) {
                return null;
            }
            return $className;
        }, 3600);

        if (is_subclass_of($className, 'yii\base\Controller')) {
            $controller = Yii::createObject($className, [$id, $this]);
            return get_class($controller) === $className ? $controller : null;
        } elseif (YII_DEBUG) {
            throw new InvalidConfigException('Controller class must extend from \\yii\\base\\Controller.');
        }

        return null;
    }

[bizley]

Is there a significant performance boost with a cache in here?

[easydowork]

Is there a significant performance boost with a cache in here?

$t1 = microtime(true);
        $className = Yii::$app->cache->getOrSet([Yii::$app->id,Yii::getAlias('@app'),'createControllerByID',$id],function () use($id){
            $pos = strrpos($id, '/');
            if ($pos === false) {
                $prefix = '';
                $className = $id;
            } else {
                $prefix = substr($id, 0, $pos + 1);
                $className = substr($id, $pos + 1);
            }

            if ($this->isIncorrectClassNameOrPrefix($className, $prefix)) {
                return null;
            }

            $className = preg_replace_callback('%-([a-z0-9_])%i', function ($matches) {
                    return ucfirst($matches[1]);
                }, ucfirst($className)) . 'Controller';
            $className = ltrim($this->controllerNamespace . '\\' . str_replace('/', '\\', $prefix) . $className, '\\');
            if (strpos($className, '-') !== false || !class_exists($className)) {
                return null;
            }
            return $className;
        });
        $t2 = microtime(true);
        print_r(bcsub($t2,$t1,4));echo PHP_EOL;exit;

The performance difference between using cache and not using cache is about 15 times, as regular expressions are not used anymore

[bizley]

Let's go for it. Could you prepare PR?

[rob006]

The performance difference between using cache and not using cache is about 15 times,

Do you have an actual benchmark? What is so slow in this method that would justify using cache? I tested preg_replace_callback() only and it is at least twice as fast as reading cache using FileCache.

[easydowork]

The performance difference between using cache and not using cache is about 15 times,

Do you have an actual benchmark? What is so slow in this method that would justify using cache? I tested preg_replace_callback() only and it is at least twice as fast as reading cache using FileCache.

I was just a simple test, calculating the running time, and indeed there is such a significant difference. Using cache does not require additional checks and judgments

[rob006]

Using cache does not require additional checks and judgments

Cache has its own overhead, it is not always worth to cache some calculation, sometimes retrieving cached value is slower than calculating it. And it has some other disadvantages: cache takes resources (memory or disk space) and it can be outdated.

[bizley]

I can see that time test is taking cache handling into the consideration, that is why I asked for an implementation proposal. We should double check the actual gain since there are some doubts.

[bizley]

@easydowork could you provide the benchmark without cache vs few cache types? cheers

[easydowork]

@easydowork could you provide the benchmark without cache vs few cache types? cheers

I have never used a benchmark before, I will try it.

[mtangoo]

@easydowork, thanks for taking time to make PR. Can you provide the benchmark with and without https://github.com/yiisoft/yii2/pull/20115/ so that we can make comparison? That would easily help decide if we should consider merge or not, based on merits

[Webkadabra]

how is anyone taking such issues seriously? Please, stop with nonsense issues

[mtangoo]

@Webkadabra who exactly and which comment are you responding to?

[easydowork]

I will do a simple test, using the microtime (true) function to calculate the difference, and replacing preg_replace_callback with cache in the createControllerByID function does indeed improve the calculation time, but it increases the complexity and instability risk of the system. So I gave up.

[mtangoo]

with cache in the createControllerByID function does indeed improve the calculation time, but it increases the complexity and instability risk of the system. So I gave up.

I'm going to close it then. Since it seems to add complexity that isn't worthy it. Correct me If I understood you wrong!