Skip to content
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

Codegen EntitySelectionTree + MergedSelections Refactor + Perf Improvments #152

Merged
merged 38 commits into from Dec 20, 2023

Conversation

AnthonyMDev
Copy link
Contributor

@AnthonyMDev AnthonyMDev commented Nov 20, 2023

This PR does a number of things to refactor the Codegen engine in preparation for the disabling of merged fragment fields and improve the performance of the Codegen engine.

  • EntitySelectionTree now computes the merged fields from merged fragments as needed when the merged fields are calculated, rather than when the fragment is initially merged in.
    • This reduces memory consumption by not storing duplicated selections.
    • This likely improves execution time as well
    • Thanks @BobaFetters for the idea to make this algorithmic improvement!
  • RootFieldEntityStorage renamed to DefinitionEntityStorage and attached to definition.
    • The new EntitySelectionTree algorithm requires the entity storage at the time of calculation merged fields (since we are no longer merging the fragments and creating new entities when merging the selections in)
  • IR.Definition refactored from an enum to a protocol.
    • This improves usage flexibility.
  • Refactor IR.SelectionSet
    • Improve initializers and clean up RootFieldBuilder logic.
    • Move merged selections to new ComputedSelectionSet
      • The merged selections require a lot of memory since they hold a lot of duplicate instances of the selections from their direct instance. Making them a lazy var on SelectionSet was convenient for use, but it meant they were retained for the entire time the SelectionSet was in memory. Since IR.Operation and IR.NamedFragment retain their rootField these were being retained for the entire execution of codegen.
      • ComputedSelectionSet, which now computes and stores the merged selections, is created as needed in the SelectionSetTemplate using the new ComputedSelectionSet.Builder. These computed selection sets can be used when needed to render the selection set templates and then be released. This should improve memory consumption greatly.
  • Changes the generated model format for using referenced fragments (precursor to hoisted types).
    • Though we haven't implemented intelligent hoisted types yet, there are times when a field accessor for a merged field points to another generated SelectionSet, rather than generating its own child selection set with the same exact shape.
    • The determination of whether a selection set can use a referenced fragment depends on the merged selections, but we no longer want to compute these and retain them on the IR.SelectionSet, which stays in memory for the entirety of execution.
    • Since we don't compute the merged selections until the generation of that actual selection set, we don't know if it will be a referenced fragment at the time of the generation of field accessors, selection set initializer parameters, etc.
    • To work around this, we are generating those references using the name of the selection set as it would be if it were to be rendered as a child of the current selection set. When we compute the merged selections, if we recognize that we should use a reference to another selection set, we then create a typealias pointing to the correct fully qualified name of the referenced fragment instead of generating the selection set.
  • Create new IRTestWrapper classes in ApolloCodegenInternalTestHelpers.
    • Because a lot of our tests are using the test subscripts to access and verify the built selections using both the direct and merged selections, they would have required a lot of changes and messy code to build the ComputedSelectionSet for each nested child entity.
    • These IRTestWrapper instances wrap the existing IR objects. The subscripts for testing are now provided by the IRTestWrapper, which handle the computation of merged fields internally as they are accessed.
    • This means the changes to the unit test code is minimal, while still maintaining the intentions and functionality of the existing tests.

TODO:

  • Finish fixing the call sites in unit tests as necessary
  • Fix type conflict validation (this has to be done in SelectionSetTemplate when the ComputedSelectionSet has been built.)
  • Improve IRTestWrapper to store the ComputedSelectionSet for reuse during a single test run.
  • Finish documentation updates
  • Run performance tests for metrics

After this PR is completed, the work to create the configuration options for disabling merged fragment fields will be unlocked.

…ragments

WIP:
See entityStorage move Stash SortedSelections.swift for next steps
# Conflicts:
#	apollo-ios-codegen/Sources/IR/IR+DirectSelections.swift
#	apollo-ios-codegen/Sources/IR/IR+EntitySelectionTree.swift
#	apollo-ios-codegen/Sources/IR/IR+RootFieldBuilder.swift
@AnthonyMDev AnthonyMDev marked this pull request as ready for review December 8, 2023 19:12
@AnthonyMDev AnthonyMDev requested a review from a team as a code owner December 8, 2023 19:12
@AnthonyMDev AnthonyMDev marked this pull request as draft December 8, 2023 19:12
Copy link

netlify bot commented Dec 16, 2023

Deploy Preview for eclectic-pie-88a2ba ready!

Name Link
🔨 Latest commit 4afc607
🔍 Latest deploy log https://app.netlify.com/sites/eclectic-pie-88a2ba/deploys/65835d7753524900099ec6b1
😎 Deploy Preview https://deploy-preview-152--eclectic-pie-88a2ba.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pm-dev
Copy link

pm-dev commented Dec 19, 2023

Looking forward to this one

@AnthonyMDev
Copy link
Contributor Author

@pm-dev. This is the foundational refactoring needed to enable disabling of fragment field merging. That will finally get us to a point where the codegen will work on your project!

@AnthonyMDev AnthonyMDev marked this pull request as ready for review December 19, 2023 17:41
Copy link
Member

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

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

Great improvements here, thank you!

AnthonyMDev and others added 5 commits December 20, 2023 13:31
Co-authored-by: Calvin Cestari <calvincestari@users.noreply.github.com>
Co-authored-by: gh-action-runner <runner@Mac-1702348611167.local>
Co-authored-by: gh-action-runner <runner@Mac-1702496898515.local>
Co-authored-by: gh-action-runner <runner@Mac-1702507023897.local>
Co-authored-by: Jeff Auriemma <bignimbus@users.noreply.github.com>
Co-authored-by: gh-action-runner <runner@Mac-1702586757511.local>
Co-authored-by: Zach FettersMoore <zach.fetters@apollographql.com>
Co-authored-by: SecOps[bot] <136828330+svc-secops@users.noreply.github.com>
Co-authored-by: gh-action-runner <runner@Mac-1702920310116.local>
Co-authored-by: gh-action-runner <runner@Mac-1702923964694.local>
@AnthonyMDev AnthonyMDev merged commit 08d02a3 into main Dec 20, 2023
19 checks passed
@AnthonyMDev AnthonyMDev deleted the entity-selection-tree-merge-fragment-ref branch December 20, 2023 21:59
BobaFetters pushed a commit to apollographql/apollo-ios-codegen that referenced this pull request Dec 20, 2023
BobaFetters pushed a commit that referenced this pull request Dec 20, 2023
2457cb65 Codegen EntitySelectionTree + MergedSelections Refactor + Perf Improvments (#152)

git-subtree-dir: apollo-ios-codegen
git-subtree-split: 2457cb659273c7938a4a757f5676b216c5f4d720
BobaFetters pushed a commit that referenced this pull request Dec 20, 2023
…ergedSelections Refactor + Perf Improvments

git-subtree-dir: apollo-ios-codegen
git-subtree-mainline: 8b2eb70
git-subtree-split: 2457cb659273c7938a4a757f5676b216c5f4d720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants