Reducing memory allocations for file path maths (#1139)

paket.dependencies

@@ -56,7 +56,8 @@ nuget GitHubActionsTestLogger
 nuget Ionide.LanguageServerProtocol >= 0.4.16
 nuget Microsoft.Extensions.Caching.Memory
 nuget OpenTelemetry.Exporter.OpenTelemetryProtocol >= 1.3.2
-
+nuget LinkDotNet.StringBuilder 1.18.0
+nuget CommunityToolkit.HighPerformance
 
 
 

paket.lock

@@ -28,6 +28,11 @@ NUGET
       System.Memory (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (&& (== netstandard2.1) (>= net461))
       System.Threading.Tasks.Extensions (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (&& (== netstandard2.1) (>= net461))
     CommandLineParser (2.4.3)
+    CommunityToolkit.HighPerformance (7.0.1)
+      Microsoft.Bcl.HashCode (>= 1.1) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (== netstandard2.0)
+      System.Memory (>= 4.5.4) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net6.0) (< netstandard2.0)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netstandard2.0)) (== netstandard2.0) (&& (== netstandard2.1) (< netstandard2.0))
+      System.Runtime.CompilerServices.Unsafe (>= 5.0) - restriction: || (&& (== net6.0) (< net5.0)) (&& (== net6.0) (< netcoreapp2.1)) (&& (== net6.0) (< netstandard2.0)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (< net5.0)) (&& (== net7.0) (< netcoreapp2.1)) (&& (== net7.0) (< netstandard2.0)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (== netstandard2.1)
+      System.Threading.Tasks.Extensions (>= 4.5.4) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net6.0) (< netstandard2.0)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netstandard2.0)) (== netstandard2.0) (&& (== netstandard2.1) (< netstandard2.0))
     Destructurama.FSharp (1.2)
       FSharp.Core (>= 4.3.4)
       Serilog (>= 2.0 < 3.0)
@@ -148,6 +153,7 @@ NUGET
       Ionide.ProjInfo.Sln (>= 0.61.3) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net6.0))
       Newtonsoft.Json (>= 13.0.1) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net6.0))
     Ionide.ProjInfo.Sln (0.61.3)
+    LinkDotNet.StringBuilder (1.18)
     McMaster.NETCore.Plugins (1.4) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net5.0)) (&& (== netstandard2.1) (>= net5.0))
       Microsoft.DotNet.PlatformAbstractions (>= 3.1.6) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= netcoreapp2.1)) (&& (== netstandard2.1) (>= netcoreapp2.1))
       Microsoft.Extensions.DependencyModel (>= 5.0) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= netcoreapp2.1)) (&& (== netstandard2.1) (>= netcoreapp2.1))
@@ -162,6 +168,7 @@ NUGET
     MessagePack.Annotations (2.4.35)
     Microsoft.Bcl.AsyncInterfaces (6.0)
       System.Threading.Tasks.Extensions (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0) (&& (== netstandard2.1) (>= net461))
+    Microsoft.Bcl.HashCode (1.1) - restriction: || (&& (== net6.0) (< netcoreapp2.1) (< netstandard2.1)) (&& (== net7.0) (< netcoreapp2.1) (< netstandard2.1)) (== netstandard2.0)
     Microsoft.Build (17.2) - copy_local: false
       Microsoft.Build.Framework (>= 17.2) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net472)) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net472)) (&& (== netstandard2.1) (>= net6.0))
       Microsoft.NET.StringTools (>= 1.0) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net472)) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net472)) (&& (== netstandard2.1) (>= net6.0))

src/Directory.Build.props

@@ -7,5 +7,8 @@
     <NoWarn>$(NoWarn);FS2003</NoWarn>
     <!-- We've got a prerelease dep on System.CommandLine, and NuGet doesn't like that. NuGet can be quiet in this instance. -->
     <NoWarn>$(NoWarn);NU5104</NoWarn>
+    <!-- Implicit convertions for string should be fine https://github.com/fsharp/fslang-design/blob/main/FSharp-6.0/FS-1093-additional-conversions.md#motivation-for-op_implicit-type-directed-conversion -->
+    <NoWarn>$(NoWarn);FS3391</NoWarn>
+
   </PropertyGroup>
 </Project>

src/FsAutoComplete.Core/CompilerServiceInterface.fs

@@ -325,6 +325,12 @@ type FSharpCompilerServiceChecker(hasAnalyzers, typecheckCacheSize) =
           else
             return r
       with ex ->
