Opened 11 years ago
Last modified 7 years ago
#1886 closed enhancement
Upgrade Spidermonkey — at Version 43
Reported by: | Yves | Owned by: | Yves |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 16 |
Component: | Core engine | Keywords: | spidermonkey javascript debugger |
Cc: | javierjc1982@… | Patch: |
Description (last modified by )
Tickets related to the Spidermonkey upgrade:
#2126
#2137
#2140
#2172
#2212
#2213
#2241
#2267
#2322
#2348
#2372
#2416
#2434
#2442
Related bugs in Spidermonkey:
Meta-Bug for performance bugs affecting 0 A.D.: Bug 897962
Overview
We are currently using Spidermonkey 1.8.5. That's a very old version and we need to upgrade to a newer version for various reasons. Unfortunately that's not very easy because the API changes quite often.
Also check the forum thread for the current status.
Reasons for upgrading:
- Switching earlier avoids writing too much code for the older API that has to be changed later
- Only when using a supported version it's possible to fix Spidermonkey security issues (I know that 0 A.D. itself most likely has more security issues at the moment)
- Learning how the new API works with multi-threading is important for our AI multi-threading design.
- Performance: During the work on upgrading we figured out that performance currently isn't much better in the new version compared to v1.8.5. The huge improvements from benchmarks don't translate to most real world applications. Loading random maps is a lot faster, but that's the only thing at the moment. It's still possible that the performance will improve once the most important bugs in Ion Monkey and Baseline are fixed.
- If we find bugs in v1.8.5 we will never get support from Mozilla. If we use the current version or if the same bug also affects the current version, getting help is much more likely.
- Getting new features into the official version of Spidermonkey is also only possible when we use the current version.
- Many Linux distributions will use the new ESR versions of the standalone library. It's better to use that library provided by the system instead of bundling our own version. v1.8.5 won't be supported by Linux distributions forever.
Version
Mozilla plans to release standalone versions of Spidermonkey for each ESR-release. We are going to upgrade to ESR24 first.
- mozjs-24 release discussion: mozilla bug 919277
JIT
The Just In Time compiler creates optimized bytecode or even native assembler code for functions or loops that are called often. That's something different than the compiling which translates the scripts to bytecode in the first step.
It's currently not possible to JIT-compile the scripts before they are executed or even store them in JIT-compiled form on the disc.
Using js::NotifyAnimationActivity it's possible to prevent the garbage collection from collecting generated JIT-code (and forcing recompilation). Unfortunately this only works if js::NotifyAnimationActivity is called at least once a second. With big lag-spikes, especially in debug mode, this can't be guaranteed at the moment. At the moment I'm working around this by modifying Spidermonkey's Runtime.cpp directly. I'll have to find a better solution.
Garbage collection
Garbage collection and memory allocation is a major bottleneck for certain parts of JS code that update a lot of data very often. I made some measurements here: http://www.wildfiregames.com/forum/index.php?showtopic=16721&#entry262183
Garbage collection could be improved by improving the way we handle compartments, especially related to the AI. That's a bit vague but I'm still trying to figure out more about that. An interesting article: http://andreasgal.com/2010/10/13/compartments/ . That article is quite old, but I think the basic concepts explained there are still valid.
Garbage collection can now even be done in a separate thread or we could run it more often (each frame for example) to avoid lag-spikes when it happens. For the AI that problem would probably be solved anyway when we move it to a separate thread.
Change History (49)
comment:1 by , 11 years ago
Status: | new → assigned |
---|
comment:2 by , 11 years ago
by , 11 years ago
Attachment: | CGameView_without_CJSObject_1.diff added |
---|
just for reference and discussion in the associate thread
by , 11 years ago
Attachment: | CGameView_without_CJSObject_2.diff added |
---|
just for reference and discussion in the associate thread
comment:3 by , 11 years ago
Keywords: | spidermonkey javascript debugger added |
---|
comment:5 by , 11 years ago
Keywords: | spidermonkey, javascript, debugger → spidermonkey javascript debugger |
---|
r13429 ref'd this ticket because I had to use some hacks to get around AIs sharing data between script contexts. Actually the solution could be much simpler if each AI's data was in a single script context (then ScriptInterface could be used to hold the serializable prototypes, and the implementation wouldn't be AI specific).
Also a heads up since probably these changes will need merging into your local repo, feel free to ask if anything causes trouble.
comment:6 by , 11 years ago
Milestone: | Alpha 14 → Backlog |
---|
comment:7 by , 11 years ago
Description: | modified (diff) |
---|
comment:9 by , 11 years ago
Description: | modified (diff) |
---|
comment:11 by , 11 years ago
Description: | modified (diff) |
---|
comment:13 by , 11 years ago
Description: | modified (diff) |
---|
comment:14 by , 11 years ago
Description: | modified (diff) |
---|
comment:15 by , 11 years ago
Description: | modified (diff) |
---|
comment:16 by , 11 years ago
Description: | modified (diff) |
---|
comment:17 by , 11 years ago
Description: | modified (diff) |
---|
comment:19 by , 11 years ago
Description: | modified (diff) |
---|
comment:21 by , 11 years ago
Description: | modified (diff) |
---|
comment:23 by , 11 years ago
Description: | modified (diff) |
---|
comment:24 by , 10 years ago
Description: | modified (diff) |
---|
comment:26 by , 10 years ago
Description: | modified (diff) |
---|
comment:29 by , 10 years ago
Description: | modified (diff) |
---|
by , 10 years ago
Attachment: | patch_26_v0.7.diff added |
---|
Current patch with SpiderMonkey version 145669:aa9ec17cf912 (http://hg.mozilla.org/mozilla-central) and 0 A.D. version r14528 . This is still heavily WIP. There's a description how to apply it here: http://www.wildfiregames.com/forum/index.php?showtopic=17289&st=80#entry271330
comment:31 by , 10 years ago
Command to find missing var statements (probably not perfect but it worked and I need a place to write it down): grep --perl-regexp -R '(for\s?\()(?!var|\svar)' --exclude=\*{~,svn-base}
comment:32 by , 10 years ago
Description: | modified (diff) |
---|
by , 10 years ago
Attachment: | SpiderMonkeyESR24_v0.8.diff added |
---|
The current WIP patch for ESR24 (download it here and copy it to libraries/source/spidermonkey: https://ftp.mozilla.org/pub/mozilla.org/js/mozjs-24.2.0.tar.bz2)
follow-ups: 34 37 comment:33 by , 10 years ago
A couple of suggestions:
- rather than hardcoding -j5 you could use -j
nproc
(nproc inside back ticks which the trac is omitting here), and fall back to 2 if nproc is not available; - the build script could do a wget for mozjs24 tar.gz, rather than having it in the svn, reducing svn size and making easy to upgrade to, example, a .0.1 version.
comment:34 by , 10 years ago
Replying to fabio:
- the build script could do a wget for mozjs24 tar.gz, rather than having it in the svn, reducing svn size and making easy to upgrade to, example, a .0.1 version.
I don't agree with this. I'd like to be able to build the game when being offline too.
comment:35 by , 10 years ago
Milestone: | Backlog → Alpha 16 |
---|
by , 10 years ago
Attachment: | SpiderMonkeyESR24_v0.9.diff added |
---|
Current WIP patch for ESR24. This version is with incremental GC. It's still WIP and there are still some known errors.
comment:36 by , 10 years ago
@Fabio Thanks for the input. I'll try to set the -j option dynamically.
I agree with Sander and would rather bundle it at the moment. I remember some discussions about scripts for getting platform-specific dependencies. It could be worth developing something, but it should be a general solution for all libraries and dependencies.
Btw. sorry for answering late. E-Mail notifications still don't work for me and I didn't notice your comment in the logs.
comment:37 by , 10 years ago
Replying to fabio:
A couple of suggestions:
- rather than hardcoding -j5 you could use -j
nproc
(nproc inside back ticks which the trac is omitting here), and fall back to 2 if nproc is not available;
Just pass -jsomething to update-workspaces.sh and the spidermonkey build.sh script will use the same value. (I'd probably leave the default at -j2 though, or change it to -j1)
comment:38 by , 10 years ago
Description: | modified (diff) |
---|
comment:41 by , 10 years ago
Description: | modified (diff) |
---|
by , 10 years ago
Attachment: | SpiderMonkeyESR24_v0.11.diff added |
---|
Tested on r14775. Remaining todos: Test on Windows, Test on Mac OS X, Fix related tickets linked in the description, Review.
comment:42 by , 10 years ago
Description: | modified (diff) |
---|
comment:43 by , 10 years ago
Description: | modified (diff) |
---|
I asked about the JIT topic and Ion-Monkey in the jsapi IRC channel. Logs here.
Summary: JIT compiled code can't be saved on the harddisk and used later because it contains memory addresses. However, a function should be compiled with Ion-Monkey when it has been run 1000 times, so everything that runs once per frame should be compiled after about 30 seconds with 30-35 fps, which should be fine for the moment.
Garbage collection occasionally throws away the JIT-ed code. To make sure this doesn't happen, calling "js::NotifyAnimationActivity" should be enough. Check Bug 753630 for more background information about that.