When adding n values to multi, avoiding looping over them n times

[TrevorBurnham]

@ivaynberg Currently, every time you call $input.select2('val', array), the setVal() function is called for every array entry, and setVal() loops over the entire array of values to ensure uniqueness. When the number of values being set is fairly large (on the order of hundreds), this has a dramatic impact on rendering performance.

This commit adds a new internal method, setUniqueVal(), to call when a value array is already known to be unique (as is the case when the call originates from updateSelection).

[TrevorBurnham]

See bugfix commits on this branch. I've had a hard time getting this logic exactly right because there's a lot of redundancy... as of the 3.4.5 release, a typical val call for a multi-select (on an input[type=hidden] element) results in 4 calls to setVal, each of which loops over the val options to check for duplicates, not to mention the DOM manipulation. So while this PR is an improvement (2 calls to setVal, one call to the loop-free setUniqueVal), there's certainly room for further improvement. Fundamentally, is it necessary to set the element's value both before and after the call to initSelection?

[kevin-brown]

Fundamentally, is it necessary to set the element's value both before and after the call to initSelection?

I feel like this isn't needed, as long as the value is always set after initSelection is called. If you run git blame on the file, the commit that added the lines may better explain why it was added and if it is still needed.

[TrevorBurnham]

@kevin-brown Can you explain the close? The fact that setVal is so inefficient for multi-selects seems like a serious issue to me. This particular patch may not be the best fix, but the issue merits fixing.

[TrevorBurnham]

FYI this performance bottleneck is still an issue as of 3.4.8.

[dconnolly]

I've been using this patch in production for several weeks and it has been performing admirably, I'd love to have it (or a similar fix) merged into main so that we don't have to maintain a fork.

[kevin-brown]

I'm going to reopen this until I can figure a few things out with this. The main ticket for performance-related problems is https://github.com/ivaynberg/select2/issues/781.

I'd rather not introduce a new setUniqueVal function, but instead fix the deeper issue for setVal being called multiple times when it should only be called once.

For version 4.0, I believe this issue will be fixed as the logic for setting the values is being changed.

[TrevorBurnham]

Glad to hear it, Kevin. Thanks for the update.

[dconnolly]

👍

[kevin-brown]

https://github.com/ivaynberg/select2/pull/2848 just came in that addresses the core issue.