-
Notifications
You must be signed in to change notification settings - Fork 416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework completion resolution #2126
Changes from all commits
322d568
99e805d
3c95dbd
185e9f5
127e565
afcd88a
21faedb
f81588e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
BenchmarkDotNet.Artifacts/* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
using BenchmarkDotNet.Attributes; | ||
using BenchmarkDotNet.Diagnostics.Windows.Configs; | ||
using OmniSharp.Models.v1.Completion; | ||
using OmniSharp.Roslyn.CSharp.Services.Completion; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using TestUtility; | ||
|
||
namespace OmniSharp.Benchmarks | ||
{ | ||
[EtwProfiler] | ||
public class ImportCompletionBenchmarks : HostBase | ||
{ | ||
public CompletionRequest Request { get; set; } = null!; | ||
|
||
[GlobalSetup] | ||
public async Task SetupAsync() | ||
{ | ||
Setup(new KeyValuePair<string, string>("RoslynExtensionsOptions:EnableImportCompletion", "true")); | ||
|
||
var builder = new StringBuilder(); | ||
|
||
builder.AppendLine("class Base"); | ||
builder.AppendLine("{"); | ||
builder.AppendLine(" void M()"); | ||
builder.AppendLine(" {"); | ||
builder.AppendLine(" $$"); | ||
builder.AppendLine(" }"); | ||
builder.AppendLine("}"); | ||
|
||
const string FileName = "ImportCompletionTest.cs"; | ||
var file = new TestFile(FileName, builder.ToString()); | ||
OmniSharpTestHost.AddFilesToWorkspace(file); | ||
|
||
var point = file.Content.GetPointFromPosition(); | ||
|
||
Request = new() | ||
{ | ||
CompletionTrigger = CompletionTriggerKind.Invoked, | ||
Line = point.Line, | ||
Column = point.Offset, | ||
FileName = FileName | ||
}; | ||
|
||
// Trigger completion once to ensure that all the runs have a warmed-up server, with full completions loaded | ||
CompletionResponse completions; | ||
do | ||
{ | ||
completions = await ImportCompletionListAsync(); | ||
} while (!completions.Items.Any(i => i.Label == "Console")); | ||
|
||
} | ||
|
||
[Benchmark] | ||
public async Task<CompletionResponse> ImportCompletionListAsync() | ||
{ | ||
var handler = OmniSharpTestHost.GetRequestHandler<CompletionService>(OmniSharpEndpoints.Completion); | ||
return await handler.Handle(Request); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<OutputType>Exe</OutputType> | ||
<TargetFramework>net472</TargetFramework> | ||
<Nullable>enable</Nullable> | ||
<LangVersion>9.0</LangVersion> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="BenchmarkDotNet" Version="0.12.1" /> | ||
<PackageReference Include="BenchmarkDotNet.Diagnostics.Windows" Version="0.12.1" /> | ||
<PackageReference Include="xunit" Version="2.4.1" /> | ||
<PackageReference Include="xunit.abstractions" Version="2.0.3" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\OmniSharp.Roslyn.CSharp\OmniSharp.Roslyn.CSharp.csproj" /> | ||
<ProjectReference Include="..\..\tests\TestUtility\TestUtility.csproj" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
using BenchmarkDotNet.Attributes; | ||
using BenchmarkDotNet.Diagnostics.Windows.Configs; | ||
using OmniSharp.Models.v1.Completion; | ||
using OmniSharp.Roslyn.CSharp.Services.Completion; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
using System.Threading.Tasks; | ||
using TestUtility; | ||
|
||
namespace OmniSharp.Benchmarks | ||
{ | ||
[EtwProfiler] | ||
public class OverrideCompletionBenchmarks : HostBase | ||
{ | ||
[Params(10, 100, 250, 500)] | ||
public int NumOverrides { get; set; } | ||
|
||
public CompletionRequest Request { get; set; } = null!; | ||
|
||
[GlobalSetup] | ||
public async Task SetupAsync() | ||
{ | ||
Setup(new KeyValuePair<string, string>( "RoslynExtensionsOptions:EnableImportCompletion", "true" )); | ||
|
||
var builder = new StringBuilder(); | ||
|
||
builder.AppendLine("namespace N1"); | ||
builder.AppendLine("{"); | ||
builder.AppendLine(" using System.Collections.Generic;"); | ||
builder.AppendLine(" class Base"); | ||
builder.AppendLine(" {"); | ||
for (int i = 0; i < NumOverrides; i++) | ||
{ | ||
builder.AppendLine($" public virtual Dictionary<string, string> M{i}(List<string> s) {{ return null; }}"); | ||
} | ||
builder.AppendLine(" }"); | ||
builder.AppendLine("}"); | ||
builder.AppendLine("namespace N2 : N1.Base"); | ||
builder.AppendLine("{"); | ||
builder.AppendLine(" class Derived"); | ||
builder.AppendLine(" {"); | ||
builder.AppendLine(" override $$"); | ||
builder.AppendLine(" }"); | ||
builder.AppendLine("}"); | ||
|
||
const string FileName = "OverrideTest.cs"; | ||
var file = new TestFile(FileName, builder.ToString()); | ||
OmniSharpTestHost.AddFilesToWorkspace(file); | ||
|
||
var point = file.Content.GetPointFromPosition(); | ||
|
||
Request = new() | ||
{ | ||
CompletionTrigger = OmniSharp.Models.v1.Completion.CompletionTriggerKind.Invoked, | ||
Line = point.Line, | ||
Column = point.Offset, | ||
FileName = FileName | ||
}; | ||
|
||
// Trigger completion once to ensure that all the runs have a warmed-up server | ||
await OverrideCompletionAsync(); | ||
} | ||
|
||
[Benchmark] | ||
public async Task<CompletionResponse> OverrideCompletionAsync() | ||
{ | ||
var handler = OmniSharpTestHost.GetRequestHandler<CompletionService>(OmniSharpEndpoints.Completion); | ||
return await handler.Handle(Request); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
using BenchmarkDotNet.Running; | ||
using Microsoft.Extensions.Configuration; | ||
using OmniSharp.Benchmarks; | ||
using OmniSharp.Utilities; | ||
using System.Collections.Generic; | ||
using TestUtility; | ||
|
||
BenchmarkRunner.Run(typeof(OverrideCompletionBenchmarks).Assembly); | ||
|
||
namespace OmniSharp.Benchmarks | ||
{ | ||
public abstract class HostBase : DisposableObject | ||
{ | ||
protected OmniSharpTestHost OmniSharpTestHost { get; set; } = null!; | ||
|
||
protected override void DisposeCore(bool disposing) | ||
{ | ||
OmniSharpTestHost.Dispose(); | ||
} | ||
|
||
public void Setup(params KeyValuePair<string, string>[]? configuration) | ||
{ | ||
var builder = new Microsoft.Extensions.Configuration.ConfigurationBuilder().AddInMemoryCollection(configuration); | ||
OmniSharpTestHost = OmniSharpTestHost.Create(configurationData: builder.Build()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,7 +112,7 @@ private static void VerifyEnumsInSync(Type enum1, Type enum2) | |
Debug.Assert(lspValues.Length == modelValues.Length); | ||
for (int i = 0; i < lspValues.Length; i++) | ||
{ | ||
Debug.Assert((int?)lspValues.GetValue(i) == (int?)modelValues.GetValue(i)); | ||
Debug.Assert((int)lspValues.GetValue(i) == (int)modelValues.GetValue(i)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, if it's int? the assert fails because these are ints, not nullable ints :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh 😅 |
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for adding this... It has been long long long overdue ✨