Jump to content

Itms

WFG Retired
  • Posts

    2.329
  • Joined

  • Last visited

  • Days Won

    104

Posts posted by Itms

  1. Removal of ObjectToIDMap

    In source/scriptinterface/third_party/ObjectToIDMap.h, we have some code, taken from Firefox, allowing us to keep a map where JavaScript objects are the keys, and the values can be anything (integers or strings in our case). This code was upgraded in Firefox 45 and the old version, which I had forgotten to update, stops working with SpiderMonkey 52.

    However, I discovered recently that the new code doesn't work either! SpiderMonkey developers tell me it is never actually used in Firefox. In fact, this code is not needed. If we have an object to which we want to attach an ID, we can use Symbol properties. JS symbols act as unique property keys that allow us to safely assign an ID to an object. Additionally, they are ignored when iterating over an object's properties, so the rest of the code won't know about them.

    f1fa7a1 (up for review at D2897 in Phabricator) In the AI manager, we used object maps in some code about "serializable prototypes" which is completely dead: no code calls into it. So I removed it. @wraitii has written D2746 which actually implements prototype serialization, but he tells me he would rather rebase his work after the upgrade.

    e9f2161 : I replaced the other use of object maps, in the serialization code, with symbol properties.

    727f26c : This allowed me to remove that code entirely. This means less external code to maintain!

    Sometimes what we actually need is a map with custom keys, and JS objects as values. Currently, we do that using std::map<KeyType,JS::Heap<JSObject*>>, but I believe we should use JS::GCHashMap. I'll look into that in the future.

    Other cleanups

    43bb449 : Previously, in SM, objects rooted on the stack did not have a default constructor, so we had created our own class wrapping them with such a default constructor. Nowadays, this wrapper is not needed anymore, so I removed its remaining use and deleted the class. Also less code to maintain.

    58ba02e : I performed an optional cleanup that would make the refactoring below easier.

    c148184 : We have a ScriptInterface::IsExceptionPending function that just calls a SpiderMonkey function with an unneeded GC request, so I removed it. TODO: I did that under SM52 and I don't know if it works under SM45. If it does, it should be committed before the upgrade.

    Refactoring of runtimes, contexts, and compartments

    :excl: This is the main part of the upgrade :excl:

    Until SM45, there were three levels of containers in the SpiderMonkey API. Each thread of the engine needed to have a runtime to execute some JavaScript. In each runtime, there could be several isolated contexts. In our case, we had one context for the simulation, another one for each page of the GUI, etc. The isolation of contexts allows us to separate properly the different parts of our code. Inside contexts, there could be several isolated compartments, but we didn't use that. Each context had its own and only compartment.

    In our code, JSContext is abstracted by our ScriptInterface class. We also have ScriptRuntime for JSRuntime.

    In SM52, the runtime and context levels are merged. Runtimes are removed from the API. There must be one context per thread, and isolation of the different parts of a thread must be done using different compartments. This is summarized in the following picture made by @Bellaz89:

    JSContext role changes

    The difficulty here is that calls to the SpiderMonkey API still use a context. Previously, it was enough to get the context from the script interface, and call SM: the target code would be in the correct compartment. Now, getting the context is not enough: we first need to make the context enter the compartment of the script interface, and only then, we can call the SM API on the context.

    In order to prevent mistakes, I decided to perform a big change. I think it's better to prevent getting the context from a script interface directly. I wrote a Request struct which allows one to get the context from a script interface, and which automatically enters and leave the associated compartment.

    In the past, we had many issues with JSAutoRequest, which is needed to properly handle the memory of the JS runtimes. In a lot of cases, we forgot to add JSAutoRequest, which created bugs. The new Request object allows us to prevent these mistakes, because it automatically opens a memory request as well.

    The changes go like this:

    c65093e and 25dc831 : Prevent getting the context, except through a Request that acts just like JSAutoRequest. I made two commits for readability but they should be committed together. This change removes quite some boilerplate code previously caused by JSAutoRequest.

    5d4e4c4 : Rename "context private" to "compartment private" (this commit is huge but changes nothing, just the name of classes and variables).

    8142e29 : Here I make the central change: each ScriptRuntime now has one single context, whereas each ScriptInterface is associated with a compartment. Getting the context through a Request automatically enters the correct compartment. The Request also provides the global object associated with the compartment. It is still possible to get the general context but this is now advertised as unsafe.

    521fd88 : There is a check made on the creation/destruction of runtimes, I change it to be made on the creation/destruction of contexts.

    17ce48e : Rename ScriptRuntime to ScriptContext to match its new role (this commit is big but changes nothing, just the name of classes and variables).

    0 A.D. is now ready to use SM52!

    Actual SM upgrade

    be5b0ed and 3013473 : In two commits, I upgrade the SM52 files, including a prebuilt version for Windows, and I adapt the build and premake scripts to use the new version.

    Simple API changes

    A lot of commits are made to match the API changes. They are very simple. I will merge them as a single commit in the future, along with links to SpiderMonkey's bugtracker. Right now I keep them separate so that I have a list.

    Some of them deserve a note:

    c4fc364 : In this one I remove all uses of runtimes, which become "unsafe contexts" with no compartment barrier.

    8c6476a : In this one I use the new API for structured clones. Those take a new argument which is a scope, depending on whether the clone will be used in the same thread, or even in the same run of the application. My choices of scopes have to be reviewed.

    Changes in the error reporting API

    cc31596: The heaviest API change is due to the error reporting made differently.

    Previously, both errors and warnings in JS code would make SM call the error/warning reporter, which we implemented ourselves. Now, only the warning reporter can be supplied. Errors are always handled by the exception system, in which all exceptions have to be caught. Sometimes, they are caught by the JS code itself, but sometimes they are not. In this case, the engine has to catch them and display them. If they stay uncaught, the engine can crash when a new exception is raised, or when performing a core operation such as creating a new compartment.

    Thus, I had to refactor a big part of our JS error handling. I think I have improved the design which was a bit hacky, but my code is not foolproof, as uncaught exceptions are maybe still possible.

    • Like 3
    • Thanks 4
  2. Hi everyone :) In this thread, I am going to present the SpiderMonkey upgrade work I have dedicated myself to since around a month. The aim of the thread is to explain what this upgrade is, why it is such a big change, and how I organized it. Please don't hesitate if you have comments or questions on anything!

    What is SpiderMonkey?

    SpiderMonkey is Mozilla's JavaScript engine, used first and foremost in Firefox. As you probably know, 0 A.D. is written in C++ but a large part of the gameplay is programmed in JavaScript, in order to be easily modifiable by mods. In order to execute the JavaScript code of the game, we include SpiderMonkey inside 0 A.D. (this is also called embedding). We are not the only ones to embed SpiderMonkey: for instance, GNOME (a well-known desktop environment) also embeds SpiderMonkey.

    I will often use the acronym SM to designate SpiderMonkey. No kinks here, just laziness to type! :ph34r:

    All about upgrading

    The advantage of embedding SM is that we don't have to take care of bugs or complicated JavaScript interpretation quirks: the folks at Mozilla have us covered. The downside, however, is that we are often out of the loop on the evolution of SpiderMonkey. In order to get the improvements of the engine, we have to upgrade it to recent versions, and this is very complicated. A big part of 0 A.D. is built upon SpiderMonkey, so when the latter changes, we have a lot of work to do to adapt.

    On top of this, SM upgrades are big changes. SM evolves along with Firefox (which has a new release every 8 weeks), but a stand-alone SM version is only released for embedders approximately once a year, when Firefox releases an Extended Support Release (a kind of Firefox LTS, which is, for instance, the one available on Debian). See the Firefox release calendar.

    The difficulty of upgrading is that, between two ESR releases, SM accumulates a lot of changes, and, whereas Firefox is always on top of them, other front-ends like 0 A.D. are faced with numerous breakages which could create bugs. Additionally, at 0 A.D. we haven't upgraded SM for a long time due to the difficulty of upgrading. Thus, it is also difficult to find information to upgrade to SM releases that are already outdated. It is not much easier to jump to the current SM release: so many changes have happened that we would be unable to make sense of them all at once.

    Current status

    I upgraded SM to version 38 in the ends of 2016. At that point, we were mostly up-to-date: version 45 was already out but it was still getting some fixes, and the stand-alone SM45 was not officially released. Nowadays, stand-alone SM versions are almost never officially released, however the SM development team has started to put out more and more news (see their brand new website) so things are certainly getting better.

    I couldn't work on the SM45 upgrade until the summer of 2019, and even then I left a few bugs in, that we will discover in the next posts :blush:

    I am releasing today the upgrade to SM52, which is a big one (hence the forum thread). That brings us to "only" two years late, as SM52 stopped receiving fixes in June of 2018.

    It is now necessary to work on the upgrade to SM60, SM68, and SM78 (the latter is still getting fixes until the beginning of 2021). However, I am going to work on other urgent things in August. I need to improve a lot of things with the CI, Jenkins and the handling of patches, and I would like to allow contributors to use git instead of SVN. I hope these exciting news will make you forgive me for delaying the SM upgrades ;) In any case, I want to get back to further SM work as soon as possible.

    In this other thread, @Bellaz89 did the daunting work of making SM68 work with no previous knowledge of 0 A.D.! :o Many of you probably saw it. Bellaz was then very helpful and answered all my questions concerning the upgrade. His spadework, which cannot be committed just as-is, made the actual upgrade a breeze; and apparently there will be no big work to perform on 0 A.D. between SM52 and SM68, apart from an encoding change. We will see...

    Overview of the upgrade

    The upgrade can be found at this git branch. It is very big. Not counting the changes to SpiderMonkey itself, in the engine of 0 A.D., it consists of 145 files changed, with 2667 insertions(+) and 3243 deletions(-)! The objective is to split the massive change into independent parts, which can be understood, tested and committed one by one.

    I tried to make a majority of changes before the actual upgrade. That way, the upgrade itself is leaner. There are a few changes that I will be able to do just after the upgrade. Right now they are not uploaded, but I will announce them here later. I will also keep the posts below updated whenever I update or commit a part of the upgrade.

    You are probably wondering about the performance. From my few tests, the performance is identical in SM45 and SM52. If I'm not mistaken, the structural changes in SM52 pave the way for performance improvements in the next SM versions. On top of this, I have written a patch for a huge performance bottleneck (D2919) that was very apparent under SM52, and I have identified a couple of easy improvements that I will list in a ticket soon. Finally, Bellaz thinks that we have a few JIT optimizations (i.e. whether JavaScript code should be compiled, which costs some time, but makes it faster) which have become counterproductive. We could try and tweak our JIT options and see if we can improve the performance that way.

    You can already checkout the branch and test it! :)

    Now, to the details for the curious programmers!

    • Like 5
    • Thanks 4
  3. Hello, indeed I was AFK for a few days. Atlas is not automatically built, but one can manually start a build of Atlas on Jenkins. I did just that, and it will be committed in a few minutes once the build finishes.

    Any team member with a Jenkins account can do it. Right now you don't have an account Angen, but I'm sending you a PM to remedy that. Else, as far as Atlas devs are concerned, trompetin17 and wraitii have an access, even if they don't use it often due to the low activity on Atlas recently. ;)

    • Thanks 2
  4. Hi @LeineX! I think you are missing the Windows 7.1 SDK, which was proposed in VS 2017 but apparently not in VS 2019. This SDK includes the gl.h file you're missing, among others.

    You can download it and install it from here: https://www.microsoft.com/en-us/download/details.aspx?id=8279

    Let us know if it works. If it does we'll add this piece of information to the build instructions :)

  5. Hi @Bellaz89, nice to meet you! I have seen your work on SM in my emails, but as you've probably been told I was unavailable because of my PhD. I will be back to programming on 0 A.D. next week, and I will start with the SM 45->52 migration. I have already prepared this migration in a way that will probably conflict with yours, so I apologize in advance.

    When I'm back I propose that we schedule a chat together (IRC or elsewhere) to discuss how I plan to do the migration to 52 and whether you could help, and after that migration, I would be happy to help you include your work in the game by targeting SM 60, 68 and probably 78 at that point! To answer the question above, yes we plan to update incrementally every time.

    Thanks for your work and see you soon (y)

    • Like 3
    • Thanks 1
    • Sad 2
  6. On 2/22/2020 at 11:20 AM, Nescio said:

    However, I looked at a couple of mods hosted via mod.io (autociv, borg-expansion-pack, no-blood-and-gore-mod as well as jp-lang, zh-lang, zh-tw-lang) and none of those had licence or copyright files, which makes me wonder why it is suddenly required in this case.

    Oh, it's not required, I'm just giving you a suggestion here :) and the language packs do include the license of packed fonts, which is the reason why I came up with the suggestion for you. However, and I'm discovering this only now, I forgot to put the license files in the a23b releases of the mods :blush: they were in a22 and a23 versions. Be assured that I will keep adding them when I don't forget, and that your mod won't set a precedent.

    On 2/22/2020 at 11:27 AM, Nescio said:

    Another question, is it possible to make the fontbuilder.py script use font features?

    The script directly calls libcairo's API, so the answer is probably yes, even though I don't know much about that API.

    I also really like the "unusual" but better perispomene accent featured on the bottom line (y) 

  7. Yeah it is necessary as the fnt and png files are derived work from the font itself. You are right that the FreeSerif license is already covered by the vanilla game (but it can't hurt to say it again in the mod); and even though Libertine and Biolinium are covered by the same license it's not written anywhere yet.

    In any case adding the license information to your mod is not going to cost you anything ;)

    • Like 1
  8. 1 hour ago, Andrettin said:

    I tried using the latest version of mozilla-build and NSPR, but to no avail

    That is the issue: in order to build SM45, you need the mozilla-build from 2016 (and I also use NSPR from back then as a security, but it might work with current NSPR, never tried). The mozilla-build from 2016 will not allow you to build with VS2017, the only options will be VS2013 and VS2015.

    • Like 1
  9. We are lagging behind SpiderMonkey and the version we use (SpiderMonkey 45) did not support VS2017, as it was released in 2016. (Actually, the code is probably buildable with VS2017, thanks to backwards compatibility, but Mozilla's build system from 2016, needed to build version 45, did not know how to use VS2017 tools)

    Upgrading SM is one of my highest priorities, but it's a lot of work. From SM52 on, VS 2017 will be supported and we'll enable support for it. (and maybe 2019 will work too, since Mozilla's build system hasn't changed significantly between 2017 and now) :)

    • Like 2
  10. @Nescio The mod looks good, I think you should include the license file for Linux Libertine and FreeSerif. You can place it next to the mod.json. (we place the license along with the font files in the fontbuilder folder, which you can't do with a mod)

    Please upload a new version of the mod, instead of trying to replace the current 5.3.0. The latter usually fails because of the way mod.io caches files. ;)

  11. Here is a patch: https://code.wildfiregames.com/D2244

    To apply it, you should use arcanist. If you are on Debian buster or later, just install it from the repository. Else follow these instructions: quick start.

    Then, you can create an account on Phabricator (that will allow you to comment directly on the patch, to be pinged if we need some information, etc).

    Then, in your terminal:

    arc install-certificate

    in order to use arcanist with your account, and then

    arc patch D2244

    in order to apply the patch. You can then build with

    ./update-workspaces.sh --with-system-nvtt
    cd gcc
    make

    And please report here or on the patch directly if you discover other bugs :)

    Thanks for the report, and in advance for testing and playing!

  12. Hi! You seem to have hit this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1266366

    What is needed is to patch spidermonkey with it, and when it's committed, then you will be able to build SVN like you tried to do, with

    On 8/22/2019 at 4:25 AM, Dansasbu said:
    
    ./update-workspaces.sh --with-system-nvtt

    Ideally we would have downloaded the latest bundle from the esr branch, instead of the release candidate from the SpiderMonkey devs, but they are not available anymore for SM45. So it would be better if you could test the patch that I will make, in case we need to patch more bugs. Are you comfortable with using our patch review platform? Else I'll provide the patch here and will give you instructions to test it.

    Thanks for the report :)

    • Like 1
    • Thanks 1
×
×
  • Create New...