Skip to content

Optimize DataFetchingSelectionSet.getImmediateFields() to avoid traversing descendants #3855

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 20, 2025
Merged

Optimize DataFetchingSelectionSet.getImmediateFields() to avoid traversing descendants #3855

merged 2 commits into from
Mar 20, 2025

Conversation

timward60
Copy link
Contributor

@timward60 timward60 commented Mar 18, 2025 •
edited
Loading

Addresses #3854.

Description

Computes immediate fields only for getImmediateFields() to avoid cost of traversing the entire sub-tree to build glob-patterns for descendant fields.

Benchmark

Before:

Benchmark                                                          Mode  Cnt   Score   Error   Units
DFSelectionSetPerformance.benchMarkThroughput                     thrpt    9  10.424 ± 0.598  ops/ms
DFSelectionSetPerformance.benchMarkThroughput_getImmediateFields  thrpt    9  21.992 ± 0.224  ops/ms
DFSelectionSetPerformance.benchMarkAvgTime                         avgt    9   0.096 ± 0.005   ms/op
DFSelectionSetPerformance.benchMarkAvgTime_getImmediateFields      avgt    9   0.044 ± 0.001   ms/op

After:

Benchmark                                                          Mode  Cnt     Score    Error   Units
DFSelectionSetPerformance.benchMarkThroughput                     thrpt    9    10.641 ±  0.118  ops/ms
DFSelectionSetPerformance.benchMarkThroughput_getImmediateFields  thrpt    9  2667.783 ± 73.924  ops/ms
DFSelectionSetPerformance.benchMarkAvgTime                         avgt    9     0.094 ±  0.003   ms/op
DFSelectionSetPerformance.benchMarkAvgTime_getImmediateFields      avgt    9    ≈ 10⁻³            ms/op

@andimarek
Copy link
Member

Hey @timward60 ... thanks a lot for the PR and even more for the Benchmark.

I have a request:

We just introduced automated performance tests, which enables us to track the performance much better over time as it is executed in a stable reproducible environment.

Can you create another PR which moves the DFSelectionSetBenchmark into the performance package and renames into DFSelectionSetPerformance?

We can merge this PR first and it gives us a clear baseline for the improvements. Thanks a lot

@timward60
Copy link
Contributor Author

Hey @timward60 ... thanks a lot for the PR and even more for the Benchmark.

I have a request:

We just introduced automated performance tests, which enables us to track the performance much better over time as it is executed in a stable reproducible environment.

Can you create another PR which moves the DFSelectionSetBenchmark into the performance package and renames into DFSelectionSetPerformance?

We can merge this PR first and it gives us a clear baseline for the improvements. Thanks a lot

Nice to see, created the PR here: #3856

@timward60 timward60 force-pushed the timward/data-fetching-selection-set-immediate-fields branch from 96fab24 to 0868473 Compare March 19, 2025 00:24
@andimarek
Copy link
Member

@timward60 you can see the performance results from master tests here: https://github.com/graphql-java/graphql-java/tree/master/performance-results

@dondonz dondonz added this to the 23.x breaking changes milestone Mar 20, 2025
bbakerman
bbakerman approved these changes Mar 20, 2025
View reviewed changes
Copy link
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

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

Thanks very much for this

@bbakerman bbakerman merged commit 9a931ba into graphql-java:master Mar 20, 2025
1 check passed
@dondonz dondonz added the performance work that is primarily targeted as performance improvements label Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbakerman bbakerman bbakerman approved these changes

@andimarek andimarek andimarek approved these changes

Assignees
No one assigned
Labels
performance work that is primarily targeted as performance improvements
Projects
None yet
Milestone
23.0 breaking changes
Development

Successfully merging this pull request may close these issues.
4 participants
@timward60 @andimarek @bbakerman @dondonz