Skip to content

Commit

Permalink
More VS cleanup (#15954)
Browse files Browse the repository at this point in the history
Option -> ValueOption in some places
More cancellable tasks
Removed creating asyncs + running in parallel on lightweight CPU bound AIO find operations
Something else.
  • Loading branch information
vzarytovskii authored Oct 10, 2023
1 parent f2b8306 commit 5b94f74
Show file tree
Hide file tree
Showing 41 changed files with 1,056 additions and 810 deletions.
3 changes: 1 addition & 2 deletions global.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
{
"sdk": {
"version": "8.0.100-rc.1.23455.8",
"allowPrerelease": true,
"rollForward": "latestMajor"
"allowPrerelease": true
},
"tools": {
"dotnet": "8.0.100-rc.1.23455.8",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,11 @@ open Microsoft.CodeAnalysis.Classification
[<AutoOpen>]
module BraceCompletionSessionProviderHelpers =

let tryGetCaretPoint (buffer: ITextBuffer) (session: IBraceCompletionSession) =
let point =
session.TextView.Caret.Position.Point.GetPoint(buffer, PositionAffinity.Predecessor)
let inline tryGetCaretPoint (buffer: ITextBuffer) (session: IBraceCompletionSession) =
ValueOption.ofNullable (session.TextView.Caret.Position.Point.GetPoint(buffer, PositionAffinity.Predecessor))

if point.HasValue then Some point.Value else None

let tryGetCaretPosition session =
session |> tryGetCaretPoint session.SubjectBuffer
let inline tryGetCaretPosition (session: IBraceCompletionSession) =
tryGetCaretPoint session.SubjectBuffer session

let tryInsertAdditionalBracePair (session: IBraceCompletionSession) openingChar closingChar =
let sourceCode = session.TextView.TextSnapshot
Expand Down Expand Up @@ -134,13 +131,13 @@ type BraceCompletionSession
// exit without setting the closing point which will take us off the stack
edit.Cancel()
undo.Cancel()
None
ValueNone
else
Some(edit.Apply()) // FIXME: perhaps, it should be ApplyAndLogExceptions()
ValueSome(edit.Apply()) // FIXME: perhaps, it should be ApplyAndLogExceptions()

match nextSnapshot with
| None -> ()
| Some (nextSnapshot) ->
| ValueNone -> ()
| ValueSome (nextSnapshot) ->

let beforePoint = beforeTrackingPoint.GetPoint(textView.TextSnapshot)

Expand Down Expand Up @@ -185,7 +182,7 @@ type BraceCompletionSession

if closingSnapshotPoint.Position > 0 then
match tryGetCaretPosition this with
| Some caretPos when not (this.HasNoForwardTyping(caretPos, closingSnapshotPoint.Subtract(1))) -> true
| ValueSome caretPos when not (this.HasNoForwardTyping(caretPos, closingSnapshotPoint.Subtract(1))) -> true
| _ -> false
else
false
Expand Down Expand Up @@ -265,7 +262,7 @@ type BraceCompletionSession

match caretPos with
// ensure that we are within the session before clearing
| Some caretPos when
| ValueSome caretPos when
caretPos.Position < closingSnapshotPoint.Position
&& closingSnapshotPoint.Position > 0
->
Expand Down Expand Up @@ -310,7 +307,7 @@ type BraceCompletionSession

member this.PostReturn() =
match tryGetCaretPosition this with
| Some caretPos ->
| ValueSome caretPos ->
let closingSnapshotPoint = closingPoint.GetPoint(subjectBuffer.CurrentSnapshot)

if
Expand Down Expand Up @@ -580,13 +577,13 @@ type BraceCompletionSessionProvider [<ImportingConstructor>]
maybe {
let! document =
openingPoint.Snapshot.GetOpenDocumentInCurrentContextWithChanges()
|> Option.ofObj
|> ValueOption.ofObj

let! sessionFactory = document.TryGetLanguageService<IEditorBraceCompletionSessionFactory>()

let! session =
sessionFactory.TryCreateSession(document, openingPoint.Position, openingBrace, CancellationToken.None)
|> Option.ofObj
|> ValueOption.ofObj

let undoHistory =
undoManager.GetTextBufferUndoManager(textView.TextBuffer).TextBufferUndoHistory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type internal FSharpClassificationService [<ImportingConstructor>] () =
ClassificationTypeNames.Text

match RoslynHelpers.TryFSharpRangeToTextSpan(text, tok.Range) with
| Some span -> result.Add(ClassifiedSpan(TextSpan(textSpan.Start + span.Start, span.Length), spanKind))
| ValueSome span -> result.Add(ClassifiedSpan(TextSpan(textSpan.Start + span.Start, span.Length), spanKind))
| _ -> ()

let flags =
Expand All @@ -79,8 +79,8 @@ type internal FSharpClassificationService [<ImportingConstructor>] () =
=
for item in items do
match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) with
| None -> ()
| Some span ->
| ValueNone -> ()
| ValueSome span ->
let span =
match item.Type with
| SemanticClassificationType.Printf -> span
Expand Down Expand Up @@ -111,7 +111,7 @@ type internal FSharpClassificationService [<ImportingConstructor>] () =
| true, items -> items
| _ ->
let items = ResizeArray()
lookup.[dataItem.Range.StartLine] <- items
lookup[dataItem.Range.StartLine] <- items
items

items.Add dataItem
Expand All @@ -120,8 +120,29 @@ type internal FSharpClassificationService [<ImportingConstructor>] () =

lookup :> IReadOnlyDictionary<_, _>

let semanticClassificationCache =
new DocumentCache<SemanticClassificationLookup>("fsharp-semantic-classification-cache")
static let itemToSemanticClassificationLookup (d: SemanticClassificationItem array) =
let lookup = Dictionary<int, ResizeArray<SemanticClassificationItem>>()

for item in d do
let items =
let startLine = item.Range.StartLine

match lookup.TryGetValue startLine with
| true, items -> items
| _ ->
let items = ResizeArray()
lookup[startLine] <- items
items

items.Add item

lookup :> IReadOnlyDictionary<_, _>

static let unopenedDocumentsSemanticClassificationCache =
new DocumentCache<SemanticClassificationLookup>("fsharp-unopened-documents-semantic-classification-cache", 5.)

static let openedDocumentsSemanticClassificationCache =
new DocumentCache<SemanticClassificationLookup>("fsharp-opened-documents-semantic-classification-cache", 2.)

interface IFSharpClassificationService with
// Do not perform classification if we don't have project options (#defines matter)
Expand Down Expand Up @@ -197,7 +218,7 @@ type internal FSharpClassificationService [<ImportingConstructor>] () =
let isOpenDocument = document.Project.Solution.Workspace.IsDocumentOpen document.Id

if not isOpenDocument then
match! semanticClassificationCache.TryGetValueAsync document with
match! unopenedDocumentsSemanticClassificationCache.TryGetValueAsync document with
| ValueSome classificationDataLookup ->
let eventProps: (string * obj) array =
[|
Expand All @@ -212,7 +233,7 @@ type internal FSharpClassificationService [<ImportingConstructor>] () =
TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.AddSemanticCalssifications, eventProps)

addSemanticClassificationByLookup sourceText textSpan classificationDataLookup result
| _ ->
| ValueNone ->
let eventProps: (string * obj) array =
[|
"context.document.project.id", document.Project.Id.Id.ToString()
Expand All @@ -226,29 +247,53 @@ type internal FSharpClassificationService [<ImportingConstructor>] () =
TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.AddSemanticCalssifications, eventProps)

let! classificationData = document.GetFSharpSemanticClassificationAsync(nameof (FSharpClassificationService))

let classificationDataLookup = toSemanticClassificationLookup classificationData
do! semanticClassificationCache.SetAsync(document, classificationDataLookup)
do! unopenedDocumentsSemanticClassificationCache.SetAsync(document, classificationDataLookup)
addSemanticClassificationByLookup sourceText textSpan classificationDataLookup result
else
let eventProps: (string * obj) array =
[|
"context.document.project.id", document.Project.Id.Id.ToString()
"context.document.id", document.Id.Id.ToString()
"isOpenDocument", isOpenDocument
"textSpanLength", textSpan.Length
"cacheHit", false
|]

use _eventDuration =
TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.AddSemanticCalssifications, eventProps)
match! openedDocumentsSemanticClassificationCache.TryGetValueAsync document with
| ValueSome classificationDataLookup ->
let eventProps: (string * obj) array =
[|
"context.document.project.id", document.Project.Id.Id.ToString()
"context.document.id", document.Id.Id.ToString()
"isOpenDocument", isOpenDocument
"textSpanLength", textSpan.Length
"cacheHit", true
|]

use _eventDuration =
TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.AddSemanticCalssifications, eventProps)

addSemanticClassificationByLookup sourceText textSpan classificationDataLookup result
| ValueNone ->

let eventProps: (string * obj) array =
[|
"context.document.project.id", document.Project.Id.Id.ToString()
"context.document.id", document.Id.Id.ToString()
"isOpenDocument", isOpenDocument
"textSpanLength", textSpan.Length
"cacheHit", false
|]

use _eventDuration =
TelemetryReporter.ReportSingleEventWithDuration(TelemetryEvents.AddSemanticCalssifications, eventProps)

let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync(nameof (IFSharpClassificationService))

let targetRange =
RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText)

let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync(nameof (IFSharpClassificationService))
let classificationData = checkResults.GetSemanticClassification(Some targetRange)

let targetRange =
RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText)
if classificationData.Length > 0 then
let classificationDataLookup = itemToSemanticClassificationLookup classificationData
do! unopenedDocumentsSemanticClassificationCache.SetAsync(document, classificationDataLookup)

let classificationData = checkResults.GetSemanticClassification(Some targetRange)
addSemanticClassification sourceText textSpan classificationData result
addSemanticClassification sourceText textSpan classificationData result
}
|> CancellableTask.startAsTask cancellationToken

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ type internal AddOpenCodeFixProvider [<ImportingConstructor>] (assemblyContentPr

let entities =
assemblyContentProvider.GetAllEntitiesInProjectAndReferencedAssemblies checkResults
|> List.collect (fun s ->
[
|> Array.collect (fun s ->
[|
yield s.TopRequireQualifiedAccessParent, s.AutoOpenParent, s.Namespace, s.CleanedIdents
if isAttribute then
let lastIdent = s.CleanedIdents.[s.CleanedIdents.Length - 1]
Expand All @@ -204,7 +204,7 @@ type internal AddOpenCodeFixProvider [<ImportingConstructor>] (assemblyContentPr
s.Namespace,
s.CleanedIdents
|> Array.replace (s.CleanedIdents.Length - 1) (lastIdent.Substring(0, lastIdent.Length - 9))
])
|])

ParsedInput.GetLongIdentAt parseResults.ParseTree unresolvedIdentRange.End
|> Option.bind (fun longIdent ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ open Microsoft.VisualStudio.TextManager.Interop
open Microsoft.VisualStudio.LanguageServices
open Microsoft.VisualStudio.Utilities
open FSharp.Compiler.EditorServices
open CancellableTasks.CancellableTaskBuilder
open CancellableTasks

type internal XmlDocCommandFilter(wpfTextView: IWpfTextView, filePath: string, workspace: VisualStudioWorkspace) =

Expand Down Expand Up @@ -67,7 +69,12 @@ type internal XmlDocCommandFilter(wpfTextView: IWpfTextView, filePath: string, w
let! document = getLastDocument ()
let! cancellationToken = Async.CancellationToken |> liftAsync
let! sourceText = document.GetTextAsync(cancellationToken)
let! parseResults = document.GetFSharpParseResultsAsync(nameof (XmlDocCommandFilter)) |> liftAsync

let! parseResults =
document.GetFSharpParseResultsAsync(nameof (XmlDocCommandFilter))
|> CancellableTask.start cancellationToken
|> Async.AwaitTask
|> liftAsync

let xmlDocables =
XmlDocParser.GetXmlDocables(sourceText.ToFSharpSourceText(), parseResults.ParseTree)
Expand Down
13 changes: 7 additions & 6 deletions vsintegration/src/FSharp.Editor/Common/CancellableTasks.fs
Original file line number Diff line number Diff line change
Expand Up @@ -451,13 +451,13 @@ module CancellableTasks =

/// <summary>
/// Builds a cancellableTask using computation expression syntax.
/// Default behaviour when binding (v)options is to return a cacnelled task.
/// Default behaviour when binding (v)options is to return a cancelled task.
/// </summary>
let foregroundCancellableTask = CancellableTaskBuilder(false)

/// <summary>
/// Builds a cancellableTask using computation expression syntax which switches to execute on a background thread if not already doing so.
/// Default behaviour when binding (v)options is to return a cacnelled task.
/// Default behaviour when binding (v)options is to return a cancelled task.
/// </summary>
let cancellableTask = CancellableTaskBuilder(true)

Expand Down Expand Up @@ -1096,17 +1096,18 @@ module CancellableTasks =
let inline whenAll (tasks: CancellableTask<'a> seq) =
cancellableTask {
let! ct = getCancellationToken ()
return! Task.WhenAll (seq { for task in tasks do yield start ct task })
let tasks = seq { for task in tasks do yield start ct task }
return! Task.WhenAll (tasks)
}

let inline whenAllTasks (tasks: CancellableTask seq) =
cancellableTask {
let! ct = getCancellationToken ()
return! Task.WhenAll (seq { for task in tasks do yield startTask ct task })
let tasks = seq { for task in tasks do yield startTask ct task }
return! Task.WhenAll (tasks)
}

let inline ignore (ctask: CancellableTask<_>) =
ctask |> toUnit
let inline ignore ([<InlineIfLambda>] ctask: CancellableTask<_>) = toUnit ctask

/// <exclude />
[<AutoOpen>]
Expand Down
13 changes: 2 additions & 11 deletions vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,8 @@ type Solution with
// It's crucial to normalize file path here (specificaly, remove relative parts),
// otherwise Roslyn does not find documents.
self.GetDocumentIdsWithFilePath(Path.GetFullPath filePath)
|> Seq.tryHead
|> Option.map (fun docId -> self.GetDocument docId)

/// Try to find the document corresponding to the provided filepath and ProjectId within this solution
member self.TryGetDocumentFromPath(filePath, projId: ProjectId) =
// It's crucial to normalize file path here (specificaly, remove relative parts),
// otherwise Roslyn does not find documents.
self.GetDocumentIdsWithFilePath(Path.GetFullPath filePath)
|> Seq.filter (fun x -> x.ProjectId = projId)
|> Seq.tryHead
|> Option.map (fun docId -> self.GetDocument docId)
|> ImmutableArray.tryHeadV
|> ValueOption.map (fun docId -> self.GetDocument docId)

/// Try to get a project inside the solution using the project's id
member self.TryGetProject(projId: ProjectId) =
Expand Down
3 changes: 3 additions & 0 deletions vsintegration/src/FSharp.Editor/Common/DocumentCache.fs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ type DocumentCache<'Value when 'Value: not struct>(name: string, ?cacheItemPolic
let policy =
defaultArg cacheItemPolicy (CacheItemPolicy(SlidingExpiration = (TimeSpan.FromSeconds defaultSlidingExpiration)))

new(name: string, slidingExpirationSeconds: float) =
new DocumentCache<'Value>(name, CacheItemPolicy(SlidingExpiration = (TimeSpan.FromSeconds slidingExpirationSeconds)))

member _.TryGetValueAsync(doc: Document) =
cancellableTask {
let! ct = CancellableTask.getCancellationToken ()
Expand Down
Loading

0 comments on commit 5b94f74

Please sign in to comment.