Use Set instead of Array

[alexeyraspopov]

Pros:

• Smaller size (181b)
• Easier to deal with handler's deletion
• Make sure the same handlers in * and in other types are called only once
• Prevent unexpected handlers duplication in one type

Cons:

• Removes the support of duplicated handlers in the same list. Personally I consider this as a pros because this behavior always leads to unexpected redundant handler calls when you don't expect it at all. Once you run into this issue, you start to wrapping code into conditions to prevent unnecessary registrations.

[developit]

What is browser support for Set like? I think I had avoided it because this library supports IE9+.

[alexeyraspopov]

@developit, unfortunately, all the trusted tables (kangax?) show info starting from IE11. Can't we drop IE9/IE10 support by default and recommend using polyfill if needed?

[developit]

Personally everything I do still needs to support IE9+, since that's what I support both for Preact-related things and at work. Might need to ponder this for a bit.

[hzoo]

@developit could output multiple builds one for modern, old for old, make one the default

[tunnckoCore]

@hzoo that's good idea, btw. Worth nothing.

[developit]

That's a really good idea. Same outward interface, just vary the internals to take advantage of Set or WeakMap for those who know that will be available in their target environment. They could just be exported as alternative entries.

[fregante]

@hzoo what about the con outlined in the first post? Would the two builds behave differently?

I don't think it's worth it, generally event listeners don't care for duplicate handlers and this would break that expectation.

[developit]

This might be interesting when paired with jsnext:main/modules main fields in the package.json. It might also be a headache, but at least that's an existing way of denoting es3/5 VS esnext, which is the closest thing we have to an expectation around whether Set is supported in the user's target env.

FWIW, I've changed the build around a bunch so that the gzipped size is actually tracking the universal bundle output, not just the commonjs output. This made it a lot harder to stay below 200 bytes, but I managed to do it. The update is here: 3a675c7

--edit-- @alexeyraspopov I don't think the ... in your emit() would have been transpiled by Buble, might have a negative impact on output size.

[fregante]

Don't confuse module with ES2015. A bundler can read that field while still not transpiling your dependency. It's akin to saying that AMD packages are always compatible with ES3 browsers

[tunnckoCore]

@developit: I don't think the ... in your emit() would have been transpiled by Buble, might have a negative impact on output size.

I already tried that PR and it don't have impact. It is still 181b.

[developit]

module refers to ES Modules, which is part of the ES2017 spec. To me, that's a lot more closely related than JS (spec) and AMD (userland pattern). I agree it's not 1:1, but typically if someone is parsing JS to transform ES Modules they are already capable of transforming the source from ESnext.

@tunnckoCore - tried the package.json changes you mean?

[fregante]

@developit even if they are doing it (which isn't necessarily true), babel doesn't automatically polyfill Set

[developit]

I know, just Set is an ES2015 feature in terms of where it got spec'd.

[alexeyraspopov]

@developit, it does some unexpected thing :(
https://buble.surge.sh/#%5B...new%20Set()%5D

[hzoo]

@bfred-it what do you mean by automatically polyfill Set? technically everything is opt-in (and that's the repl which has specific presets?)

[developit]

@alexeyraspopov that's a fairly odd thing for buble to do, I guess it's just not accounted for at all.

[hzoo]

^ It does the same thing as Babel in loose mode since it assumes it's an array

[tunnckoCore]

Just read its docs.

Bublé only transpiles language features – it doesn't attempt to shim or polyfill the environment. If you want to use things like array.findIndex(...) or 'x'.repeat(3) or Map and Set you'll have to bring your own polyfill (check out core-js or es6-shim).

Nothing surprising for me.

[hzoo]

And that is the same as Babel - either you use babel-polyfill, your own polyfill, or transform-runtime https://babeljs.io/#polyfill

[fregante]

@hzoo that's the issue: configuration. By default browserify doesn't run on node_modules at all so even if it babel-polyfill is in place it probably won't detect Set in an external package, unless… more config.

In short, this is not automatic and it's essentially the same as raising the support to IE11, since it requires the user to supply polyfills.

[hzoo]

Yeah this is what w3ctag/polyfills#6 is all about. It's not the same if you provide multiple builds like I mentioned. If someone really wants the savings they can use the other alternative build with Set, etc

[tunnckoCore]

Most easier thing is to provide two versions and I dont seeing the problem of that. One using Set, one that uses array. Based on one codebase. It can be done with with Rollup (the replace plugin) and most things from the npm scripts line would go into two separate rollup config files.

[developit]

Any chance we could compare performance between Array and Set for this use-case? I don't mind setting something up on ESBench but I'll likely not get to it for a few days

[dotproto]

I'm not well versed in putting together these kinds of benchmarks, but I took a stab at it: https://esbench.com/bench/587ed73899634800a0347623 . I'm happy to fix/change whatever's necessary. The following results were gathered by running my test with Babel transpilation disabled.

Chrome 55 (Current)

impl	ops/s	diff
array	1,126,083 ops/s ±0.93%	521.0%
set	216,137 ops/s ±0.92%	19.2%

Chrome 57 (Canary)

impl	ops/s	diff
array	1,704,497 ops/s ±1.45%	181.3%
set	939,693 ops/s ±1.33%	55.1%

Firefox 50.1.0 (current)

impl	ops/s	diff
array	1,988,618 ops/s ±0.76%	1,482.7%
set	134,117 ops/s ±15.16%	6.7%

Firefox 52.0a2 (Dev Edition)

impl	ops/s	diff
array	2,022,999 ops/s ±0.65%	3,933.5%
set	51,430 ops/s ±6.85%	2.5%

Safari 10.0.2

impl	ops/s	diff
array	1,210,535 ops/s ±2.89%	746.4%
set	162,189 ops/s ±2.01%	13.4%

Safari Tech Preview Release 21

impl	ops/s	diff
array	1,665,032 ops/s ±2.55%	957.7%
set	173,858 ops/s ±1.33%	10.4%

[developit]

wow you are amazing @SVincent 🌟🙇

I can't say I'm entirely surprised Set performs so poorly compared to Array. One of the key advantages of Array is that it's optimized more like a primitive type in JS - if a few checks pass (like Array.prototype being unmodified), most modern JS engines will actually inline methods like .push(). It's pretty awesome, and one of the reasons I recommend people put together benchmarks like the one @SVincent did. Sometimes performance characteristics that seem intuitive are actually quite the opposite in JS because engines optimize for the common case, not necessarily for the easy case.

[dotproto]

While Set vastly underperforms today, I very much hope JS engines will optimize it to be near (or better than) Array's performance. I always assumed that Set and Map would allow for a new class of optimizations not possible with Array and Object. We can already see evidence of perf improvements in the Chrome 55 vs. 57 numbers. So there's hope, I think.

Of course we also live in the here and now. Given the wild variance in Set performance it's probably best to stick with Array until all evergreens are within (say) at least 50% of Array's perf.

[developit]

Agreed with both opinions there. Plus we have a PR sitting here for when Set gets hyper-optimized and we need to switch for perf reasons 👍