+        checkerLogger.error (
+          Log.setMessage "{opName} completed with exception: {ex}"
+          >> Log.addContextDestructured "opName" opName
+          >> Log.addExn ex
+        )
+
         return! ResultOrString.Error(ex.ToString())
     }
 

src/FsAutoComplete.Core/FileSystem.fs

@@ -822,22 +822,22 @@ type FileSystem(actualFs: IFileSystem, tryFindFile: string<LocalPath> -> Volatil
     member _.IsPathRootedShim(p: string) =
       let r = isWindowsStyleRootedPath p || isUnixStyleRootedPath p
 
-      fsLogger.debug (
-        Log.setMessage "Is {path} rooted? {result}"
-        >> Log.addContext "path" p
-        >> Log.addContext "result" r
-      )
+      // fsLogger.debug (
+      //   Log.setMessage "Is {path} rooted? {result}"
+      //   >> Log.addContext "path" p
+      //   >> Log.addContext "result" r
+      // )
 
       r
 
     member _.GetFullPathShim(f: string) =
       let expanded = Path.FilePathToUri f |> Path.FileUriToLocalPath
 
-      fsLogger.debug (
-        Log.setMessage "{path} expanded to {expanded}"
-        >> Log.addContext "path" f
-        >> Log.addContext "expanded" expanded
-      )
+      // fsLogger.debug (
+      //   Log.setMessage "{path} expanded to {expanded}"
+      //   >> Log.addContext "path" f
+      //   >> Log.addContext "expanded" expanded
+      // )
 
       expanded
 

src/FsAutoComplete.Core/Utils.fs

@@ -10,6 +10,10 @@ open FSharp.Compiler.CodeAnalysis
 open FSharp.UMX
 open FSharp.Compiler.Symbols
 open System.Runtime.CompilerServices
+open LinkDotNet.StringBuilder
+open CommunityToolkit.HighPerformance.Buffers
+open FsAutoComplete.Logging
+open System.Globalization
 
 
 /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
@@ -106,39 +110,52 @@ type Document =
     GetLineText0: int -> string
     GetLineText1: int -> string }
 
+
 /// <summary>
 /// Checks if the file ends with `.fsx` `.fsscript` or `.sketchfs`
 /// </summary>
-let isAScript (fileName: string) =
-  let ext = Path.GetExtension(fileName)
-
-  [ ".fsx"; ".fsscript"; ".sketchfs" ] |> List.exists ((=) ext)
+let inline isAScript (fileName: ReadOnlySpan<char>) =
+  fileName.EndsWith ".fsx"
+  || fileName.EndsWith ".fsscript"
+  || fileName.EndsWith ".sketchfs"
 
 /// <summary>
 /// Checks if the file ends with `.fsi`
 /// </summary>
-let isSignatureFile (fileName: string) = fileName.EndsWith ".fsi"
+let inline isSignatureFile (fileName: ReadOnlySpan<char>) = fileName.EndsWith ".fsi"
 
 /// <summary>
 /// Checks if the file ends with `.fs`
 /// </summary>
-let isFsharpFile (fileName: string) = fileName.EndsWith ".fs"
+let isFsharpFile (fileName: ReadOnlySpan<char>) = fileName.EndsWith ".fs"
+
+let inline internal isFileWithFSharpI fileName =
+  isAScript fileName || isSignatureFile fileName || isFsharpFile fileName
+
 
 /// <summary>
 /// This is a combination of `isAScript`, `isSignatureFile`, and `isFsharpFile`
 /// </summary>
 /// <param name="fileName"></param>
 /// <returns></returns>
-let isFileWithFSharp fileName =
-  [ isAScript; isSignatureFile; isFsharpFile ]
-  |> List.exists (fun f -> f fileName)
-
-let normalizePath (file: string) : string<LocalPath> =
-  if isFileWithFSharp file then
-    let p = Path.GetFullPath file
-    UMX.tag<LocalPath> ((p.Chars 0).ToString().ToLower() + p.Substring(1))
+let inline isFileWithFSharp (fileName: string) = isFileWithFSharpI fileName
+
+let inline internal normalizePathI (file: ReadOnlySpan<char>) : string<LocalPath> =
+  if isFileWithFSharpI file then
+
+    let p = (Path.GetFullPath(StringPool.Shared.GetOrAdd file)).AsSpan()
+    let buffer = SpanOwner<char>.Allocate(p.Length)
+
+    try
+      let written = p.Slice(0, 1).ToLower(buffer.Span, CultureInfo.InvariantCulture)
+      p.Slice(written).CopyTo(buffer.Span.Slice(written))
+      UMX.tag<LocalPath> (StringPool.Shared.GetOrAdd buffer.Span)
+    finally
+      buffer.Dispose()
   else
