Skip to content

Commit a0050e1

Browse files
KevinRansomKevinRansom
authored
Merge pull request #1479 from manofstick/manofstick-array-filter
[WIP] Alternative Array.Filter implementation
2 parents 6fd485d + 198c30a commit a0050e1

File tree

2 files changed

+220
-37
lines changed

2 files changed

+220
-37
lines changed

‎src/fsharp/FSharp.Core.Unittests/FSharp.Core/Microsoft.FSharp.Collections/ArrayModule.fs‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,64 @@ type ArrayModule() =
724724
let nullArr = null:string[]
725725
CheckThrowsArgumentNullException (fun () -> Array.filter funcStr nullArr |> ignore)
726726

727-
()
727+
()
728+
729+
[<Test>]
730+
member this.Filter2 () =
731+
// The Array.filter algorith uses a bitmask as a temporary storage mechanism
732+
// for which elements to filter. This introduces some possible error conditions
733+
// around how the filter is filled and subsequently used, so filter test
734+
// does a pretty exhaustive test suite.
735+
// It works by first generating arrays which consist of sequences of unique
736+
// positive and negative numbers, as per arguments, it then filters for the
737+
// positive values, and then compares the results agains the original array.
738+
739+
let makeTestArray size posLength negLength startWithPos startFromEnd =
740+
let array = Array.zeroCreate size
741+
742+
let mutable sign = if startWithPos then 1 else -1
743+
let mutable count = if startWithPos then posLength else negLength
744+
for i = 1 to size do
745+
let idx = if startFromEnd then size-i else i-1
746+
array.[idx] <- (idx+1) * sign
747+
count <- count - 1
748+
if count <= 0 then
749+
sign <- sign * -1
750+
count <- if sign > 0 then posLength else negLength
751+
752+
array
753+
754+
let checkFilter filter (array:array<_>) =
755+
let filtered = array |> filter (fun n -> n > 0)
756+
757+
let mutable idx = 0
758+
for item in filtered do
759+
while array.[idx] < item do
760+
idx <- idx + 1
761+
if item <> array.[idx] then
762+
Assert.Fail ()
763+
idx <- idx + 1
764+
while idx < array.Length do
765+
if array.[idx] > 0 then
766+
Assert.Fail ()
767+
idx <- idx + 1
768+
769+
let checkCombinations filter maxSize =
770+
for size = 0 to maxSize do
771+
for posLength = 1 to size do
772+
for negLength = 1 to size do
773+
for startWithPos in [true; false] do
774+
for startFromEnd in [true; false] do
775+
let testArray = makeTestArray size posLength negLength startWithPos startFromEnd
776+
checkFilter filter testArray
777+
778+
// this could probably be a bit smaller, but needs to at least be > 64 to test chunk copying
779+
// of data, and > 96 gives a safer feel, so settle on a nice decimal rounding of one hundred
780+
// to appease those with digits.
781+
let suitableTestMaxLength = 100
782+
783+
checkCombinations Array.filter suitableTestMaxLength
784+
728785

729786

730787
[<Test>]

‎src/fsharp/FSharp.Core/array.fs‎

Lines changed: 162 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -518,44 +518,170 @@ namespace Microsoft.FSharp.Collections
518518
else
519519
empty
520520

