Smaug123
commented
7 months ago Hi - thanks for taking a look!
A bunch of this is based on some very old knowledge which may not be valid any more. I believe I'm the one who requested this particular analyser, and I didn't do any work to verify that it's still necessary.
However, Set.contains is specialised to logarithmic time rather than linear, isn't it? In fact Set.contains was uniquely my main motivation for this analyser; I just proposed that the other replacements tag along for the ride! (Array.length is type-tested to a special case, so if the JIT or AOT compiler are doing their jobs then there should be no performance cost to Seq.length over Array.length, but "if the JIT is doing its job" is the kind of condition that makes me very nervous.)
By the way, please note that there are a couple of reasons this set of analysers is separate from the central Ionide analysers. The main reason is that these ones are highly opinionated about code style; but also they're a bit of a proving ground and experiment for the more general analysers technology coming in F# 9. The idea is to get some real-world practice to help inform decisions there, by taking a bunch of ideas and seeing how easy and/or edge-casey they are to implement in the Ionide SDK. This particular analyser has served that purpose at least for me, in clarifying that the current setup probably doesn't give us easy access to the IL.
OkkeHendriks
Hello,
I saw your VirtualCall Analyzer and I was wondering based on which 'proof' , i.e. benchmarks or other information has the decision been made to make the suggestions it does. Especially for the cases where there is no specialized algorithm (i.e.
Set.contains)?I did some quick benchmarks, not an expert, and got inconclusive results.
I dit see https://github.com/G-Research/fsharp-analyzers/pull/30, but this is not really discussed there.