-
-
Notifications
You must be signed in to change notification settings - Fork 569
Optimize deferred #1805
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
base: master
Are you sure you want to change the base?
Optimize deferred #1805
Conversation
SummaryThis PR optimizes memory usage for deferred execution while preserving the DataLoader pattern that was broken in #1790. ProblemPR #1790 attempted to optimize memory but broke DataLoader batching - batch loads executed incrementally before all IDs were collected, causing null values with 600+ items. SolutionReduces memory usage by replacing closure-based queue storage with direct object/array storage:
Tests Added
Verification
|
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>
|
@shadowhand @zimzat @joshbmarshall Can you give this a review? |
|
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! |
Resolves #1803
Follow up to #1790