Skip to content
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

Patched luajit compile with gc64 flag to fix 2GB address limitation t… #118

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rfortier
Copy link

…hat is hit on large builds, or modest builds with Skyrim Together Reborn

…hat is hit on large builds, or modest builds with Skyrim Together Reborn
@rfortier
Copy link
Author

rfortier commented Jul 28, 2024

The PR finishes the conversion of JContainers64.dll to the x64 address model. This entails adding "gc64" to the build instructions for luajit to remove the restriction that luajit could only allocate memory from the first 2GB of the virtual address space.

64-bit address space support is considered beta in the current luajit version 2.1.0-beta3. Might want more than a patch bump to the version, or even a beta release first . I've seen zero issues is testing, but JContainers is a heavily used mod. A test build for Skyrim SE 1.6.1170 is available on the author's release page. Please try it out and post any issues here or as an issue on my page.

The fix entails 2 parts. The first is a change to lua_module.cpp to check for the failure of luaL_newstate() and retry every second until it works. Without this check, when no memory below 2GB is available on large builds, you get a CTD from reopen_if_closed() when loading a save. With this fix it won't crash, but it can take several minutes before a big enough chunk of memory is available below 2GB to continue.

You could still encounter a crash later on other allocations that aren't protected, though.

The complete fix involved compiling luajit with the gc64 flag to remove the restriction that allocated memory had to be below the first 2GB.

The problem is particularly bad if you use the Skyrim Together Reborn mod, which changes the memory layout to load it's own logic. A modest build with STR, JContainers, AH Hotkeys (requires JContainers) and a big inventory is enough to provoke the problem (which is why I hunted it down and fixed it).

@ryobg
Copy link
Owner

ryobg commented Jul 30, 2024

Thanks for the proposal. I have no intention to make a release just for that for now. Please, do test meanwhile.

  • waiting for several minutes to proceed does not sound good at all
  • personally, I would prefer to have some kind of time limit on that waiting

@rfortier
Copy link
Author

rfortier commented Jul 30, 2024 via email

@ryobg
Copy link
Owner

ryobg commented Jul 30, 2024

would you mind if I linked my "beta" build in the PR comments?

Nope, go ahead.

if a timeout is added to the wait loop, the game will just crash immediately.

I meant something along the lines, that loop to have an end condition. Currently it is wait forever until it is done.

@rfortier
Copy link
Author

rfortier commented Jul 30, 2024

Thanks.

There are a couple of options:

  1. With gc64 enabled, it should never happen. So just log a message and let it crash if it does.
  2. Keep the loop and an option to compile WITHOUT gc64, but log a message when the wait starts, wait no more than ~3minutes. If it times out log a message and let it crash.

Or pick a shorter timeout, but something seems to happen at about the 2:30 mark. I don't know if base skyrim frees a chunk of memory or it's one of my mods, but that's how long it takes to not crash with the 2GB memory model.

@rfortier
Copy link
Author

As requested, updated the "wait for memory" loop to timeout. Also added explicit logging to the JContainers64.log to tell people it is waiting, why it is waiting, and why it crashes when it times out.

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.

2 participants