Skip to content
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

No-op SymbolSearchUpdateEngine on non-Windows OS #45764

Merged
merged 5 commits into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ internal partial class SymbolSearchUpdateEngine : ISymbolSearchUpdateEngine
private readonly ConcurrentDictionary<string, IAddReferenceDatabaseWrapper> _sourceToDatabase =
new ConcurrentDictionary<string, IAddReferenceDatabaseWrapper>();

/// <summary>
/// Don't call directly. Use <see cref="SymbolSearchUpdateEngineFactory"/> instead.
/// </summary>
public SymbolSearchUpdateEngine(
ISymbolSearchLogService logService,
ISymbolSearchProgressService progressService)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@

#nullable enable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Remote;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.SymbolSearch
{
Expand All @@ -19,6 +18,9 @@ namespace Microsoft.CodeAnalysis.SymbolSearch
/// implementation produces an engine that will run in-process. Implementations at
/// other layers can behave differently (for example, running the engine out-of-process).
/// </summary>
/// <remarks>
/// This returns an No-op engine on non-Windows OS, because the backing storage depends on Windows APIs.
/// </remarks>
internal static partial class SymbolSearchUpdateEngineFactory
{
public static async Task<ISymbolSearchUpdateEngine> CreateEngineAsync(
Expand All @@ -36,10 +38,37 @@ public static async Task<ISymbolSearchUpdateEngine> CreateEngineAsync(
}

// Couldn't go out of proc. Just do everything inside the current process.
return new SymbolSearchUpdateEngine(logService, progressService);
return CreateEngineInProcess(logService, progressService);
}

private sealed partial class RemoteUpdateEngine : ISymbolSearchUpdateEngine
/// <summary>
/// This returns a No-op engine if called on non-Windows OS, because the backing storage depends on Windows APIs.
/// </summary>
public static ISymbolSearchUpdateEngine CreateEngineInProcess(
ISymbolSearchLogService logService,
ISymbolSearchProgressService progressService)
{
return RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
? new SymbolSearchUpdateEngine(logService, progressService)
: (ISymbolSearchUpdateEngine)new NoOpUpdateEngine();
}

private sealed class NoOpUpdateEngine : ISymbolSearchUpdateEngine
{
public Task<ImmutableArray<PackageWithAssemblyResult>> FindPackagesWithAssemblyAsync(string source, string assemblyName, CancellationToken cancellationToken)
=> Task.FromResult(ImmutableArray<PackageWithAssemblyResult>.Empty);

public Task<ImmutableArray<PackageWithTypeResult>> FindPackagesWithTypeAsync(string source, string name, int arity, CancellationToken cancellationToken)
=> Task.FromResult(ImmutableArray<PackageWithTypeResult>.Empty);

public Task<ImmutableArray<ReferenceAssemblyWithTypeResult>> FindReferenceAssembliesWithTypeAsync(string name, int arity, CancellationToken cancellationToken)
=> Task.FromResult(ImmutableArray<ReferenceAssemblyWithTypeResult>.Empty);

public Task UpdateContinuouslyAsync(string sourceName, string localSettingsDirectory)
=> Task.CompletedTask;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separate file please.

Copy link
Member Author

@genlu genlu Jul 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, we have 3 private types defined in this file, NoOpUpdateEngine, RemoteUpdateEngine and CallbackObject, their implementation are simple only used within this file. I'd prefer to keep them here, unless separate files is absolutely required by our code style.


private sealed class RemoteUpdateEngine : ISymbolSearchUpdateEngine
{
private readonly SemaphoreSlim _gate = new SemaphoreSlim(initialCount: 1);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@ namespace Microsoft.CodeAnalysis.Remote
{
internal partial class RemoteSymbolSearchUpdateEngine : ServiceBase, IRemoteSymbolSearchUpdateEngine, ISymbolSearchLogService, ISymbolSearchProgressService
{
private readonly SymbolSearchUpdateEngine _updateEngine;
private readonly ISymbolSearchUpdateEngine _updateEngine;

public RemoteSymbolSearchUpdateEngine(
Stream stream, IServiceProvider serviceProvider)
: base(serviceProvider, stream)
{
_updateEngine = new SymbolSearchUpdateEngine(
logService: this, progressService: this);
_updateEngine = SymbolSearchUpdateEngineFactory.CreateEngineInProcess(logService: this, progressService: this);

StartService();
}
Expand All @@ -32,7 +31,7 @@ public Task UpdateContinuouslyAsync(string sourceName, string localSettingsDirec
// In non-test scenarios, we're not cancellable. Our lifetime will simply be that
// of the OOP process itself. i.e. when it goes away, it will just tear down our
// update-loop itself. So we don't need any additional controls over it.
return _updateEngine.UpdateContinuouslyAsync(sourceName, localSettingsDirectory, CancellationToken.None);
return _updateEngine.UpdateContinuouslyAsync(sourceName, localSettingsDirectory);
}, CancellationToken.None);
}

Expand Down