Skip to content

Commit

Permalink
minor #52253 [Messenger] Add call to gc_collect_cycles() after each…
Browse files Browse the repository at this point in the history
… message is handled (jwage)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Messenger] Add call to `gc_collect_cycles()` after each message is handled

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| License       | MIT

I have been working on improving memory usage in my messenger worker queues and I had been chasing what I thought was a memory leak, but I haven't been able to find it. In my troubleshooting, I pushed a change to production which simply logs the result of `gc_status()` after each message is handled, and I noticed that it was taking a very long time for the automatic garbage collection to run. Additionally, each time it would finally run, the PHP algorithm internally would extend the threshold, making it take even longer to run the next time. So the number of objects in memory waiting to be collected would keep growing and the memory per process would grow. In my application, it is important for the memory for each worker process to stay relatively flat so that I can know how many worker processes I can fit on each worker server.

I added a call to `gc_collect_cycles()` in my application after each message is handled, and it seems to have improved things for me. I suppose it is still possible I have a memory leak, but I haven't been able to find it.

Here is a screenshot of memory usage from one of my worker servers after I deployed that change.

![Screenshot 2023-10-23 at 12 32 46 PM](https://github.com/symfony/symfony/assets/97422/5f7c287a-278c-46b7-af7f-6bd6d71b37c9)

In my application, it takes < 1ms to call `gc_collect_cycles()` after each message is handled and that is satisfactory for me in order to keep memory usage lower.

If this is accepted, should we make it optionally enabled?

Commits
-------

b0df65a [Messenger] Add call to `gc_collect_cycles()` after each message is handled
  • Loading branch information
nicolas-grekas committed Oct 25, 2023
2 parents ab665d6 + b0df65a commit 8720b78
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
19 changes: 19 additions & 0 deletions src/Symfony/Component/Messenger/Tests/WorkerTest.php
Expand Up @@ -578,6 +578,25 @@ public function testFlushBatchOnStop()

$this->assertSame($expectedMessages, $handler->processedMessages);
}

public function testGcCollectCyclesIsCalledOnMessageHandle()
{
$apiMessage = new DummyMessage('API');

$receiver = new DummyReceiver([[new Envelope($apiMessage)]]);

$bus = $this->createMock(MessageBusInterface::class);

$dispatcher = new EventDispatcher();
$dispatcher->addSubscriber(new StopWorkerOnMessageLimitListener(1));

$worker = new Worker(['transport' => $receiver], $bus, $dispatcher);
$worker->run();

$gcStatus = gc_status();

$this->assertGreaterThan(0, $gcStatus['runs']);
}
}

class DummyReceiver implements ReceiverInterface
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Messenger/Worker.php
Expand Up @@ -118,6 +118,8 @@ public function run(array $options = []): void
// this should prevent multiple lower priority receivers from
// blocking too long before the higher priority are checked
if ($envelopeHandled) {
gc_collect_cycles();

break;
}
}
Expand Down

0 comments on commit 8720b78

Please sign in to comment.