-    UMX.tag<LocalPath> file
+    UMX.tag<LocalPath> (StringPool.Shared.GetOrAdd file)
+
+let inline normalizePath (file: string) : string<LocalPath> = normalizePathI file
 
 let inline combinePaths path1 (path2: string) =
   Path.Combine(path1, path2.TrimStart [| '\\'; '/' |])
@@ -510,84 +527,116 @@ type Path with
   /// Algorithm from https://stackoverflow.com/a/35734486/433393 for converting file paths to uris,
   /// modified slightly to not rely on the System.Path members because they vary per-platform
   static member FilePathToUri(filePath: string) : string =
-    let filePath, finished =
-      if filePath.Contains "Untitled-" then
-        let rg = System.Text.RegularExpressions.Regex.Match(filePath, @"(Untitled-\d+).fsx")
 
-        if rg.Success then
-          rg.Groups.[1].Value, true
-        else
-          filePath, false
-      else
-        filePath, false
+    let mutable filePathSpan = filePath.AsSpan()
+    let mutable finished = false
+
+    if filePathSpan.Contains("Untitled-", StringComparison.InvariantCultureIgnoreCase) then
+
+      let rg = System.Text.RegularExpressions.Regex.Match(filePath, @"(Untitled-\d+).fsx")
+
+      if rg.Success then
+        filePathSpan <- rg.Groups.[1].Value.AsSpan()
+        finished <- true
 
     if not finished then
-      let uri = System.Text.StringBuilder(filePath.Length)
-
-      for c in filePath do
-        if
-          (c >= 'a' && c <= 'z')
-          || (c >= 'A' && c <= 'Z')
-          || (c >= '0' && c <= '9')
-          || c = '+'
-          || c = '/'
-          || c = '.'
-          || c = '-'
-          || c = '_'
-          || c = '~'
-          || c > '\xFF'
-        then
-          uri.Append(c) |> ignore
-        // handle windows path separator chars.
-        // we _would_ use Path.DirectorySeparator/AltDirectorySeparator, but those vary per-platform and we want this
-        // logic to work cross-platform (for tests)
-        else if c = '\\' then
-          uri.Append('/') |> ignore
-        else
-          uri.Append('%') |> ignore
-          uri.Append((int c).ToString("X2")) |> ignore
+      let uri = ValueStringBuilder()
+
+      try
+        let mutable length = 0
+
+        for c in filePathSpan do
+          if
+            (c >= 'a' && c <= 'z')
+            || (c >= 'A' && c <= 'Z')
+            || (c >= '0' && c <= '9')
+            || c = '+'
+            || c = '/'
+            || c = '.'
+            || c = '-'
+            || c = '_'
+            || c = '~'
+            || c > '\xFF'
+          then
+            uri.Append(c)
+            length <- length + 1
+          // handle windows path separator chars.
+          // we _would_ use Path.DirectorySeparator/AltDirectorySeparator, but those vary per-platform and we want this
+          // logic to work cross-platform (for tests)
+          else if c = '\\' then
+            uri.Append('/')
+            length <- length + 1
+          else
+            uri.Append('%')
+            let buffer = SpanOwner<char>.Allocate 2
+            let mutable out = 0
+
+            try
+              (int c).TryFormat(buffer.Span, &out, "X2") |> ignore
+              uri.Append(buffer.Span)
+            finally
+              buffer.Dispose()
+
+            length <- length + 2
+
+        let file =
+          if uri.Length >= 2 && uri.[0] = '/' && uri.[1] = '/' then // UNC path
+            "file:".AsSpan()
+          else
+            uri.TrimStart('/')
+            "file:///".AsSpan()
+
+        let buffer = SpanOwner<char>.Allocate(file.Length + uri.Length)
+        let finalResult = ValueStringBuilder(buffer.Span)
+
+        try
+          finalResult.Append(file)
+          finalResult.Append(uri.AsSpan())
+
+          StringPool.Shared.GetOrAdd(finalResult.AsSpan())
+        finally
+          finalResult.Dispose()
+          buffer.Dispose()
+      finally
+        uri.Dispose()
 
