Skip to content

Conversation

@spawnia
Copy link
Collaborator

@spawnia spawnia commented Dec 5, 2025

Resolves #1803

Follow up to #1790

spawnia and others added 2 commits December 5, 2025 15:11
@spawnia
Copy link
Collaborator Author

spawnia commented Dec 5, 2025

Summary

This PR optimizes memory usage for deferred execution while preserving the DataLoader pattern that was broken in #1790.

Problem

PR #1790 attempted to optimize memory but broke DataLoader batching - batch loads executed incrementally before all IDs were collected, causing null values with 600+ items.

Solution

Reduces memory usage by replacing closure-based queue storage with direct object/array storage:

  1. Store executor on promise instance - $executor property instead of closure context
  2. Queue promise objects directly - enqueue($this) instead of enqueue(function() {...})
  3. Use arrays for waiting handlers - [$promise, $onFulfilled, $onRejected, $state, $result] instead of closures
  4. Explicit cleanup - $task->executor = null and unset($task) enable GC during long queues

Tests Added

Verification

spawnia and others added 7 commits December 9, 2025 14:26
Store promise references and arrays in the queue instead of closures
to reduce memory footprint. This approach:

- Stores the executor callable on the SyncPromise instance rather than
  capturing it in a closure
- Queues the promise reference itself (smaller than a closure)
- Uses arrays instead of closures in enqueueWaitingPromises()
- Preserves DataLoader batching pattern (no premature queue processing)

Key changes:
- Add $executor property to SyncPromise for storing the executor
- Update __construct() to store executor and queue $this
- Update runQueue() to handle promise refs, arrays, and callables
- Add processWaitingTask() for processing waiting promise arrays
- Update enqueueWaitingPromises() to queue arrays instead of closures

This maintains full compatibility with the DataLoader pattern since
all Deferred executors are still queued BEFORE any are executed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@spawnia spawnia marked this pull request as ready for review December 9, 2025 15:58
@spawnia
Copy link
Collaborator Author

spawnia commented Dec 9, 2025

@shadowhand @zimzat @joshbmarshall Can you give this a review?

@joshbmarshall
Copy link

I loaded it up on the system that was having problems with v15.27.0

It still has issues with v15.27.0 tag, but this is working for me at 1954f81

For my query, the memory usage is identical for this pull request as for master branch. But I think it's because I have nested resolvers:

event date > event > event dates

I think it's waiting for all the event dates to be queued before resolving, which is how I'd expect it to be. I don't think that can be managed!

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.

Getting null instead of a list of items for v15.27.0

3 participants