tormjens / eventy

WordPress-like actions and filters for Laravel
422 stars 48 forks source link

Performance concerns #88

Closed guiyumin closed 3 weeks ago

guiyumin commented 4 weeks ago

Hi,

Your library is very powerful. While I appreciate it, I do have one concern: performance.

Do you think it's possible that if we add too many filters and actions, the performance could be hurt ?

So, first of all, is my concern legit, or just imaginary ? Do we have plan to address this concern, if it's indeed a legit concern ?

Thanks.


public function fire($action, $args)
    {
        $this->value = isset($args[0]) ? $args[0] : ''; // get the value, the first argument is always the value

        foreach ($this->getListeners($action) as $listener) {
            $parameters = [];
            $args[0] = $this->value;
            for ($i = 0; $i < $listener['arguments']; $i++) {
                $value = $args[$i] ?? null;
                $parameters[] = $value;
            }
            $this->value = call_user_func_array($this->getFunction($listener['callback']), $parameters);
        }

        return $this->value;
    }

in this function, it iterates the array. what if some lunatic adds 10000 actions or filters ?

If I am overthinking it, I apologize. But I am really concerned.

tormjens commented 4 weeks ago

Well, that depends on what code the lunatic adds to their listeners. Any code that is "injected" might affect performance if not optimised properly.

That would be more on the implementation side of things, though, and not a consideration I would take into account in this package.

Simple mutations of a string (filtering) would be fine even in the thousands, but if you add queries in each listener, you would add a significant performance overhead. That would also be true if you used Laravel's events.

If you have a suggestion how you think this package can help mitigate such an issue, I'm all ears.

guiyumin commented 3 weeks ago

Well, that depends on what code the lunatic adds to their listeners. Any code that is "injected" might affect performance if not optimised properly.

That would be more on the implementation side of things, though, and not a consideration I would take into account in this package.

Simple mutations of a string (filtering) would be fine even in the thousands, but if you add queries in each listener, you would add a significant performance overhead. That would also be true if you used Laravel's events.

If you have a suggestion how you think this package can help mitigate such an issue, I'm all ears.

Thank you so much for the quick reply.

I think I do understand that we cannot prevent lunatics from abusing our system/library.

And I have no idea how to address this issue. Sorry.