fix: resolve server startup hang on macOS ARM64 (Apple Silicon)#3902
Open
fix: resolve server startup hang on macOS ARM64 (Apple Silicon)#3902
Conversation
LuaJIT runs in interpreter-only mode on macOS ARM64 (JIT disabled). The require() function's package.searchpath iterates through every entry in package.path doing string.sub operations for each call. With 50+ nested require calls during quest catalog loading, the cumulative cost causes the server to hang for 10+ minutes at startup. Three changes to fix this: - Set minimal package.path in C++ after luaL_openlibs to strip system/env LUA_PATH entries; extend it in core.lua with only the needed libs directory - Replace require() with dofile()/loadfile() for the quest loading chain, eliminating package.searchpath overhead entirely - Remove the custom package.searchers hook that was needed for the require-based approach https://claude.ai/code/session_01HUV9qCDaWCyvugmnzL9Qk1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRefactors quest loading to use direct file execution (dofile/loadfile) instead of Lua's require(), adds explicit catalogDirectory paths, mutates package.path/package.cpath handling, and removes a custom package.searchers-based catalog loader, with no public API surface changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
data/core.lua (1)
9-11: Makepackage.pathextension idempotent.Line 10 always appends, so reloading
core.luaduplicates entries and growspackage.path.Diff suggestion
local sep = package.config:sub(1, 1) -package.path = package.path .. ";" .. CORE_DIRECTORY .. sep .. "libs" .. sep .. "?.lua" .. ";" .. CORE_DIRECTORY .. sep .. "libs" .. sep .. "?" .. sep .. "init.lua" +local libsLua = CORE_DIRECTORY .. sep .. "libs" .. sep .. "?.lua" +local libsInit = CORE_DIRECTORY .. sep .. "libs" .. sep .. "?" .. sep .. "init.lua" +if not package.path:find(libsLua, 1, true) then + package.path = table.concat({ package.path, libsLua, libsInit }, ";") +end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/core.lua` around lines 9 - 11, The code unconditionally appends entries to package.path causing duplicates on reload; update core.lua to make the extension idempotent by computing the two candidate entries using CORE_DIRECTORY and sep (the same values used now) and only append each entry to package.path if package.path:find(entry, 1, true) is nil; adjust the logic around sep/CORE_DIRECTORY/package.path so package.path is modified only when those exact entries are missing.data/lib/core/quests/catalog.lua (1)
36-45: loadfile approach with fallback is well-implemented.Good error handling for
loadfilefailures. The fallback torequirewhencatalogDirectoryis falsy maintains backward compatibility.One optional consideration: if
loader()throws a runtime error during chunk execution (line 42), the error message won't include the module name context. This is minor since the stack trace will point to the file, but you could wrap it for consistency with the loadfile error format:♻️ Optional: Add context to runtime errors
if not loader then error(string.format("Quest module %s failed to load: %s", moduleName, errMsg)) end - quest = loader() + local ok, result = pcall(loader) + if not ok then + error(string.format("Quest module %s failed to execute: %s", moduleName, result)) + end + quest = result else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/lib/core/quests/catalog.lua` around lines 36 - 45, The runtime invocation of loader() can raise an error without module context; wrap the chunk execution (the call to loader() that assigns quest) in a protected call (e.g., pcall or xpcall) to catch runtime errors and rethrow or error with a formatted message that includes moduleName (same style as the existing loadfile error) and optionally the traceback; update the branch where loader is used so any error from loader() is reported as "Quest module <moduleName> failed to execute: <err>" while still returning/assigning quest on success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@data/core.lua`:
- Around line 9-11: The code unconditionally appends entries to package.path
causing duplicates on reload; update core.lua to make the extension idempotent
by computing the two candidate entries using CORE_DIRECTORY and sep (the same
values used now) and only append each entry to package.path if
package.path:find(entry, 1, true) is nil; adjust the logic around
sep/CORE_DIRECTORY/package.path so package.path is modified only when those
exact entries are missing.
In `@data/lib/core/quests/catalog.lua`:
- Around line 36-45: The runtime invocation of loader() can raise an error
without module context; wrap the chunk execution (the call to loader() that
assigns quest) in a protected call (e.g., pcall or xpcall) to catch runtime
errors and rethrow or error with a formatted message that includes moduleName
(same style as the existing loadfile error) and optionally the traceback; update
the branch where loader is used so any error from loader() is reported as "Quest
module <moduleName> failed to execute: <err>" while still returning/assigning
quest on success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1723fe80-b6f3-4bee-87d5-cc77b772cc26
📒 Files selected for processing (11)
data-canary/lib/core/load.luadata-canary/lib/core/quests.luadata-canary/lib/core/quests/catalog/init.luadata-otservbr-global/lib/core/load.luadata-otservbr-global/lib/core/quests.luadata-otservbr-global/lib/core/quests/catalog/init.luadata/core.luadata/lib/core/quests/catalog.luadata/lib/core/quests/loader.luadata/libs/functions/quests.luasrc/lua/functions/lua_functions_loader.cpp
Contributor
|
✅ Updated vcpkg baseline to 2026.03.18 ( |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LuaJIT runs in interpreter-only mode on macOS ARM64 (JIT disabled).
The require() function's package.searchpath iterates through every
entry in package.path doing string.sub operations for each call.
With 50+ nested require calls during quest catalog loading, the
cumulative cost causes the server to hang for 10+ minutes at startup.
Three changes to fix this:
system/env LUA_PATH entries; extend it in core.lua with only the
needed libs directory
chain, eliminating package.searchpath overhead entirely
require-based approach
Summary by CodeRabbit
Refactor
Chores