View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0004520||The Dark Mod||Saving/Loading||public||03.05.2017 03:32||06.08.2017 07:59|
|Priority||normal||Severity||minor||Reproducibility||have not tried|
|Target Version||TDM 2.06||Fixed in Version||TDM 2.06|
|Summary||0004520: Make savegames of x86 and x64 builds interchangeable|
|Description||It would be great if savegames of builds with different bitness were compatible to each other.|
It seems that dhewm3 decided to leave x86 and x64 saves incompatible.
The greatest problem here is how idProgram is working...
Ok, scripts were changed in dhewm3 fo 64-bit compatibility. As a result, some arguments have changed their size (e.g., int takes 64-bit in x64 simply because it is stored in union with pointer).
Right now several data containers are saved/loaded as byte arrays, and their byte-level representation is different in 32-bit and 64-bit modes.
In order to make sure that x64 version saves and loads exactly the same data as the 32-bit one, we have to:
1. Implement save + restore for idInterpreter::localstack.
2. Implement save + restore for idProgram::variables|variablesDefault.
3. Implement save + restore for idScriptObject::data.
4. Reimplement idProgram::CalculateChecksum so that it does not depend on variable indices (they change becausew argsize immediate values are different, so they are merged together in different way).
Maybe I have forgotten something =)
In my opinion, this is a lot of work. In the first 3 points data is stored as byte, so in order to understand what is stored there, information must be taken from other places (not clear from where).
Alternative way to support save/loads is to make sure all objects have same size in x64 version.
For this to happen, we have to revert most of the changes dhewm3 did to scripting (revert intptr_t back to int). After that we can replace pointers inside varEval_s union with Ptr32<T> wrappers, which store int inside them. Then we add getter and setter for them, each requiring caller to specify base pointer. The Ptr32 wrapper only stores 32-bit offset relative to the base pointer (which may be local stack, global variables). After that, we have to fix all compile errors: replace direct access to pointer fields with getter/setter calls.
This is still considerable work, and it is not clear whether it would be successful. If it would, savegames would be compatible between 32-bit and 64-bit automatically.
P.S. And of course there is an option to leave 32-bit and 64-bit savegames incompatible like dhewm3 did =)
Ok, I have tried to restore compatibility of scripts, and it turned out to be not so hard. Moreover, after fixing the problem with scripts, all the savegames immediately became 32/64-bit interchangeable =)
When dhewm3 first tackled the problem of running script in 64-bit mode, they had to fix two issues:
1. When event is called, all its arguments are put into "int args;" local array, which are later passed to C++ function callbacks. These arguments could be float, integer or pointer. Pointers do not fit into 32-bit int.
2. Sometimes interpreter puts pointers onto localstack (or other script storage). The only opcode which does this is OP_ADDRESS. Since type ev_pointer has size = sizeof(int), interpreter expects address to fit into 32-bit space.
The first problem was fixed by changing local arrays to "intptr_t data". It means that each argument is pointer-sized, which is exactly what x86 and x64 calling conventions expect from caller. These changes are perfectly OK, since they only affect temporary storage of event callback arguments.
The second problem was fixed by increasing sizes of all basic script types from sizeof(int) to sizeof(intptr_t). As a result, in x64 builds scripts have completely different layout of data in their memory. Moreover, even the programs are somewhat different, because all ev_argsize immediate values have changed. Since script storage is saved/restored "as is" (i.e. as a raw chunk of bytes), this change makes 32/64-bit savegames incompatible. This is a bad change which must be reverted.
Without increasing sizes of basic script types, everything works perfectly, except for the fact that interpreter tries to put 64-bit pointers into 32-bit storage prepared for ev_pointer variables by compiler.
To fix this problem, it is necessary to find a safe way to encode addresses stored by OP_ADDRESS opcodes by 32-bit values. All these pointers are either NULL, or point into the idScriptObject::data memory chunk.
The simplest fix is to create a special memory zone for all the memory allocations of idScriptObject::data. As long as the whole zone is less than 2 GB in size (which is obviously true), we can encode any pointer into it with 32-bit offset relative to its beginning (plus one to allow 0 = NULL). This only requires one addition of global variable to decode, and one subtraction of global variable to encode.
The special memory zone and encoding/decoding are implemented in idProgram::ScriptObjectMemory_* methods. The memory zone itself is located in idProgram singleton. Note that the recoding is performed only in 64-bit mode: in 32-bit mode address of pointer is stored "as is", and memory is allocated with Mem_Alloc, just as before.
The special memory allocator must serve all memory requests from within a single memory block without ever requesting for more memory.
Luckily, we can derive some upper bounds:
1. We know sizes of all script object types after compilation, so we can easily find the largest one. Let it have size = "maxSize"
2. Each script object is tied to an entity, and each entity is tied at most one script object. So the number of alive script objects never exceeds MAX_GENTITIES.
This means that maximal total amount of memory allocated for script objects cannot exceed (MAX_GENTITIES * maxSize). If we allocate a memory chunk twice as large as this number, then we can be sure that every memory request can be served, regardless of fragmentation.
Unfortunately, idlib has only one general-purpose allocator: idDynamicBlockAlloc. It seems that it can work with single memory chunk, but there are two reasons against it. First, it is too complicated, so it is hard to say anything exactly about its properties. Second, its block size is template argument, that's why we cannot make it proportional to maxSize at runtime.
Taking ready-to-use memory allocator from outside (like TLSF) is bad for the first reason too. Plus it means putting more dependencies for this minor issue.
As a result, I decided to write a small allocator myself specifically for this purpose: idEmbeddedAllocator. We allocate a single buffer large enough to surely hold MAX_GENTITIES allocations of size <= maxSize at any moment.
The allocator maintains an array of free memory blocks within it: offset and size for each block (sorted by offset). On allocation, the first suitable free block is taken. Size of allocation is stored in the first 4 bytes of the block. On deallocation, the block is inserted back into sorted list. Bookkeeping ensures that all free blocks are nonempty and that no two free blocks are neighbors.
Note that using arrays is not good in general, but: there cannot have more than 8192 elements at any moment, and entities are rarely created/destroyed. In fact, I have never noticed more than two free blocks in real practice, so this is not a problem. This also means that despite huge size of memory chunk (about 16 MB), only the first 0000038:0000010-100 KB are really used.
|Committed all changes in revisions 6878 and 6879.|
|03.05.2017 03:32||stgatilov||New Issue|
|03.05.2017 03:32||stgatilov||Tag Attached: x64|
|07.05.2017 14:02||stgatilov||Note Added: 0008848|
|08.05.2017 04:03||stgatilov||Note Added: 0008850|
|08.05.2017 04:12||stgatilov||Note Added: 0008851|
|09.05.2017 04:43||stgatilov||Assigned To||=> stgatilov|
|09.05.2017 04:43||stgatilov||Status||new => assigned|
|09.05.2017 09:26||stgatilov||Note Added: 0008853|
|09.05.2017 09:39||stgatilov||Note Added: 0008854|
|09.05.2017 09:40||stgatilov||Note Edited: 0008854||View Revisions|
|09.05.2017 10:05||stgatilov||Note Added: 0008855|
|09.05.2017 17:33||stgatilov||Note Added: 0008857|
|10.05.2017 02:24||stgatilov||Status||assigned => resolved|
|10.05.2017 02:24||stgatilov||Fixed in Version||=> TDM 2.06|
|10.05.2017 02:24||stgatilov||Resolution||open => fixed|
|10.05.2017 02:25||stgatilov||Target Version||=> TDM 2.06|
|28.06.2017 03:24||stgatilov||Relationship added||related to 0004549|
|06.08.2017 07:59||stgatilov||Relationship added||related to 0004598|