Troubleshoot Spidermonkey JIT

[historic_bruno]

I've been porting the random map generation code over to JavaScript, and per Philip's suggestion I tried adding the JSOPTION_JIT flag for Spidermonkey, to enable the just-in-time compiler. Getting this to work is critical if map generation will be written in JS, and given how the same code performs in Firefox 3.6 and FF 4 beta, it seems a very reasonable plan. Without JIT, the code is very slow, which is not surprising since the map generation is heavily dependent on loops to check distance between points and evaluate noise maps.

Now I'm not sure if we have an outdated/broken Spidermonkey implementation and need to upgrade per ​#622, which we may have to do anyway, or it may be that enabling JIT is a bit more complex than adding that flag. Whatever the case, here's a patch to add a random map system in JS to troubleshoot the cause of this.

The above description does not apply now, as I was compiling in Debug mode rather than Release mode, which disabled JIT. However we still need to troubleshoot some issues in JIT behavior especially if Mozilla will only be supporting SSE2 x86 hardware in FF4 (see ​their wiki), and so this ticket remains very relevant.

[historic_bruno]

For experimental use

[Philip Taylor]

With new SpiderMonkey (not tested old version), in Release mode (not tested Debug), the tracing JIT works and reduces runtime; the method JIT also works and reduces runtime more. So I think it's important to fix #622 for performance, and the RMS code is already fine in its JIT interaction.

Adding a patch that does a few things:

• Fixes 64-bit compile error (need FromJSVal<u64> to handle size_ts)
• Fixes GCC syntax error on string constants (I don't remember the correct syntax so I changed it to a way that uses fewer distinct string constants, which is probably better anyway)
• Fixes 'reference to undefined property' warnings
• Adds timer to GenerateMap
• Adds autostart mode (launch with -autostart=latium.js -autostart-random)

[historic_bruno]

Added new patch for testing new SpiderMonkey's JIT, also it includes fixes from Philip's patch.

[historic_bruno]

To keep track of some patches I've applied to fix JIT/SpiderMonkey for random map implementation:

​#580515: TM: LIR_cmovd mishandled with X86_FORCE_SSE2=no

The JIT generates incorrect floating-point native code on non-SSE2 x86 platforms (FloatArrays return garbage data).

​#612334: have typed arrays treat length params more sanely

This bug prevents typed arrays initializing properly when they are given an integer length which JIT thinks is a double (argument error is thrown).

[Kieran P]

Bumping this to critical. As I understand it, JIT is important for the new RMS, as it speeds up the map generation a lot.

And since we want RMS in Alpha 4, I'd say this is fairly important.

I turned on JIT and compiled a few days ago, and everything seemed to work fine. Is there any remaining issues?

[Kieran P]

I've pushed RMS back to Alpha 5. It still needs work. So I'm pushing this back with it.

[historic_bruno]

Random map system committed (see #6). It will need testing on different platforms (especially older, non-SSE2 x86s). I've enabled all JIT options. Ticket #734 is relevant, as the latest Spidermonkey revisions should include the above patches when and if we choose to upgrade. I still haven't seen their decision on supporting non-SSE2 platforms in the future...

[historic_bruno]

I can test this on my old PC later. I'm not seeing any complaints about RMS not working, or any weirdness that might be due to JIT, but maybe nobody with non-SSE2 hardware has tested it.

[historic_bruno]

I am seeing one error even on a new computer, which pertains to ​https://bugzilla.mozilla.org/show_bug.cgi?id=612334. Starting the game in autostart mode with random maps will cause an error, because the map settings are stringified in order to pass to the map generator. The map size is not picked as an integer when the data is parsed, for whatever reason, and typed arrays don't like that. I've inserted a temporary hack using Math.floor() to force the map size to an integer. But it would be nice to include that patch at least before Alpha 5, and probably the other one as well, if not upgrading SM completely :)

[Philip Taylor]

This depends on #734 - would still be nice to fix that before A5 if possible, but I don't know if it's possible.

[historic_bruno]

Replying to Philip:

This depends on #734 - would still be nice to fix that before A5 if possible, but I don't know if it's possible.

If we can't get to #734 before A5, then I'd recommend applying the above patches to our current Spidermonkey revision, otherwise random maps will not be reliable.

[ben]

(In [9435]) Balances random map resources. Attempts to avoid "invalid argument" errors in typed arrays by forcing them to integer (see #658). Removes script preloading from map generator (VFS is thread-safe now) Removes thread checking from ScriptInterface file loading functions. Adjusts starting entities in civ data.

[ben]

(In [9725]) Adds hack for random map data saved as typed arrays (see #658)

[Philip Taylor]

Can this be closed now that #734 is done?

[historic_bruno]

Sure, and if there's any further issues we can make new tickets.