Use Set in uniq() When Available In Environment

[svozza]

A recent discussion on Ramda (ramda/ramda#1512) prompted me to have a look at our uniq function so I ported over the benchmarks from there and ran them against our code base. The results were not pretty:

.uniq x 10,000 (heterogenous)

underscore ######################### 61ms
lodash #### 8ms
highland ############################################################ 151ms

.uniq x 10,000 (numbers)

underscore ############################## 5ms
lodash ############ 2ms
highland ############################################################ 10ms

.uniq x 10,000 (all unique)

underscore #################### 316ms
lodash # 8ms
highland ############################################################ 966ms

However, by using Set when it is available in the environment the performance can be improved dramatically.

.uniq x 10,000 (heterogenous)

underscore ############################################################ 68ms
lodash ######## 8ms
highland ##### 5ms

.uniq x 10,000 (numbers)

underscore ############################################################ 5ms
lodash ############ 1ms
highland #################################### 3ms

.uniq x 10,000 (all unique)

underscore ############################################################ 329ms
lodash ## 8ms
highland ## 6ms

I normally shy away from this sort of feature sniffing but this is such an easy gain that will benefit all our users running Node 0.12, 4.x and 5.x that I think it's worth it in this case. Thoughts?

[apaleslimghost]

typeof Set is never going to be undefined (it might be 'undefined'), so this block will always run.

[svozza]

Yeah, I know, silly mistake.

[vqvu]

We can use !_.isUndefined(_global.Set). We detect Symbol for iterator support in the same way.

[vqvu]

This looks like a big enough win that it's worth it for me. I'll let travis run before merging.

[svozza]

Yep, and we're lucky we don't have to jump through the hoops Ramda does because they use value equality across the board.

[vqvu]

Something just occurred to me. Equality in sets is not the same as === (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set). So the current implementation will behave differently on different Nodes.

Specifically, Set uses SameValueZero, which is different from StrictEqualityComparison. Although the only difference is that NaN !== NaN, while SameValueZero(NaN, NaN) === true.

We need to check for NaN in the Set implementation and always pass it through.

Also, I personally don't think NaN !== NaN is intuitive despite IEEE's opinion on the matter, especially in the context of a uniq transform. Should we switch to using SameValueZero for the equality algorithm in 3.0.0?

For comparison

• lodash uses SameValueZero
• underscore uses StrictEqualityComparison
• rambda uses their own value equality algorithm

Edit: Typo. I meant to say that NaN !== NaN is unintuitive.

[svozza]

No, I think we should stay with strict equality as that's what uniq is supposed to do. It's just a leaky abstraction if we allow the underlying implementation to change the semantics like that.

[vqvu]

You might have misunderstood me. For 2.x, we should definitely fix the Set implementation to use strict equality. My question was whether or not we should switch to SameValueZero for both implementation in 3.0.0.

[svozza]

No, I got what you meant, I think we should keep strict equality for 3.0; for me, uniq always should be defined in terms of === .

[vqvu]

Fair enough.