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 Logger::addRecord #1489

Merged
merged 1 commit into from Dec 10, 2020
Merged

Conversation

AntonAcc
Copy link
Contributor

@AntonAcc AntonAcc commented Aug 13, 2020

I was wondering if next/current is faster than foreach. The test


$startTime = microtime(true);
for ($i = 1; $i < 1000000; $i++) {
    $handlerList = [];
    $handlerList[] = 'a';
    $handlerList[] = 'b';
    $handlerList[] = 'c';
    $handlerList[] = 'd';

    foreach ($handlerList as $key => $handler) {
    }
}
echo 'foreach      - ' . (microtime(true) - $startTime) . "\n";

$startTime = microtime(true);
for ($i = 1; $i < 1000000; $i++) {
    $handlerList = [];
    $handlerList[] = 'a';
    $handlerList[] = 'b';
    $handlerList[] = 'c';
    $handlerList[] = 'd';

    while ($handler = current($handlerList)) {
        next($handlerList);
    }
}
echo 'next/current - ' . (microtime(true) - $startTime) . "\n";
foreach      - 0.17546820640564
next/current - 0.38347506523132

http://sandbox.onlinephpfunctions.com/code/0c1d5b1a723309ce669c1d8348b3617def6d3d6b

shows that foreach is faster than next/current.

Then I've done tests


class Handler
{
    private $level = 100;

    private $isHandling = false;

    public function __construct($isHandling)
    {
        $this->isHandling = $isHandling;
    }

    public function isHandling(array $record)
    {
        $result = $record['level'] >= $this->level;

        return $this->isHandling;
    }

    public function handle(array $record)
    {
        $this->isHandling($record);
    }
}

// Current algorithm
$startTime = microtime(true);
for ($i = 1; $i < 100000; $i++) {
    $handlerList = [];
    $handlerList[] = new Handler(false);
    $handlerList[] = new Handler(false);
    $handlerList[] = new Handler(true);
    $handlerList[] = new Handler(false);

    $record = ['level' => 300];
// check if any handler will handle this message so we can return early and save cycles
    $handlerKey = null;
    foreach ($handlerList as $key => $handler) {
        if ($handler->isHandling($record)) {
            $handlerKey = $key;
            break;
        }
    }

    if (null === $handlerKey) {
        continue;
    }

// advance the array pointer to the first handler that will handle this record
    reset($handlerList);
    while ($handlerKey !== key($handlerList)) {
        next($handlerList);
    }

    while ($handler = current($handlerList)) {
        if (true === $handler->handle($record)) {
            break;
        }

        next($handlerList);
    }
}
echo (microtime(true) - $startTime) . "\n";

// Preparing $handleableHandlers algorithm
$startTime = microtime(true);
for ($i = 1; $i < 100000; $i++) {
    $handlerList = [];
    $handlerList[] = new Handler(false);
    $handlerList[] = new Handler(false);
    $handlerList[] = new Handler(true);
    $handlerList[] = new Handler(false);

    $record = ['level' => 300];

    $handleableHandlers = [];
    foreach ($handlerList as $handler) {
        if ($handler->isHandling($record)) {
            $handleableHandlers[] = $handler;
        }
    }

    if (!$handleableHandlers) {
        continue;
    }

    foreach ($handleableHandlers as $handler) {
        if (true === $handler->handle($record)) {
            break;
        }
    }
}
echo (microtime(true) - $startTime) . "\n";

// Suggested algorithm
$startTime = microtime(true);
for ($i = 1; $i < 100000; $i++) {
    $handlerList = [];
    $handlerList[] = new Handler(false);
    $handlerList[] = new Handler(false);
    $handlerList[] = new Handler(true);
    $handlerList[] = new Handler(false);

    $record = ['level' => 300];

    $offset = 0;
    foreach ($handlerList as $handler) {
        if ($handler->isHandling($record)) {
            break;
        }

        $offset++;
    }
    // cut off checked not handling handlers
    $remainedHandlers = array_slice($handlerList, $offset);

    if (!$remainedHandlers) {
        continue;
    }

    foreach ($remainedHandlers as $handler) {
        if (true === $handler->handle($record)) {
            break;
        }
    }
}
echo (microtime(true) - $startTime) . "\n";
0.16189312934875
0.12785387039185
0.13638997077942

http://sandbox.onlinephpfunctions.com/code/6323a783b033b0d6b8217336d5f1b358e15d2f47

As a result, a current algorithm is slowest among others.
The algorithm with preparing $handleableHandlers is faster than current, but slower than suggested. However it has more clean code than suggested.
The suggested algorithm is fastest among others and it is more useful in context of logger than more clean code of algorithm with preparation of $handleableHandlers

PS: However tests on http://sandbox.onlinephpfunctions.com shows that algorithm with preparation of $handleableHandlers is fastest. Plus, it has a more clean code. Maybe, it worth to use this one. I'll be glad to read your opinions.

@Seldaek Seldaek changed the base branch from master to main December 9, 2020 22:32
@Seldaek Seldaek added this to the 2.x milestone Dec 10, 2020
@Seldaek Seldaek added the Bug label Dec 10, 2020
@Seldaek Seldaek merged commit c203cf8 into Seldaek:main Dec 10, 2020
@Seldaek
Copy link
Owner

Seldaek commented Dec 10, 2020

Thanks, merged and simplified this even further in df4d93f as I realized we can actually just do it one loop, I think it's faster than doing array_slice etc, and it reads less confusing.

@lyrixx lyrixx mentioned this pull request Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants