Skip to content

[K/N] Use topological order for resolved libraries#5889

Merged
KotlinBuild merged 2 commits intomasterfrom
prr/troels/toposort-klib-dependencies
Apr 29, 2026
Merged

[K/N] Use topological order for resolved libraries#5889
KotlinBuild merged 2 commits intomasterfrom
prr/troels/toposort-klib-dependencies

Conversation

@troelsbjerre
Copy link
Copy Markdown
Collaborator

Replace the BFS-based traversal in KotlinLibraryResolverResultImpl with a DFS-based one to ensure that resolved libraries are returned in topological order (dependencies before dependents). This change also introduces explicit cyclic dependency detection during resolution.

The explicit call to legacyKlibReverseTopoSort in
NativeSecondStageCompilationConfig is removed as the resolved library list provided by the resolver is now already sorted correctly.

This change speeds up our link step with 5000+ klibs by 10x, as it now does not rely on parsing the dependencies from manifest properties.

@troelsbjerre troelsbjerre requested a review from ddolovov April 17, 2026 13:33
@kotlin-safe-merge
Copy link
Copy Markdown

kotlin-safe-merge Bot commented Apr 17, 2026

Code Owners

RuleOwnersApproval
/​compiler/​ir/​serialization.​common/​, /​compiler/​util-​klib-​metadata/​, /​native/​native.​tests/​klib-​ir-​inliner/​
kotlin-common-backend

@ddolovov 🔒
/​kotlin-​native/​
kotlin-native

@anton-bannykh

@kotlin-safe-merge kotlin-safe-merge Bot dismissed SvyatoslavScherbina’s stale review April 21, 2026 07:26

New changes affect files owned by this reviewer. Re-review required.

@ddolovov
Copy link
Copy Markdown
Contributor

Could you please also drop the other calls to legacyKlibReverseTopoSort() where they happen after a call to getFullList()?

@ddolovov
Copy link
Copy Markdown
Contributor

/dry-run

@KotlinBuild
Copy link
Copy Markdown

THIS IS A DRY RUN

Quality gate is triggered at https://buildserver.labs.intellij.net/build/932359132 — use this link to get full insight.

Quality gate was triggered with the following revisions:

kotlin
Branch: refs/merge/GITHUB-5889/safe-merge
Commit: fb16b36

@KotlinBuild
Copy link
Copy Markdown

Quality gate finished successfully.

Replace the BFS-based traversal in KotlinLibraryResolverResultImpl with
a DFS-based one to ensure that resolved libraries are returned in
topological order (dependencies before dependents). This change also
introduces explicit cyclic dependency detection during resolution.

The explicit call to legacyKlibReverseTopoSort in
NativeSecondStageCompilationConfig is removed as the resolved library
list provided by the resolver is now already sorted correctly.

This change speeds up our link step with 5000+ klibs by 10x, as it now
does not rely on parsing the dependencies from manifest properties.
The legacyKlibReverseTopoSort function, which performed topological
sorting by manually parsing KLIB manifest properties, is no longer needed
now that the KLIB resolver returns libraries in the correct order.

This commit:
- Removes legacyKlibReverseTopoSort from LegacyKlibDependencyUtils.kt.
- Removes remaining call sites in the Kotlin/Native compiler and stub generator.
- Deletes LegacyKlibTopoSortSanityTest.kt as it is no longer relevant.

^KT-70118
@troelsbjerre troelsbjerre force-pushed the prr/troels/toposort-klib-dependencies branch from 7959186 to 06bc8e8 Compare April 28, 2026 13:07
@kotlin-safe-merge kotlin-safe-merge Bot dismissed SvyatoslavScherbina’s stale review April 28, 2026 13:07

New changes affect files owned by this reviewer. Re-review is required.
For internal PRs, review can be finalized by adding /final to the approving message.

@troelsbjerre
Copy link
Copy Markdown
Collaborator Author

/dry-run

@kotlin-safe-merge
Copy link
Copy Markdown

Failed to process command due to an unexpected exception.

@ddolovov
Copy link
Copy Markdown
Contributor

/dry-run

@KotlinBuild
Copy link
Copy Markdown

THIS IS A DRY RUN

Quality gate is triggered at https://buildserver.labs.intellij.net/build/936545097 — use this link to get full insight.

Quality gate was triggered with the following revisions:

kotlin
Branch: refs/merge/GITHUB-5889/safe-merge
Commit: df452b0

@KotlinBuild
Copy link
Copy Markdown

Quality gate finished successfully.

Copy link
Copy Markdown
Contributor

@ddolovov ddolovov left a comment

Choose a reason for hiding this comment

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

/final

Copy link
Copy Markdown
Contributor

@anton-bannykh anton-bannykh left a comment

Choose a reason for hiding this comment

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

👍

@anton-bannykh
Copy link
Copy Markdown
Contributor

/safe-merge

@KotlinBuild
Copy link
Copy Markdown

Quality gate is triggered at https://buildserver.labs.intellij.net/build/937397776 — use this link to get full insight.

Quality gate was triggered with the following revisions:

kotlin
Branch: refs/merge/GITHUB-5889/safe-merge
Commit: 4db5e2f

@KotlinBuild
Copy link
Copy Markdown

Quality gate finished successfully.

@KotlinBuild KotlinBuild merged commit 430b076 into master Apr 29, 2026
2 checks passed
@KotlinBuild KotlinBuild deleted the prr/troels/toposort-klib-dependencies branch April 29, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants