[Optimization] avoid copying for intersectInPlace()

[dduan]

By droping down to native storage implementation, we can perform in place
intersection without making a copy of the set.

Clears tests in validation-test/stdlib/Set.swift

[nadavrot]

@dduan Did you measure the performance of this change? Are you seeing any wins?

[dduan]

@nadavrot to be honest, I only took the hint from the "FIXME" comment and reasoned through the code. I'd love to collect some empirical evidence. Is there any established tools/convention for performance measuring for swift?

[gribozavr]

Indentation is 2 space.

[gribozavr]

Please leave a FIXME(performance) comment about the necessity to shrink the storage.

[gribozavr]

SequenceType.contains() consumes the sequence. If our tests didn't catch this, it means this patch needs to add tests that use intersectInPlace() with MinimalSequence as the argument.

To fix this issue, you need to use the _preprocessingPass() method and only query the sequence directly multiple times when it is multi-pass. Otherwise, the sequence needs to be copied to a contiguous array first.

[gribozavr]

@dduan Thank you for the patch. I support this change, but we need some changes to make sure we keep the code testable and tested.

Please take a look at this comment in the file:

   //
   // APIs below this comment should be implemented strictly in terms of
   // *public* APIs above.  `_variantStorage` should not be accessed directly.
   //
   // This separates concerns for testing.  Tests for the following APIs need
   // not to concern themselves with testing correctness of behavior of
   // underlying storage (and different variants of it), only correctness of the
   // API itself.
   //

Your change needs to move the method to a different category, and expand tests for this method to be more extensive (that is, to test with both native and bridged sets).

[dduan]

@gribozavr thanks for the comments. I'll make the changes when I get the time.

[nadavrot]

@dduan At the moment there is no good way to test the performance of your patch. Can you write a small benchmark program and test the performance before/after? Adding code to the standard library has a cost (in terms of readability and compile time) and we need to have at least on example that shows a performance win.

@gribozavr

[dduan]

@nadavrot will do!

[dduan]

@nadavrot I ran this program and the speed improvement is around 42% percent (no joke!). Here's the output from my late 2013 MBP with i7:

unoptimized, —debug-swift:
small ∩ large * 100 -- 0.291758000850677
large ∩ small * 100 -- 7.83746302127838
small ∩ small * 100 -- 0.297053933143616
large ∩ large * 100 -- 18.2310382127762
total time spent ----- 26.6573131680489

optimized, —debug-swift
small ∩ large * 100 -- 0.0793769359588623
large ∩ small * 100 -- 7.71100395917892
small ∩ small * 100 -- 0.0776850581169128
large ∩ large * 100 -- 7.35030424594879
total time spent ----- 15.2183701992035

[lattner]

Whoa!

[dduan]

@gribozavr I think I've addressed most of your comments. Care to take another look?

@lattner right back at you!

[nadavrot]

@dduan Thank you. The numbers look excellent!

[dduan]

Closing this one for now. If done right, optimization here would also speed up removeAll(). See discussion on another PR for detail.

Thanks @nadavrot @gribozavr