Skip to content

Remove dead projection recursion state - #964

Open
iuliiasobolevska wants to merge 1 commit into
masterfrom
cleanup/remove-dead-projection-recursion-state
Open

iuliiasobolevska wants to merge 1 commit into
masterfrom
cleanup/remove-dead-projection-recursion-state

Conversation

@iuliiasobolevska

@iuliiasobolevska iuliiasobolevska commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • remove the unused processedEdges and queryDepth state from client projection generation
  • avoid copying the unused edge set for every projectable field
  • add direct and mutual cycle coverage pinned to the existing generated projection bytes

The removed state was a cycle/depth guard in the retired v1 generator, but the surviving implementation only copied and forwarded it. Projection generation remains bounded by generatedClasses.

Performance

Three alternating fresh-JVM runs of the exact base (fd6723e3) and PR (e315c9ea) revisions used the same Java 17 CLI configuration and 3.7 MB consumer schema.

Revision Runs Median Range
Base 109.04 s, 130.35 s, 115.60 s 115.60 s 109.04–130.35 s
PR 104.58 s, 105.93 s, 110.13 s 105.93 s 104.58–110.13 s

Median wall time improved by 9.67 seconds, or 8.4%. The base had a wide 21.31-second range, so this is best treated as a directional result for this workload rather than a precise or universal speedup. Both variants generated 9,000 byte-identical files.

Verification

  • JAVA_HOME=/Users/isobolevska/.sdkman/candidates/java/17.0.19-zulu ./gradlew :graphql-dgs-codegen-core:formatKotlinMain :graphql-dgs-codegen-core:formatKotlinTest :graphql-dgs-codegen-core:test --no-scan
  • generated the 3.7 MB consumer schema before and after the change
  • confirmed all 9,000 generated files are byte-identical with diff -rq

assertThat(projectionHashes)
.containsExactlyEntriesOf(
linkedMapOf(
"CycleProjectionRoot" to "2824004364fedef8ee06b9474c405fbac55a1551f28f9c23c806307aecf6b550",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Asserting the hashed values as strings feels a bit brittle, could maybe consider something like

val projectionNames = codeGenResult.clientProjections.map { it.typeSpec().name() }
        assertThat(projectionNames)
            .containsExactlyInAnyOrder(
                "CycleProjectionRoot",
                "DirectCycleProjection",
                "MutualAProjection",
                "MutualBProjection",
            )

        val projections = codeGenResult.clientProjections.associateBy { it.typeSpec().name() }
        assertThat(projections.getValue("DirectCycleProjection").typeSpec().methodSpecs())
            .extracting("name")
            .contains("name", "self", "mutual")
...

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants