shield icon indicating copy to clipboard operation
shield copied to clipboard

refactor: `AuthToken`/`AuthJWT` Config Loading by Initializing in Constructor

Open datamweb opened this issue 9 months ago • 7 comments

Description AuthToken and AuthJWT Config Loading by Initializing in Constructor

Checklist:

  • [x] Securely signed commits
  • [ ] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [ ] Unit testing, with >80% coverage
  • [ ] User guide updated
  • [x] Conforms to style guide

datamweb avatar Feb 16 '25 00:02 datamweb

Do you have a benchmark for this change?

The presence of Optimize probably led to this question. It seems I made a mistake in choosing it, as my main goal in this configuration was to prevent the repeated use of config('AuthToken') and config('AuthJWT'), nothing more.

But now I’m curious—does this change actually impact performance or not? Your benchmark results could be really helpful! @paulbalandan If possible, please share them so we can analyze them together.

datamweb avatar Feb 16 '25 13:02 datamweb

Yes, the word Optimize in your PR title made me curious about the impact to performance since we will now be loading the configs everytime a class is instantiated. I just remembered one time when I did a similar change to the Seeder class instantiating Faker's Generator everytime in the constructor caused a massive slowdown.

paulbalandan avatar Feb 17 '25 00:02 paulbalandan

Are we actually calling an attempt method multiple times per request?

michalsn avatar Feb 27 '25 13:02 michalsn

Are we actually calling an attempt method multiple times per request?

I didn't quite understand your question, and I'm not sure how it relates to this PR. Could you please clarify?

datamweb avatar Feb 27 '25 22:02 datamweb

Sorry, your reason for this change was described as:

my main goal in this configuration was to prevent the repeated use of config('AuthToken') and config('AuthJWT')

So my question is. is this really the case? Are we actually calling these configs multiple times, or only once per request?

michalsn avatar Feb 28 '25 05:02 michalsn

is this really the case? Are we actually calling these configs multiple times, or only once per request?

In fact, the configuration is only called once per request. Therefore, it seems this change wouldn’t have a significant impact on performance. 100%

Which one do you find more suitable and why?"

First method:

<?php

declare(strict_types=1);

class MyClass
{

    public function __construct() {
        $this->config = config('Xconfig');
    }

    public function Y()
    {
        $this->config;
    }

    public function X()
    {

        $this->config;

    }
}

Second method:

<?php

declare(strict_types=1);

class MyClass
{

    public function __construct() {}

    public function Y()
    {
        $config = config('Xconfig);
    }

    public function X()
    {

        $config = config('Xconfig);

    }
}

datamweb avatar Feb 28 '25 06:02 datamweb

Ok, sorry - that was my mistake. I thought we call this config only in one method. But now I see we call it several times.

michalsn avatar Feb 28 '25 07:02 michalsn