search

G-Research / fsharp-analyzers

Analyzers for F#
https://g-research.github.io/fsharp-analyzers/
Apache License 2.0
14 stars 1 forks source link

Update sdk and add analyzer for Seq functions that have an equivalent in the collection modules #27

Closed dawedawe closed 10 months ago

dawedawe commented 11 months ago

@Smaug123 Would you like to see warnings for all functions from the Seq module with equivalents in List|Array|Set or is there a known subset that is definitely causing expensive virtual calls?

I'm not sure, there's a reasonable way to check programmatically if we would end up with a virtual call for a Seq function.

Smaug123 commented 11 months ago

I guess Set is the really serious one, because it's designed for fast lookup. The others aren't that important in the grand scheme of things.

Smaug123 commented 11 months ago

Array.length vs Seq.length is the other silly one.

Smaug123 commented 11 months ago

(Eliminating virtual calls is indeed better than not doing so, but I guess there's a less arcane justification in the case of array length and many set operations: they're much more efficient when you don't have to traverse the entire collection to do them.)

nojaf commented 11 months ago

I'm not challenging the impact of virtual calls. I just don't think our current approach of checking each Seq function, necessarily hits a potential virtual call all the time.

For (example), I don't immediately see the virtual call there.

nojaf commented 10 months ago

@Smaug123 are you okay with the principle this analyzer wields right now? Being more of a technical check that Seq.xyz can be replaced by Module.xyz if it exists? Instead of really really knowing there indeed will be a virtual call in play?

Smaug123 commented 10 months ago

Yep, seems fine to me.