1807895 - Slow performance of `new Set(anotherSet)`

Status: UNCONFIRMED

(Core :: JavaScript Engine, defect, P3)

Description

[james.mountain]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36

Steps to reproduce:

const arr = new Array(1000 * 1000).fill(0).map((_, i) => i)
const s1 = new Set(arr)

// Performance of:
new Set(s1)

// Is ~40% slower than:
new Set(arr)

// And is ~10% slower than:
new Set([...s1])

// And is ~10% slower than:
const s2 = new Set()
s.forEach(v => s2.add(v))

https://jsbench.me/2qlc7mg84t/1

Actual results:

Slow performance when passing an existing set into the constructor of a new set.

Expected results:

Comparable performance to passing an array (or better).

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Performance' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

We definitely have a specific optimization path that we use only for creating a set from an array, which explains this performance gap.

While it would be nice to improve this, without evidence this is a frequent issue sites run into, I'll triage this at a relatively low priority.

Comment 3

[james.mountain]

In ui frameworks (e.g. React), it is necessary to return a new object to trigger a render. Returning a mutated Set (e.g. via s.add) will not cause a re-render. The slow performance of cloning an existing Set in mozilla means that Set is useless for use with modern ui frameworks.

Comparing performance (of the benchmark posted above) with Safari:

new Set(s)
// safari: 15 op/s
// firefox: 4.4 op/s

new Set(a)
// safari: 8.49 op/s
// firefox: 8.27 op/s

Comment 4

[james.mountain]

To add. The performance difference is even worse with smaller sets of data. E.g. with 1000 items in the set/array:

new Set(s)
// 13,000 op/s

new Set(a)
// 27,000 op/s

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

I'll bump the severity here: Will be interesting to see if Bug 1808675 aids appreciably or not.

Comment 6

[André Bargull [:anba]]

Bug 1432565 has some old patches to improve this use case.

Comment 7

[Mayank Bansal]

Firefox

• new Set(s1) : https://share.firefox.dev/42aJovk (190ms)
• new Set(arr): https://share.firefox.dev/4naOtMm (180ms)
• new Set([...s1]) : https://share.firefox.dev/42hvQy2 (200ms)
• s.forEach(v => s2.add(v)) : https://share.firefox.dev/46nZzrA (105ms)

Chrome

• new Set(s1) : https://share.firefox.dev/4g8gnGl (150ms)
• new Set(arr): https://share.firefox.dev/45QZLQ5 (200ms)
• new Set([...s1]) : https://share.firefox.dev/4gb7gVB (170ms)
• s.forEach(v => s2.add(v)) : https://share.firefox.dev/4lW7tNr (65ms)