Reduce allocations by favoring TryGetValue over TryFind

[forki]

No description provided.

[forki]

@KevinRansom @cartermp the build error seems unrelated. Something wrong?

[KevinRansom]

@forki I'm just going over the weekends PR's right now. I will get back to you.

[KevinRansom]

I think they may be related:

open namespaces or file ordering perhaps?

2018-10-01T11:17:41.1413914Z "D:\a\1\s\fcs\FSharp.Compiler.Service\FSharp.Compiler.Service.fsproj" (Build target) (10:3) ->
2018-10-01T11:17:41.1414027Z   D:\a\1\s\src\absil\illib.fs(1136,48): error FS0039: The field, constructor or member 'TryGetValue' is not defined. [D:\a\1\s\fcs\FSharp.Compiler.Service\FSharp.Compiler.Service.fsproj]
2018-10-01T11:17:41.1414088Z   D:\a\1\s\src\absil\illib.fs(1150,45): error FS0039: The field, constructor or member 'TryGetValue' is not defined. [D:\a\1\s\fcs\FSharp.Compiler.Service\FSharp.Compiler.Service.fsproj]
2018-10-01T11:17:41.1414163Z   D:\a\1\s\src\absil\illib.fs(1170,47): error FS0039: The field, constructor or member 'TryGetValue' is not defined. [D:\a\1\s\fcs\FSharp.Compiler.Service\FSharp.Compiler.Service.fsproj]
2018-10-01T11:17:41.1414218Z   D:\a\1\s\src\absil\illib.fs(1176,39): error FS0039: The field, constructor or member 'TryGetValue' is not defined. [D:\a\1\s\fcs\FSharp.Compiler.Service\FSharp.Compiler.Service.fsproj]

[forki]

ok let me check

[forki]

@KevinRansom it looks like FCS is using outdated fsharp.core.

[KevinRansom]

@forki, I really think that is "By design" .... we are going to have to wait for @dsyme to determine whether he wants to move FCS upto the latest FSharp.Core.

Kevin

[TIHan]

Can we get some perf numbers on this change?

[KevinRansom]

I imagine the perf improvements are going to be negligable. My intuition is that there will be indeed be fewer allocations, but not nearly enough to notice.

It is however a decent enough refactoring, I am fairly confident that if TryGetValue had existed when the code was originaly written, it would have been written this way. My sole concern, is having to push fcs up to FSharp.Core 4.5.

Kevin

[forki]

I don't expect this to be measurable. These little Options are easy to collect. It's just that we open up more and more and more of these refactors and one day type checker is again 10 percent faster than last release. Like we did over last year. Kevin Ransom (msft) <notifications@github.com> schrieb am Mo., 1. Okt. 2018, 21:36:

…

I imagine the perf improvements are going to be negligable. My intuition is that there will be indeed be fewer allocations, but not nearly enough to notice. It is however a decent enough refactoring, I am fairly confident that if TryGetValue had existed when the code was originaly written, it would have been written this way. My sole concern, is having to push fcs up to FSharp.Core 4.5. Kevin — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5715 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNGgz8FdivcoT7lZP0J4wD0O68JP0ks5ugm7MgaJpZM4XBnXn> .

[KevinRansom]

@forki,

Thanks for this.