Math.log2 should be exact for exact powers

[Yaffle]

Math.log2(1.0 * (1 << 29)) === 29.000000000000004 in Firefox

[cscott]

Really? Does the spec say this should be exact?

[ljharb]

I'm all for making Math shims more accurate, even if the spec doesn't require it.

However, we'd need the following:

• tests that ensure the behavior is correct
• example engines that implement this correctly
• a link to the relevant spec section that, at the least, leaves it to "implementation-dependent"

[cscott]

Well, at least two projects that I know of have recently switched to core-js for their shims because es6-shim was "too slow". Adding extra cruft to methods to obtain accuracy not required by the spec will not help es6-shim if it slows down the library for all users.

[Yaffle]

@ljharb , I have updated my patch a little

[ljharb]

@cscott What is "too slow"? Which methods? Did they use profiling to determine that?

[zloirock]

@ljharb

What is "too slow"? Which methods? Did they use profiling to determine that?

Simple example, in all modern environment with fully complying with the specification Map and Set, es6-shim replaces it to slow shim with O(n) search complexity for objects. Try with es6-shim and without:

var array = [];
for(var i = 0; i < 100000; i++)array.push([{}, {}]);
array = array.concat(array);
console.time('Map test');
var map = new Map(array);
console.log('Map size: ' + map.size);
console.timeEnd('Map test');

BTW, in the specification clearly states:

Map object must be implemented using either hash tables or other mechanisms that, on average, provide access times that are sublinear on the number of elements in the collection.

[ljharb]

@zloirock they're not actually fully complying if the shim is replacing them (zero engines fully comply at the moment). But yes, that is a limitation, since it's impossible to a) not mutate the objects and also b) have O(1) lookup for them.

However, I've filed https://github.com/paulmillr/es6-shim/issues/326 so that in this situation, we use the native Map for O(1) lookup, which should alleviate the speed issue.

[ljharb]

@Yaffle This code is totally fine now, thanks! Could you also add a test that fails without your changes?

[zloirock]

@ljharb

they're not actually fully complying if the shim is replacing them

they're fully complying, but your subclassing detection is broken :)

that is a limitation, since it's impossible

but why core-js and some other shims polyfill it? :D

not mutate the objects

in the spec I don't see anything about it.

[ljharb]

@zloirock core-js must be modifying the objects. I'd be happy to accept a pull request if you know of a way to do it, or a link to their implementation. Nothing must ever mutate unless the spec requires it to, so if it's mutating, it's violating the spec.

As for the subclassing detection, please file an issue or open a PR, but I don't think it's broken - you can't actually subclass the native Map/Set implementations at the moment precisely because of how the test fails without the shim ¯(ツ)/¯

[Yaffle]

@zloirock object mutation is not good and may even affect performance (I hope yo read this page - https://developers.google.com/v8/design ) Anyway, this depends on the use case: if some user wants to store objects in Map AND thinks that it is OK to mutate them, then He can use core-js.

If the number of object is not big or he can map.set(object.uid) or he wants to store only numbers/strings, he may use es6-shim.

[zloirock]

@Yaffle broken and slow collections much worse :) Currently, it's required only for old IE, other actual browsers has native collections. Better fix broken methods than replace all collection :)

[ljharb]

Let's debate this on another issue, this isn't the one for it, but no, slow is infinitely better than broken (ours is not broken in any way), and mutation is absolutely broken (and as pointed out, insanely deoptimizes objects in v8, so it'd end up being slower in many ways as well).

[zloirock]

@ljharb this line should not work in ES6. All modern browsers, includes V8, contains native collections with correct logic. All from me :)

[zloirock]

Oh, it's more: https://github.com/babel/babel/issues/1172

[ljharb]

@zloirock How then can you subclass Map/Set without class? I'll confirm with TC39 but I'm reasonably sure that's, at best, an unintended side effect.

[zloirock]

@ljharb see link. It's only emulation, but correct in most case and much better replacing correct collection to broken.

[Yaffle]

@ljharb , example engines that implement this correctly: the engine of Edge, Chrome, Safari, Firefox (with an exception of Math.log2 results under Windows), Konqueror