Opened 10 years ago

Last modified 9 years ago

#2475 closed task

Decide on using new ES6 keyed collections (e.g. Map, Set) — at Version 3

Reported by: Yves Owned by: Yves
Priority: Should Have Milestone: Alpha 18
Component: Core engine Keywords:
Cc: Patch:

Description (last modified by Yves)

I'd like to have a team decision if we start using the new keyed collections which are available with SpiderMonkey 24 and later. These features are marked "experimental".

Overview

Mozilla writes this:

This is an experimental technology, part of the Harmony (ECMAScript 6) proposal. Because this technology's specification has not stabilized, check the compatibility table for >usage in various browsers. Also note that the syntax and behavior of an experimental technology is subject to change in future version of browsers as the spec changes.

Check the descriptions of these objects and also the browser compatibility list at the end of the articles:

Why should we use them?

It's mainly for performance reasons.

Performance example

Because I don't like generic benchmarks, here's an example of a test with our code I made today.

It's a non-visual replay of a 2vs2 AI game with aegis bots on Oasis 04 going over around 15000 turns. I've measured the total runtime in seconds of the get function in entity.js. I've used my v31 testing branch for the measurements but I don't know if it makes a difference compared to v24. The function gets called 51'206'230 times in total and the hash is the same with and without patch. The patch is attached. It's very simple and we could improve performance even further if we used it at some more key locations that use key/value pairs.

Before:
Time in seconds for get function:                      43.91533856391563
Total execution time  (some variantion is normal):    995 seconds

Get taking 4.4% of total execution time

After (2nd version of the patch):
Time in seconds for get function:                     21.613286443973905
Total execution time (some variation is normal):     987 seconds

Get taking 2.19% of total execution time

Change History (5)

by Yves, 10 years ago

Attachment: map_replacement.diff added

performance testpatch (can be committed if the decision is positive)

comment:1 by Yves, 10 years ago

My personal opinion is that we should use these. I don't think they are going to change completely before the specification is finalized and as you see in this patch it's quite simple to make the change (or change it back again).

by Yves, 10 years ago

Attachment: map_replacement_v2.diff added

second patch, now also using Map for the aura stuff

comment:2 by Yves, 10 years ago

Description: modified (diff)

comment:3 by Yves, 10 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.