JavaScript Map

1

Hi,

every time I look at the generated js output I find the code for Map set and get very noisy. Shouldn’t it be possible to use:

// JavaScript
h = Object.create(null);

instead of:

h = {};

and test the reserved symbols with __map_reserved ?

the trick with __map_reserved shouldn’t be needed with an object without prototype, or am I missing something ?

2

There is still an issue with __proto__ in some environments, see [js] use Object.create(null) for StringMap when -D js-es5 · Issue #4372 · HaxeFoundation/haxe · GitHub.

We might want to revisit this, because those run-times are probably now long obsolete, and looking up __proto__ could be considered an very rare edge-case anyway.

Another topic is that StringMap.get/set could be made non-inline not to spam the JS output, but I think we also benchmarked that at some point and that caused actual slowdown. But again, maybe we should check this again.

3

thanks for link. Didn’t know about the __proto__ problem.
It is so weird we have -D js-es=5 and -D js-es=6 and the StringMap is still from the stone-age. At least for ES6 we should have a more elegant solution with JS Map.

4

This is yet another interesting discussion/research. We also did some benchmarks with native Map and found out that it was actually slower than the current implementation in all cases except some specific one.

I assume this is due to the fact ES6 Map guarantees element order, but this is something to research deeper.

Then again, I think there must be an option to make Haxe Maps use native ES6 Map, but I’m not sure if we should enable it by default with js-es=6.

5

Still I would prefer to have the inline removed from get, set, exists, because that gets really noisy with inlined __map_reserved.
I once created an issue:

but it was closed and inlinig (and code duplication) remained.

6

I personally think how it performs is more important than how it looks, because in general the output is executed more than it is read by people.

But, in theory you can copy/paste haxe.ds.StringMap from haxe/js/_std/haxe/ds/StringMap to your project using same structure and remove the inline if it bothers you that much. I haven’t tried this, but it should work.

7

Well that’s not true, that issue was about the anonymous function duplication which was not really related to StringMap and was fixed.

8

Sounds like the same decision problem again and again. With JSES5 we don’t use Object.create(null) because not all ES5 browsers support it. With JSES6 we don’t use Map because not all ES6 browsers support it. Maybe it makes sense then to support Object.create(null) with -D js-es=6 and maybe start to use Map with -D js-es=6++. If JS-Map has performance problems against our current StringMap, at least Object.create(null) should be faster (much ?) than current StringMap.

9

I don’t care “how it looks”, but I don’t like bloated JS. As I wrote, removing inline decreased file size by 10%.

Actually I do this, but it’s not a good solution in the long run. I constantly have to adapt that file to the Haxe version. And also overriding haxe.ds gets tricky when you use macros.

Oh, that’s right, sorry. I was looking at JS created with an older Haxe version. There is a temp var created now for anonymous functions, my bad.
So I tested with current Haxe version and for one of my projects I get 96kB with inline vs 91.6kB with inline removed for StringMap. So it’s better now, but still like 5% difference (admittedly I used quite a lot of StringMaps there).

Maybe I just don’t use StringMap anymore in JS but Object.create(null) with DynamicAccess. It’s supported in IE11, as far as I see, that’s the worst I need to support.