-      if uri.Length >= 2 && uri.[0] = '/' && uri.[1] = '/' then // UNC path
-        "file:" + uri.ToString()
-      else
-        "file:///" + (uri.ToString()).TrimStart('/')
-    // handle windows path separator chars.
-    // we _would_ use Path.DirectorySeparator/AltDirectorySeparator, but those vary per-platform and we want this
-    // logic to work cross-platform (for tests)
     else
-      "untitled:" + filePath
+      String.Concat("untitled:", filePathSpan)
+
+
+  /// a test that checks if the start of the line is a windows-style drive string, for example
+  /// /d:, /c:, /z:, etc.
+  static member inline internal IsWindowsStyleDriveLetterMatch(s: ReadOnlySpan<char>) =
+    let slice = s.Slice(0, 3)
+    slice.Length = 3 && slice[0] = '/' && Char.IsLetter slice[1] && slice[2] = ':'
+
 
   /// handles unifying the local-path logic for windows and non-windows paths,
   /// without doing a check based on what the current system's OS is.
   static member FileUriToLocalPath(uriString: string) =
-    /// a test that checks if the start of the line is a windows-style drive string, for example
-    /// /d:, /c:, /z:, etc.
-    let isWindowsStyleDriveLetterMatch (s: string) =
-      match s.[0..2].ToCharArray() with
-      | [||]
-      | [| _ |]
-      | [| _; _ |] -> false
-      // 26 windows drive letters allowed, only
-      | [| '/'; driveLetter; ':' |] when Char.IsLetter driveLetter -> true
-      | _ -> false
-
-    let initialLocalPath = Uri(uriString).LocalPath
-
-    let fn =
-      if isWindowsStyleDriveLetterMatch initialLocalPath then
-        let trimmed = initialLocalPath.TrimStart('/')
-
-        let initialDriveLetterCaps =
-          string (System.Char.ToLower trimmed.[0]) + trimmed.[1..]
-
-        initialDriveLetterCaps
+
+    let uriStringSpan = uriString.AsSpan()
+    let mutable initialLocalPath = Uri(uriString).LocalPath.AsSpan()
+    let mutable builder = ValueStringBuilder()
+
+    try
+      let isWindowPath = Path.IsWindowsStyleDriveLetterMatch initialLocalPath
+
+      if isWindowPath then
+        initialLocalPath <- initialLocalPath.TrimStart('/')
+        builder.Append(System.Char.ToLower initialLocalPath.[0])
+        builder.Append(initialLocalPath.Slice(1))
       else
-        initialLocalPath
+        builder.Append initialLocalPath
+
+      if uriStringSpan.StartsWith("untitled:") then
+        builder.Append(".fsx")
+
+      StringPool.Shared.GetOrAdd(builder.AsSpan())
+    finally
+      builder.Dispose()
+
 
-    if uriString.StartsWith "untitled:" then
-      (fn + ".fsx")
-    else
-      fn
 
 let inline debug msg = Printf.kprintf Debug.WriteLine msg
 let inline fail msg = Printf.kprintf Debug.Fail msg

src/FsAutoComplete.Core/paket.references

@@ -17,3 +17,5 @@ Ionide.LanguageServerProtocol
 Ionide.KeepAChangelog.Tasks
 Microsoft.Extensions.Caching.Memory
 Microsoft.CodeAnalysis
+LinkDotNet.StringBuilder
+CommunityToolkit.HighPerformance

src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs

@@ -18,6 +18,7 @@ open System.Reactive
 open System.Reactive.Linq
 open System.Runtime.CompilerServices
 open System.Runtime.InteropServices
+open System.Buffers
 open FsAutoComplete.Adaptive
 
 open FSharp.Control.Reactive
@@ -1264,9 +1265,9 @@ type AdaptiveFSharpLspServer
 
       match result with
       | Error e ->
-        logger.info (
+        logger.error (
           Log.setMessage "Typecheck failed for {file} with {error}"
-          >> Log.addContextDestructured "file" file
+          >> Log.addContextDestructured "file" file.FileName
           >> Log.addContextDestructured "error" e
         )
 

src/FsAutoComplete/paket.references

@@ -26,3 +26,5 @@ FSharp.Data.Adaptive
 Microsoft.Extensions.Caching.Memory
 OpenTelemetry.Exporter.OpenTelemetryProtocol
 Microsoft.CodeAnalysis
+LinkDotNet.StringBuilder
+CommunityToolkit.HighPerformance