Optimize ListSet.setify

[rojepp]

Avoid allocatey List.foldBack and one List.rev.

I noticed that there are no tests for this, what's a good place to add some?

[cartermp]

cc @forki for review as well, since he spent some time looking at this

[forki]

There are no tests since this is only compiler internal.

[rojepp]

@forki Yeah I just tested it in a script file.

[forki]

It looks good. I'm actually wondering why I did not see that. Can you run a quick property based test in some external test project to make sure they are actually the same? But since it's green I'm confident it's OK.

[rojepp]

I’ll do that in a while. I also figured that the CI would break if it was broken.

[rojepp]

#r "paket:
source https://api.nuget.org/v3/index.json
nuget FsCheck //"
open FsCheck

let inline contains f x l = List.exists (f x) l
let insert f x l = if contains f x l then l else x::l
let setify f l = List.foldBack (insert f) (List.rev l) [] |> List.rev
let setifynew f l = List.fold (fun acc x -> insert f x acc) [] l |> List.rev
let oldAndNewIdentical (xs:list<int>) = setify (=) xs = setifynew (=) xs

Check.Quick oldAndNewIdentical

FsCheck reports:

Ok, passed 100 tests.

[forki]

Hugs Robert Jeppesen <notifications@github.com> schrieb am Do., 27. Sep. 2018, 20:39:

…

#r "paket:source https://api.nuget.org/v3/index.jsonnuget FsCheck //"open FsCheck let inline contains f x l = List.exists (f x) llet insert f x l = if contains f x l then l else x::llet setify f l = List.foldBack (insert f) (List.rev l) [] |> List.revlet setifynew f l = List.fold (fun acc x -> insert f x acc) [] l |> List.revlet oldAndNewIdentical (xs:list<int>) = setify (=) xs = setifynew (=) xs Check.Quick oldAndNewIdentical FsCheck reports: Ok, passed 100 tests. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5706 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNIdtmpkQlQXOar1hOkEnG8vRTSAwks5ufRtWgaJpZM4W77Ug> .

[dsyme]

Why was the comment removed? It is still quadratic.

[rojepp]

My mistake, sorry. While it is faster, it is clearly still quadratic. This has already been merged, do you want a PR to bring the comment back?