Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance: Prevent Duplicate Calls to getCredentials() #452

Closed
1 task done
ComputerTinker opened this issue Apr 4, 2024 · 3 comments
Closed
1 task done

Performance: Prevent Duplicate Calls to getCredentials() #452

ComputerTinker opened this issue Apr 4, 2024 · 3 comments

Comments

@ComputerTinker
Copy link
Contributor

Checklist

Description

Our web app contains several pages that return collections of thousands of Eloquent models (which frequently include user records) via AJAX for display within data grids. After installing Auth0 I noticed significant performance degradation on many of those pages, on the order of a 2x-6x slowdown:

  • One page which took 17-18s to load, now takes 39-40s
  • Another page which took 23s to load, now takes 1:45-1:48
  • Etc.

In order to get to the bottom of the issue I installed XDebug and ran profiles of a particular page, before and after installing Auth0. (Note that the page was temporarily limited to retrieving 2,000 records so that the profile dump didn't grow too large) As you can see from the profile on the right, a slowdown was seen in Illuminate\Support\Facades\Facade:user (ignore App\User->hasRole):

image

First, though, let me show you what happens if I drill into the Facade:user calls on the left side (without Auth0). Here we can see that Illuminate\Auth\SessionGuard->user is called 2,000 times (once for each Eloquent object I'm retrieving, apparently), but that most of these end with a check of is_null, which takes virtually no time.

image

Looking at their code, you can see that they retrieve the user info once for each request, but then they set $this->user with the results and just return that if it's already set instead of re-retrieving the user details each time.

image

In contrast, if we drill into profile that was taken with Auth0 installed, we can see the performance bottleneck is coming from AuthenticationGuard->getCredential which is being invoked all 2,000 times:

image

A simple hack-ish solution to restrict the credential refresh to only happening the first time per request might go something like this:

public function getCredential(): ?CredentialEntityContract
{
    if ($this->isImpersonating()) {
        return $this->getImposter();
    }

    if (!empty($this->didCredRefresh)) {
        return $this->credential;
    }

    if ($this->credential instanceof CredentialEntityContract) {
        $updated = $this->findSession();
        $this->setCredential($updated);
        $this->pushState($updated);
        $this->didCredRefresh = true;
    }

    return $this->credential;
}

Feel free to replace with something more elegant if you like, but the above hack "worked for me" in my local testing and was able to return performance back to normal.

@evansims
Copy link
Member

evansims commented Apr 9, 2024

👋 Just wanted to thank you for putting together such a detailed breakdown and performance metrics. This is a complex issue for a few reasons. I'll dig into this more as time permits me. Once we figure out a solution, I'd like to create a performance benchmark for our unit tests that can be automated to track regressions there.

I appreciate this and your PR contributions! I'll think on it further

@ComputerTinker
Copy link
Contributor Author

Sounds good @evansims, much appreciated.

@evansims
Copy link
Member

evansims commented Jun 4, 2024

Thanks again for your patience and contributions on this! I released 7.15.0 yesterday, which includes your performance enhancements!

@evansims evansims closed this as completed Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants