GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GQLGW-5297-optimise-incremental-part-execution-for-defer-requests by llin2 · Pull Request #4174 · graphql-java/graphql-java

Conversation

@llin2

@llin2 llin2 commented Nov 16, 2025

Copy link
Copy Markdown
Contributor

This is to optimise defer execution for incremental results to begin processing as soon as the first incremental call is detected rather than on completion of the initial result.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@llin2
GQLGW-5297-optimise-incremental-part-execution-for-defer-requests
dd8313b
andimarek
andimarek requested changes Nov 17, 2025
View reviewed changes
Comment thread src/main/java/graphql/execution/Execution.java Resolved

Uh oh!

There was an error while loading. Please reload this page.
llin2 added 2 commits November 17, 2025 14:57
@llin2
GQLGW-666-do-not-start-on-separate-thread
7e8d69a
@llin2
GQLGW-666-move-start-to-after-initial-fields-resolve-with-info
a5c458e
bbakerman
bbakerman reviewed Nov 17, 2025
View reviewed changes
Comment thread src/test/groovy/graphql/execution/incremental/DeferExecutionSupportIntegrationTest.groovy Outdated
initialResult.incrementalItemPublisher.subscribe(sub)

then:
Awaitility.await().untilTrue(sub.isDone())

@bbakerman bbakerman Nov 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs an assert on the contents of the Publisher

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
bbakerman
bbakerman reviewed Nov 17, 2025
View reviewed changes
Comment thread src/test/groovy/graphql/execution/incremental/DeferExecutionSupportIntegrationTest.groovy Resolved

Uh oh!

There was an error while loading. Please reload this page.
bbakerman
bbakerman reviewed Nov 17, 2025
View reviewed changes
Comment thread src/main/java/graphql/execution/ExecutionStrategy.java Outdated
&& deferredExecutionSupport.deferredFieldsCount() > 0
&& executionContext.getGraphQLContext().getBoolean(IncrementalExecutionContextKeys.ENABLE_EAGER_DEFER_START, false)) {

executionContext.getIncrementalCallState().startDeferredCalls();

@bbakerman bbakerman Nov 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call here is only need as a side effect to create the memoizing supplier (of the Publisher)

I think we should invent a new method that does that and hides the side effect

executionContext.getIncrementalCallState().startEarlyDraining();

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bbakerman bbakerman Nov 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hang on you have that - startDrainingNow() and it calls publisher.get()

I dont think executionContext.getIncrementalCallState().startDeferredCalls(); is needed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@llin2
GQLGW-5297-assert-on-result-and-remove-redundant-call
50169f0
bbakerman
bbakerman reviewed Nov 19, 2025
View reviewed changes
Comment thread src/main/java/graphql/execution/incremental/IncrementalExecutionContextKeys.java

}


@bbakerman bbakerman Nov 19, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to create a new entry in the unusual config - lets put it in

graphql.GraphQLUnusualConfiguration.IncrementalSupportConfig

        /**
         * This controls whether @defer field execution starts as early as possible.
         */
        @ExperimentalApi
        public IncrementalSupportConfig enableEarlyFieldExecution(boolean enable) {
            contextConfig.put(YOUR_KEY_HERE, enable);
            return this;
        }

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@llin2
GQLGW-5297-add-unusual-config
0f365b5
andimarek
andimarek reviewed Nov 19, 2025
View reviewed changes
Comment thread src/main/java/graphql/execution/incremental/IncrementalCallState.java Outdated
}

public void startDrainingNow() {
startDeferredCalls();

@andimarek andimarek Nov 19, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to call start Deferred calls as it just creates the publisher

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@llin2
GQLGW-5297-remove-start-defer-calls
18207de
@andimarek
andimarek merged commit aa0b31e into graphql-java:master Nov 30, 2025
4 checks passed

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bbakerman bbakerman bbakerman left review comments
@andimarek andimarek andimarek requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@llin2 @bbakerman @andimarek
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.