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

Speed up eventy event firing #3263

Merged

Conversation

OttoWinter
Copy link
Contributor

Freescout appears to use Eventy to allow modules to customize certain aspects of freescout, for example the user getFullName:

https://github.com/freescout-helpdesk/freescout/blob/40fc84546168c7fed1eacf55759ff008371815f4/app/User.php#L216

Unfortunately, this eventy library appears to be quite inefficient, especially when you have large amounts of listeners (it does a O(n) search through all listeners globally to find matching ones, plus each time does a O(n log n) sort by priority).

For example, on an instance I was profiling, there were ~120 listeners, thus making a single (!) User::getFullName take a whopping 3ms (with ~100 getFullName calls for listing assignable users this has a major impact; and that's just for the getFullName calls).

This PR overrides Eventy, to replace its internal data structure to be more efficient (keyed by the hook).

In total, this reduced mailbox loading times for my organization from ~1.5s to ~0.5s.

Note: I barely know PHP (in fact most of this code was generated by Copilot), so the code probably is very bad ^^ If there's something that can be done better let me know.

Also note that technically I'm changing the (public) getListeners function argument types here. This method only appears to be used inside Eventy, but this could still cause problems (it works on my instance though).

@freescout-helpdesk freescout-helpdesk merged commit 321a87c into freescout-help-desk:master Aug 15, 2023
@freescout-helpdesk
Copy link
Collaborator

freescout-helpdesk commented Aug 15, 2023

Thanks. Very good improvement. Your code is good.

For us page load speed improved by an average of about 30%.

@raramuridesign
Copy link

@OttoWinter Thank you for this, it has improved our installations
M.

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

Successfully merging this pull request may close these issues.

None yet

3 participants