Optimize completion

[vasily-kirichenko]

First invocation

Before

After

Second invocation

Before

After

[cartermp]

@dotnet-bot test this please

[vasily-kirichenko]

@cartermp it's not compilable at the moment.

[forki]

Never mind. Vasily Kirichenko <notifications@github.com> schrieb am Sa., 2. Juni 2018, 09:11:

…

***@***.**** commented on this pull request. ------------------------------ In src/fsharp/service/ServiceAssemblyContent.fs <#5066 (comment)> : > @@ -186,14 +187,31 @@ type IAssemblyContentCache = module AssemblyContentProvider = open System.IO - let private createEntity ns (parent: Parent) (entity: FSharpEntity) = + let unresolvedSymbol (topRequireQualifiedAccessParent: Idents option) (cleanedIdents: Idents) (fullName: string) = + let getNamespace (idents: Idents) = + if idents.Length > 1 then Some idents.[..idents.Length - 2] else None + + let ns = + topRequireQualifiedAccessParent + |> Option.bind getNamespace + |> Option.orElseWith (fun () -> getNamespace cleanedIdents) + |> Option.defaultValue [||] What do you mean? [||] is a singleton, no allocations here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5066 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNCVeV8NhG-DO3ObG2xKzF418Qipqks5t4jqUgaJpZM4UWqUx> .

[vasily-kirichenko]

@forki @smoothdeveloper all done, thanks.

[vasily-kirichenko]

Before caching ILTypeDef.CustomAttrs

After

[dsyme]

This is great work!

[vasily-kirichenko]

Before storing attrs in ILTypeDef (after executing completion in FSharp.Editor project):

After:

[vasily-kirichenko]

@dsyme Do I understand right that we still cannot use struct tuples in FSharp.Compiler.Private project?

[vasily-kirichenko]

I spot a lack of fast comparer for bool values. This is number of allocations caused by a single completion popup on FSharp.Editor project:

After providing a comparer, it look like this:

It's eliminated 1M boxed bool and 700K tuples allocations.

[dsyme]

Do I understand right that we still cannot use struct tuples in FSharp.Compiler.Private project?

You can if you really need to but can cause some problems with packaging various releases and the Fable build of the compiler - but it's normally clearer to introduce a named struct

[dsyme]

I spot a lack of fast comparer for bool values.

@vasily-kirichenko Nice catch. We should put that change into F# 4.5. Could you isolate that 3-line change out into a separate PR?

[vasily-kirichenko]

@dsyme done #5076

[vasily-kirichenko]

You can if you really need to but can cause some problems with packaging various releases and the Fable build of the compiler - but it's normally clearer to introduce a named struct

OK, replacing tuples with structs needs benchmarking anyway.

[dsyme]

Re Array.Empty, I meant:

type EmptyArray<'T>() = 
    static let empty = ([| |] : 'T [])
    static member Empty = empty

and so on. i.e. type-indexed by automagic of .NET generics.

.NET 4.something actually has Array.Empty() here:

> (System.Array.Empty() : int[]);;
val it : int [] = [||]

which is a singleton. Hwoever it's only available since .NET 4.6 and FSharp.Core needs to run on .NET 4.5. We could use it for .NET Standard though

[vasily-kirichenko]

@dsyme I've added the array empty singleton.

[vasily-kirichenko]

So, this makes completion 2x faster on FSharp.Editor project:

Master

This branch (release)

[vasily-kirichenko]

It turns out that

open System
open BenchmarkDotNet
open BenchmarkDotNet.Attributes
open BenchmarkDotNet.Running

type Test() =
    [<Benchmark(Baseline = true)>]
    member __.Sprintf() = sprintf "%06d" 123456

    [<Benchmark>]
    member __.Format() = (123456).ToString("D6")

[<EntryPoint>]
let main _ = 
    BenchmarkRunner.Run<Test>() |> ignore
    0

BenchmarkDotNet=v0.10.14, OS=Windows 10.0.17134
Intel Core i5-7600 CPU 3.50GHz (Kaby Lake), 1 CPU, 4 logical and 4 physical cores
  [Host]     : .NET Framework 4.6.1 (CLR 4.0.30319.42000), 32bit LegacyJIT-v4.7.3101.0
  DefaultJob : .NET Framework 4.6.1 (CLR 4.0.30319.42000), 32bit LegacyJIT-v4.7.3101.0


  Method |     Mean |    Error |   StdDev | Scaled |
-------- |---------:|---------:|---------:|-------:|
 Sprintf | 340.3 ns | 5.908 ns | 5.237 ns |   1.00 |
  Format | 111.1 ns | 1.295 ns | 1.148 ns |   0.33 |

[vasily-kirichenko]

@dotnet-bot test Ubuntu16.04 Release_default Build please

[cartermp]

@dsyme can you re-review this? We have until Tuesday afternoon, then we will make the cutoff for dev15.8. I'd love to get this in for that.