-
-
Notifications
You must be signed in to change notification settings - Fork 301
Add support for parallel linking in IL2CPP builds #764
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
base: main
Are you sure you want to change the base?
Conversation
* Introduced 'enableParallelLinking' input in action.yml to control parallel linking. * Updated Builder.cs to apply parallel linking settings based on the new input. * Modified build scripts for macOS, Ubuntu, and Windows to include the parallel linking option.
📝 WalkthroughWalkthroughAdds a new boolean action input Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #764 +/- ##
==========================================
+ Coverage 38.37% 38.43% +0.05%
==========================================
Files 78 78
Lines 3171 3174 +3
Branches 665 666 +1
==========================================
+ Hits 1217 1220 +3
Misses 1809 1809
Partials 145 145
🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/model/input.ts (1)
146-150: Boolean getter works; consider whether default should betrueeverywhereThe getter follows the existing pattern for
manualExit/enableGpuand will correctly returntruewhen the action input is'true'. In GitHub Actions runs, theaction.ymldefault'true'means this flag is enabled by default, but in non‑GitHub contexts (e.g. CLI/CloudRunner without explicit input) the?? falsefallback makes the default effectivelyfalse.If you want “parallel linking on by default” consistently across all entrypoints, consider defaulting the fallback to
'true'instead:const input = Input.getInput('enableParallelLinking') ?? 'true'; return input === 'true';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
dist/default-build-script/Assets/Editor/UnityBuilderAction/Builder.csis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/platforms/mac/steps/build.shis excluded by!**/dist/**dist/platforms/ubuntu/steps/build.shis excluded by!**/dist/**dist/platforms/windows/build.ps1is excluded by!**/dist/**
📒 Files selected for processing (5)
action.yml(1 hunks)src/model/build-parameters.ts(2 hunks)src/model/image-environment-factory.ts(1 hunks)src/model/input.ts(1 hunks)src/model/platform-setup/setup-mac.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-05T17:12:46.110Z
Learnt from: GabLeRoux
Repo: game-ci/unity-builder PR: 0
File: :0-0
Timestamp: 2024-12-05T17:12:46.110Z
Learning: For Unity version 6 and above, the -activeBuildProfile parameter cannot be used together with -buildTarget parameter as they are mutually exclusive. Build profiles are a new feature that encapsulate build settings including the target platform.
Applied to files:
action.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests
🔇 Additional comments (5)
action.yml (1)
46-49: NewenableParallelLinkinginput is well‑named and consistentInput name, default, and description are clear and align with the downstream
Input.enableParallelLinking/ENABLE_PARALLEL_LINKINGwiring. No issues from the action metadata side.src/model/platform-setup/setup-mac.ts (1)
197-197: Mac host env wiring forENABLE_PARALLEL_LINKINGlooks correctUsing
buildParameters.enableParallelLinking.toString()matches the existing patterns forMANUAL_EXIT/ENABLE_GPUand keeps host‑side scripts in sync with the Docker env naming.src/model/image-environment-factory.ts (1)
47-47: Docker env propagation ofENABLE_PARALLEL_LINKINGis consistentAdding
{ name: 'ENABLE_PARALLEL_LINKING', value: parameters.enableParallelLinking }alongsideENABLE_GPUcleanly exposes the new flag to the container; the existinggetEnvVarStringlogic will stringify the boolean correctly.src/model/build-parameters.ts (2)
37-37: NewenableParallelLinkingproperty fits existing parameter modelBoolean field placement next to
enableGpumatches the existing configuration style and keeps the model coherent.
166-166: Propagation fromInput.enableParallelLinkingintoBuildParametersis correctPassing
Input.enableParallelLinkingthrough increate()ensures all consumers (Docker env, mac setup, etc.) receive the new flag without extra plumbing.
The nitpick comment here has been addressed in onibi-gg/unity-builder@ff72fec . |
|
Is that flag documented somewhere? I couldn't find it from a google search. It would be nice to add it to unity-test-runner also to speed up standalone builds. |
This only has effect with IL2CPP since mono builds do not require a linking stage. I'd be surprised if the test runner used IL2CPP, I can't see much benefit to it except in very limited situations where code would have different results on mono vs. IL2CPP. |
|
I use it to test il2cpp builds for my library. The build part takes up half the test time, so any speedup would be appreciated. 🙂 |
Ah. Does the test-runner have a separate repo and build script? This same technique should work on that too, I'd guess. |
|
BTW I need to update the patch, since this looks at the targetplatform to set platform-specific linker flags, but I need to change it to look at the build platform since that is where the compiler differences are. EDIT: this is done |
|
I'll see what I can do. |
|
For visibility: game-ci/unity-test-runner#300 (comment) |

Changes
This adds support for and enables by default parallel linking for IL2CPP builds.
IL2CPP builds spend a lot of time in the linking stage, since the compilation step is already parallel. This updates the build to make unity use parallel linking, which can significantly speed up builds, especially if the library and incremental build cache is used, in which cases linking is where most of the build time is spent.
Successful Workflow Run Link
I've been using this in our private repo for a bit and it seems to work, although ios, osX and Android builds are untested.
TODOs
If people are interested in this change I'll update documentation and README as well.
Checklist
code of conduct
in the documentation repo)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.