Slice source text instead of copying it line by line for outlining - #20443
Slice source text instead of copying it line by line for outlining#20443xperiandri wants to merge 6 commits into
Conversation
The editor's block structure built the sourceLines array for Structure.getOutliningRanges by calling ToString() per line, allocating a fresh string for the entire file on every outlining pass, once per keystroke. getOutliningRanges now takes ReadOnlyMemory<char>[] and slices the already-materialized source text once (SourceText.GetLinesAsMemory()) instead. ReadOnlySpanCharExtensions in illib mirrors the existing Ordinal string helpers so span call sites read the same way string call sites do. A local recursive function closing over a ReadOnlySpan<char>-typed sibling cannot be compiled - the CLR disallows instantiating FSharpFunc<ReadOnlySpan<char>, _> as a closure field (FS0412) - so commentTypeOf moves to module scope, next to the CommentType it classifies. StructureTests.fs slices its own lines the same way at the call site, and FSharp.Compiler.Service.Tests needs a direct System.Memory PackageReference: FSharp.Compiler.Service's own reference to it is only transitive through the net472 ProjectReference's SetTargetFramework override, mirroring the FSharp.Core pin already in this project for the same reason. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CommentList kept a copy of every comment line next to its line number, but the number alone identifies the line in the source array the function already holds, and only the first and last lines of a group are ever read back to compute the fold's columns. Store the numbers and index the source at the end, so grouping comments allocates no tuple per line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CheckCodeFormatting flagged illib.fsi for a stray space before the colon in the ReadOnlySpanCharExtensions signatures; dotnet fantomas fixes it mechanically, no signature changes. check_release_notes also requires an entry for changes under vsintegration/src. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Plain_Build_Windows and Plain_Build_Linux both failed with NU1510: on the .NET Core inner build System.Memory ships with the framework, and NuGet's package-pruning check treats an unconditional explicit PackageReference to it as an error. The pin is only needed on net472, where FSharp.Compiler.Service's own PackageReference to System.Memory doesn't flow through the netstandard2.0 SetTargetFramework override. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
BenchmarkBenchmarkDotNet, both versions of the changed code copied verbatim into a standalone harness (the compiler-internal helpers they use are Inputs are two real files from this repo:
"Build + scan" is what one outlining pass pays outside the AST walk, and the editor pays it per keystroke. On the large file that is 4.4 MB of garbage per pass before, 1.0 MB after, a 77% reduction; on the small file 76%. Where the allocations went:
Variance is high on the multi-modal rows (the harness reports bimodal distributions on the scan benchmarks, and the full-pass rows carry a wide StdDev), so treat the time ratios as order-of-magnitude rather than precise. The allocation numbers are exact and are the substance of the change. Two caveats worth stating plainly:
|
This comment has been minimized.
This comment has been minimized.
Wrap commentTypeOf's doc comment in <summary>, move the FS0412 rationale into <remarks>, and reference the types through <see cref> rather than inline code spans. Use the shorthand lambda for the whitespace check, per review suggestion. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
🔍 Tooling Safety Check — Affects-Build-Infra, Affects-Restore
|
Description
The editor's block structure built the
sourceLinesarray forStructure.getOutliningRangesby callingToString()on every line, allocating a fresh string for the whole file on each outlining pass, that is once per keystroke.getOutliningRangesnow takesReadOnlyMemory<char>[]and the editor slices the already materialized source text once through a newSourceText.GetLinesAsMemory()helper. Inside the function, comment detection trims and classifies lines over spans instead of building trimmed strings, and comment groups track line numbers only, since only the first and last lines of a group are ever read back to compute the fold's columns.ReadOnlySpanCharExtensionsinillibmirrors the existing ordinalStringhelpers so span call sites read the same way.This is a breaking change for
FSharp.Compiler.Serviceconsumers that callgetOutliningRangeswith astring[]; the migration isArray.map (fun line -> line.AsMemory()).Fixes # (no issue)
Checklist
StructureTests(39 cases) exercise every fold kind through the new signature; the public surface area baseline is updated.docs/release-notes/.FSharp.Compiler.Service/11.0.100.md, Breaking Changes.🤖 Generated with Claude Code