521+
// The filter module is a space and performance for Array.filter based optimization that uses
522+
// a bitarray to store the results of the filtering of every element of the array. This means
523+
// that the only additional temporary garbage that needs to be allocated is {array.Length/8} bytes.
524+
//
525+
// Other optimizations include:
526+
// - arrays < 32 elements don't allocate any garbage at all
527+
// - when the predicate yields consecutive runs of true data that is >= 32 elements (and fall
528+
// into maskArray buckets) are copied in chunks using System.Array.Copy
529+
module Filter =
530+
let private populateMask<'a> (f:'a->bool) (src:array<'a>) (maskArray:array<uint32>) =
531+
let mutable count = 0
532+
for maskIdx = 0 to maskArray.Length-1 do
533+
let srcIdx = maskIdx * 32
534+
let mutable mask = 0u
535+
if f src.[srcIdx+0x00] then mask <- mask ||| (1u <<< 0x00); count <- count + 1
536+
if f src.[srcIdx+0x01] then mask <- mask ||| (1u <<< 0x01); count <- count + 1
537+
if f src.[srcIdx+0x02] then mask <- mask ||| (1u <<< 0x02); count <- count + 1
538+
if f src.[srcIdx+0x03] then mask <- mask ||| (1u <<< 0x03); count <- count + 1
539+
if f src.[srcIdx+0x04] then mask <- mask ||| (1u <<< 0x04); count <- count + 1
540+
if f src.[srcIdx+0x05] then mask <- mask ||| (1u <<< 0x05); count <- count + 1
541+
if f src.[srcIdx+0x06] then mask <- mask ||| (1u <<< 0x06); count <- count + 1
542+
if f src.[srcIdx+0x07] then mask <- mask ||| (1u <<< 0x07); count <- count + 1
543+
if f src.[srcIdx+0x08] then mask <- mask ||| (1u <<< 0x08); count <- count + 1
544+
if f src.[srcIdx+0x09] then mask <- mask ||| (1u <<< 0x09); count <- count + 1
545+
if f src.[srcIdx+0x0A] then mask <- mask ||| (1u <<< 0x0A); count <- count + 1
546+
if f src.[srcIdx+0x0B] then mask <- mask ||| (1u <<< 0x0B); count <- count + 1
547+
if f src.[srcIdx+0x0C] then mask <- mask ||| (1u <<< 0x0C); count <- count + 1
548+
if f src.[srcIdx+0x0D] then mask <- mask ||| (1u <<< 0x0D); count <- count + 1
549+
if f src.[srcIdx+0x0E] then mask <- mask ||| (1u <<< 0x0E); count <- count + 1
550+
if f src.[srcIdx+0x0F] then mask <- mask ||| (1u <<< 0x0F); count <- count + 1
551+
if f src.[srcIdx+0x10] then mask <- mask ||| (1u <<< 0x10); count <- count + 1
552+
if f src.[srcIdx+0x11] then mask <- mask ||| (1u <<< 0x11); count <- count + 1
553+
if f src.[srcIdx+0x12] then mask <- mask ||| (1u <<< 0x12); count <- count + 1
554+
if f src.[srcIdx+0x13] then mask <- mask ||| (1u <<< 0x13); count <- count + 1
555+
if f src.[srcIdx+0x14] then mask <- mask ||| (1u <<< 0x14); count <- count + 1
556+
if f src.[srcIdx+0x15] then mask <- mask ||| (1u <<< 0x15); count <- count + 1
557+
if f src.[srcIdx+0x16] then mask <- mask ||| (1u <<< 0x16); count <- count + 1
558+
if f src.[srcIdx+0x17] then mask <- mask ||| (1u <<< 0x17); count <- count + 1
559+
if f src.[srcIdx+0x18] then mask <- mask ||| (1u <<< 0x18); count <- count + 1
560+
if f src.[srcIdx+0x19] then mask <- mask ||| (1u <<< 0x19); count <- count + 1
561+
if f src.[srcIdx+0x1A] then mask <- mask ||| (1u <<< 0x1A); count <- count + 1
562+
if f src.[srcIdx+0x1B] then mask <- mask ||| (1u <<< 0x1B); count <- count + 1
563+
if f src.[srcIdx+0x1C] then mask <- mask ||| (1u <<< 0x1C); count <- count + 1
564+
if f src.[srcIdx+0x1D] then mask <- mask ||| (1u <<< 0x1D); count <- count + 1
565+
if f src.[srcIdx+0x1E] then mask <- mask ||| (1u <<< 0x1E); count <- count + 1
566+
if f src.[srcIdx+0x1F] then mask <- mask ||| (1u <<< 0x1F); count <- count + 1
567+
maskArray.[maskIdx] <- mask
568+
count
569+
570+
let private createMask<'a> (f:'a->bool) (src:array<'a>) (maskArrayOut:byref<array<uint32>>) (leftoverMaskOut:byref<uint32>) =
571+
let maskArrayLength = src.Length / 0x20
572+
573+
// null when there are less than 32 items in src array.
574+
let maskArray =
575+
if maskArrayLength = 0 then Unchecked.defaultof<_>
576+
else Array.zeroCreateUnchecked<uint32> maskArrayLength
577+
578+
let mutable count =
579+
match maskArray with
580+
| null -> 0
581+
| maskArray -> populateMask f src maskArray
582+
583+
let leftoverMask =
584+
match src.Length % 0x20 with
585+
| 0 -> 0u
586+
| _ ->
587+
let mutable mask = 0u
588+
let mutable elementMask = 1u
589+
for arrayIdx = maskArrayLength*0x20 to src.Length-1 do
590+
if f src.[arrayIdx] then mask <- mask ||| elementMask; count <- count + 1
591+
elementMask <- elementMask <<< 1
592+
mask
593+
594+
maskArrayOut <- maskArray
595+
leftoverMaskOut <- leftoverMask
596+
count
597+
598+
let private populateDstViaMask<'a> (src:array<'a>) (maskArray:array<uint32>) (dst:array<'a>) =
599+
let mutable dstIdx = 0
600+
let mutable batchCount = 0
601+
for maskIdx = 0 to maskArray.Length-1 do
602+
let mask = maskArray.[maskIdx]
603+
if mask = 0xFFFFFFFFu then
604+
batchCount <- batchCount + 1
605+
else
606+
let srcIdx = maskIdx * 0x20
607+
608+
if batchCount <> 0 then
609+
let batchSize = batchCount * 0x20
610+
System.Array.Copy (src, srcIdx-batchSize, dst, dstIdx, batchSize)
611+
dstIdx <- dstIdx + batchSize
612+
batchCount <- 0
613+
614+
if mask <> 0u then
615+
if mask &&& (1u <<< 0x00) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x00]; dstIdx <- dstIdx + 1
616+
if mask &&& (1u <<< 0x01) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x01]; dstIdx <- dstIdx + 1
617+
if mask &&& (1u <<< 0x02) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x02]; dstIdx <- dstIdx + 1
618+
if mask &&& (1u <<< 0x03) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x03]; dstIdx <- dstIdx + 1
619+
if mask &&& (1u <<< 0x04) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x04]; dstIdx <- dstIdx + 1
620+
if mask &&& (1u <<< 0x05) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x05]; dstIdx <- dstIdx + 1
621+
if mask &&& (1u <<< 0x06) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x06]; dstIdx <- dstIdx + 1
622+
if mask &&& (1u <<< 0x07) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x07]; dstIdx <- dstIdx + 1
623+
if mask &&& (1u <<< 0x08) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x08]; dstIdx <- dstIdx + 1
624+
if mask &&& (1u <<< 0x09) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x09]; dstIdx <- dstIdx + 1
625+
if mask &&& (1u <<< 0x0A) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x0A]; dstIdx <- dstIdx + 1
626+
if mask &&& (1u <<< 0x0B) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x0B]; dstIdx <- dstIdx + 1
627+
if mask &&& (1u <<< 0x0C) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x0C]; dstIdx <- dstIdx + 1
628+
if mask &&& (1u <<< 0x0D) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x0D]; dstIdx <- dstIdx + 1
629+
if mask &&& (1u <<< 0x0E) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x0E]; dstIdx <- dstIdx + 1
630+
if mask &&& (1u <<< 0x0F) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x0F]; dstIdx <- dstIdx + 1
631+
if mask &&& (1u <<< 0x10) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x10]; dstIdx <- dstIdx + 1
632+
if mask &&& (1u <<< 0x11) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x11]; dstIdx <- dstIdx + 1
633+
if mask &&& (1u <<< 0x12) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x12]; dstIdx <- dstIdx + 1
634+
if mask &&& (1u <<< 0x13) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x13]; dstIdx <- dstIdx + 1
635+
if mask &&& (1u <<< 0x14) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x14]; dstIdx <- dstIdx + 1
636+
if mask &&& (1u <<< 0x15) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x15]; dstIdx <- dstIdx + 1
637+
if mask &&& (1u <<< 0x16) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x16]; dstIdx <- dstIdx + 1
638+
if mask &&& (1u <<< 0x17) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x17]; dstIdx <- dstIdx + 1
639+
if mask &&& (1u <<< 0x18) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x18]; dstIdx <- dstIdx + 1
640+
if mask &&& (1u <<< 0x19) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x19]; dstIdx <- dstIdx + 1
641+
if mask &&& (1u <<< 0x1A) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x1A]; dstIdx <- dstIdx + 1
642+
if mask &&& (1u <<< 0x1B) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x1B]; dstIdx <- dstIdx + 1
643+
if mask &&& (1u <<< 0x1C) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x1C]; dstIdx <- dstIdx + 1
644+
if mask &&& (1u <<< 0x1D) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x1D]; dstIdx <- dstIdx + 1
645+
if mask &&& (1u <<< 0x1E) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x1E]; dstIdx <- dstIdx + 1
646+
if mask &&& (1u <<< 0x1F) <> 0u then dst.[dstIdx] <- src.[srcIdx+0x1F]; dstIdx <- dstIdx + 1
647+
648+
if batchCount <> 0 then
649+
let srcIdx = maskArray.Length * 0x20
650+
let batchSize = batchCount * 0x20
651+
System.Array.Copy (src, srcIdx-batchSize, dst, dstIdx, batchSize)
652+
dstIdx <- dstIdx + batchSize
653+
654+
dstIdx
655+
656+
let private filterViaMask (maskArray:array<uint32>) (leftoverMask:uint32) (count:int) (src:array<_>) =
657+
let dst = Array.zeroCreateUnchecked count
658+
659+
let mutable dstIdx = 0
660+
let srcIdx =
661+
match maskArray with
662+
| null -> 0
663+
| _ ->
664+
dstIdx <- populateDstViaMask src maskArray dst
665+
maskArray.Length*0x20
666+
667+
let mutable elementMask = 1u
668+
for srcIdx = srcIdx to src.Length-1 do
669+
if leftoverMask &&& elementMask <> 0u then dst.[dstIdx] <- src.[srcIdx]; dstIdx <- dstIdx + 1
670+
elementMask <- elementMask <<< 1
671+
672+
dst
673+
674+
let filter f (src:array<_>) =
675+
let mutable maskArray = Unchecked.defaultof<_>
676+
let mutable leftOverMask = Unchecked.defaultof<_>
677+
match createMask f src &maskArray &leftOverMask with
678+
| 0 -> empty
679+
| count -> filterViaMask maskArray leftOverMask count src
680+
521681
[<CompiledName("Filter")>]
522682
let filter f (array: _[]) =
523-
checkNonNull "array" array
524-
let mutable i = 0
525-
while i < array.Length && not (f array.[i]) do
526-
i <- i + 1
527-
528-
if i <> array.Length then
529-
let mutable element = array.[i]
530-
let chunk1 : 'T[] = Array.zeroCreateUnchecked (((array.Length-i) >>> 2) + 1)
531-
let mutable count = 1
532-
chunk1.[0] <- element
533-
i <- i + 1
534-
while count < chunk1.Length && i < array.Length do
535-
element <- array.[i]
536-
if f element then
537-
chunk1.[count] <- element
538-
count <- count + 1
539-
i <- i + 1
540-
541-
if i < array.Length then
542-
let chunk2 = Array.zeroCreateUnchecked (array.Length-i)
543-
count <- 0
544-
while i < array.Length do
545-
element <- array.[i]
546-
if f element then
547-
chunk2.[count] <- element
548-
count <- count + 1
549-
i <- i + 1
550-
551-
let res : 'T[] = Array.zeroCreateUnchecked (chunk1.Length + count)
552-
Array.Copy(chunk1,res,chunk1.Length)
553-
Array.Copy(chunk2,0,res,chunk1.Length,count)
554-
res
555-
else
556-
Array.subUnchecked 0 count chunk1
557-
else empty
558-
683+
checkNonNull "array" array
684+
Filter.filter f array
559685

560686
[<CompiledName("Where")>]
561687
let where f (array: _[]) = filter f array

0 commit comments

Comments
 (0)