fix: insufficient shared memory available in Unity 6.6 beta#841
fix: insufficient shared memory available in Unity 6.6 beta#841mob-sakai wants to merge 1 commit into
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesDocker Shared Memory Size Configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 #841 +/- ##
=======================================
Coverage 45.34% 45.34%
=======================================
Files 36 36
Lines 688 688
Branches 199 199
=======================================
Hits 312 312
Misses 337 337
Partials 39 39
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/model/docker.ts (1)
71-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated magic value
1025macross both templates.The shared memory size literal is duplicated verbatim in both
getLinuxCommandandgetWindowsCommand. Extracting it to a named constant would make the rationale (matching Unity's exact1025Mrequest) clearer and avoid future drift if one copy is updated without the other.♻️ Proposed refactor
+const DOCKER_SHM_SIZE = '1025m'; + class Docker {return `docker run \ - --shm-size=1025m \ + --shm-size=${DOCKER_SHM_SIZE} \ --workdir ${dockerWorkspacePath} \return `docker run \ - --shm-size=1025m \ + --shm-size=${DOCKER_SHM_SIZE} \ --workdir c:${dockerWorkspacePath} \Also applies to: 119-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/model/docker.ts` at line 71, The shared memory size value is duplicated in both command builders, so extract the `1025m` literal into a named constant and reuse it from `getLinuxCommand` and `getWindowsCommand`. Update the Docker command templates to reference that shared constant so the Unity-specific shared memory requirement stays consistent in one place and avoids drift between the two methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/model/docker.ts`:
- Line 71: The shared memory size value is duplicated in both command builders,
so extract the `1025m` literal into a named constant and reuse it from
`getLinuxCommand` and `getWindowsCommand`. Update the Docker command templates
to reference that shared constant so the Unity-specific shared memory
requirement stays consistent in one place and avoids drift between the two
methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2a523b0e-be3b-4b9b-97bb-3128c54fa3d4
⛔ Files ignored due to path filters (3)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**
📒 Files selected for processing (1)
src/model/docker.ts
mob-sakai
force-pushed
the
fix/support-unity6.6
branch
from
0d7771e to
6d3e4dc
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/model/docker.ts (1)
71-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix looks correct; consider making shm-size configurable.
The hardcoded
--shm-size=1025mmatches the value recommended in the reported Docker error and resolves the reported build failures. SincedockerCpuLimit/dockerMemoryLimitare already exposed as configurableDockerParameters, consider exposing shm-size similarly so users with different project sizes/CI environments can tune it without a code change.♻️ Optional: expose as a parameter
- return `docker run \ - --shm-size=1025m \ + return `docker run \ + --shm-size=${dockerShmSize ?? '1025m'} \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/model/docker.ts` at line 71, The Docker setup in docker.ts hardcodes the shm-size value, so make it configurable like dockerCpuLimit and dockerMemoryLimit in DockerParameters. Update the Docker command-building logic to accept a shm-size parameter and thread it through the same path used by the existing configurable limits, so users can tune the shared memory setting without changing code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/model/docker.ts`:
- Line 71: The Docker setup in docker.ts hardcodes the shm-size value, so make
it configurable like dockerCpuLimit and dockerMemoryLimit in DockerParameters.
Update the Docker command-building logic to accept a shm-size parameter and
thread it through the same path used by the existing configurable limits, so
users can tune the shared memory setting without changing code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 238fa1b1-710a-4a6f-a0c1-b05c1d8af6d0
⛔ Files ignored due to path filters (3)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**
📒 Files selected for processing (1)
src/model/docker.ts
mob-sakai
force-pushed
the
fix/support-unity6.6
branch
from
6d3e4dc to
bc9b9ac
Compare
1 participant
Changes
--shm-size=1025mas an argument todocker runand tests and builds completed successfully.Related Issues
Related PRs
Successful Workflow Run Link
PRs don't have access to secrets so you will need to provide a link to a successful run
of the workflows from your own repo.
game-ci/unity-builder@mainmob-sakai/unity-builder@0d7771ecc39c2b208a80daf0f5e0ef5f2d528ccd(fixed)Checklist
in the documentation repo)
Summary by CodeRabbit
Summary
runcommand to include--shm-size=1025m(for both Linux and Windows execution paths), improving reliability for memory-intensive workloads inside containers.