From 4edb8322006089c9b7fba7390488c1126f92d598 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 29 Sep 2018 22:09:50 -0700 Subject: [PATCH 1/8] Memory --- .../Interpreter/Ast/AstPythonInterpreter.cs | 17 ++++++++++------ src/Analysis/Engine/Impl/ModuleTable.cs | 2 +- src/Analysis/Engine/Impl/PythonAnalyzer.cs | 6 +++--- .../Impl/Implementation/Server.Completion.cs | 3 ++- .../Impl/Implementation/Server.cs | 20 +++++++------------ 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/Analysis/Engine/Impl/Interpreter/Ast/AstPythonInterpreter.cs b/src/Analysis/Engine/Impl/Interpreter/Ast/AstPythonInterpreter.cs index 90b3fd66d..d52f3c853 100644 --- a/src/Analysis/Engine/Impl/Interpreter/Ast/AstPythonInterpreter.cs +++ b/src/Analysis/Engine/Impl/Interpreter/Ast/AstPythonInterpreter.cs @@ -274,6 +274,7 @@ public void Initialize(PythonAnalyzer state) { } _analyzer = state; + RemoveImportedModules(); if (state != null) { lock (_userSearchPathsLock) { @@ -289,12 +290,7 @@ public void Initialize(PythonAnalyzer state) { private void Analyzer_SearchPathsChanged(object sender, EventArgs e) { lock (_userSearchPathsLock) { - // Remove imported modules from search paths so we will - // import them again. - foreach (var name in _userSearchPathImported.MaybeEnumerate()) { - IPythonModule mod; - _modules.TryRemove(name, out mod); - } + RemoveImportedModules(); _userSearchPathImported = null; _userSearchPathPackages = null; _userSearchPaths = _analyzer.GetSearchPaths(); @@ -302,6 +298,15 @@ private void Analyzer_SearchPathsChanged(object sender, EventArgs e) { ModuleNamesChanged?.Invoke(this, EventArgs.Empty); } + private void RemoveImportedModules() { + lock (_userSearchPathsLock) { + // Remove imported modules from search paths so we will import them again. + foreach (var name in _userSearchPathImported.MaybeEnumerate()) { + _modules.TryRemove(name, out var mod); + } + } + } + public IEnumerable GetModulesNamed(string name) { var usp = GetUserSearchPathPackagesAsync(CancellationToken.None).WaitAndUnwrapExceptions(); var ssp = _factory.GetImportableModulesAsync(CancellationToken.None).WaitAndUnwrapExceptions(); diff --git a/src/Analysis/Engine/Impl/ModuleTable.cs b/src/Analysis/Engine/Impl/ModuleTable.cs index b78925ec5..e3d955a32 100644 --- a/src/Analysis/Engine/Impl/ModuleTable.cs +++ b/src/Analysis/Engine/Impl/ModuleTable.cs @@ -172,7 +172,7 @@ public ModuleReference GetOrAdd(string name) { /// Modules that are already in the table as builtins are replaced or /// removed, but no new modules are added. /// - public void ReInit() { + internal void Reload() { var newNames = new HashSet(_interpreter.GetModuleNames(), StringComparer.Ordinal); foreach (var keyValue in _modules) { diff --git a/src/Analysis/Engine/Impl/PythonAnalyzer.cs b/src/Analysis/Engine/Impl/PythonAnalyzer.cs index f1e328edd..4a734e646 100644 --- a/src/Analysis/Engine/Impl/PythonAnalyzer.cs +++ b/src/Analysis/Engine/Impl/PythonAnalyzer.cs @@ -174,16 +174,16 @@ private async Task LoadKnownTypesAsync(CancellationToken token) { } try { - _interpreterFactory.NotifyImportNamesChanged(); - _modules.ReInit(); + _modules.Reload(); _interpreter.Initialize(this); await LoadKnownTypesAsync(token); - foreach (var mod in _modulesByFilename.Values) { mod.Clear(); mod.EnsureModuleVariables(this); } + + _interpreterFactory.NotifyImportNamesChanged(); } finally { _reloadLock.Release(); } diff --git a/src/LanguageServer/Impl/Implementation/Server.Completion.cs b/src/LanguageServer/Impl/Implementation/Server.Completion.cs index f17856b37..7c7170235 100644 --- a/src/LanguageServer/Impl/Implementation/Server.Completion.cs +++ b/src/LanguageServer/Impl/Implementation/Server.Completion.cs @@ -22,13 +22,14 @@ using Microsoft.Python.LanguageServer.Extensions; using Microsoft.PythonTools; using Microsoft.PythonTools.Analysis; -using Microsoft.PythonTools.Analysis.Infrastructure; using Microsoft.PythonTools.Parsing; using Microsoft.PythonTools.Parsing.Ast; namespace Microsoft.Python.LanguageServer.Implementation { public sealed partial class Server { public override async Task Completion(CompletionParams @params, CancellationToken cancellationToken) { + await ReloadModulesAsync(CancellationToken.None); + var uri = @params.textDocument.uri; ProjectFiles.GetEntry(@params.textDocument, @params._version, out var entry, out var tree); diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 69d98b374..c2cfdf4c7 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -40,12 +40,12 @@ public sealed partial class Server : ServerBase, ILogger, IPythonLanguageServer, /// Implements ability to execute module reload on the analyzer thread /// private sealed class ReloadModulesQueueItem : IAnalyzable { - private readonly PythonAnalyzer _analyzer; + private readonly Server _server; private TaskCompletionSource _tcs = new TaskCompletionSource(); public Task Task => _tcs.Task; - public ReloadModulesQueueItem(PythonAnalyzer analyzer) { - _analyzer = analyzer; + public ReloadModulesQueueItem(Server server) { + _server = server; } public void Analyze(CancellationToken cancel) { if (cancel.IsCancellationRequested) { @@ -54,7 +54,10 @@ public void Analyze(CancellationToken cancel) { var currentTcs = Interlocked.Exchange(ref _tcs, new TaskCompletionSource()); try { - _analyzer.ReloadModulesAsync(cancel).WaitAndUnwrapExceptions(); + _server.Analyzer.ReloadModulesAsync(cancel).WaitAndUnwrapExceptions(); + foreach (var entry in _server.Analyzer.ModulesByFilename) { + _server.AnalysisQueue.Enqueue(entry.Value.ProjectEntry, AnalysisPriority.Normal); + } currentTcs.TrySetResult(true); } catch (OperationCanceledException oce) { currentTcs.TrySetCanceled(oce.CancellationToken); @@ -393,8 +396,6 @@ private async Task DoInitializeAsync(InitializeParams @params, CancellationToken SetSearchPaths(@params.initializationOptions.searchPaths); SetTypeStubSearchPaths(@params.initializationOptions.typeStubSearchPaths); - - Analyzer.Interpreter.ModuleNamesChanged += Interpreter_ModuleNamesChanged; } private void DisplayStartupInfo() { @@ -405,13 +406,6 @@ private void DisplayStartupInfo() { : Resources.InitializingForPythonInterpreter.FormatInvariant(Analyzer.InterpreterFactory.Configuration.InterpreterPath)); } - private void Interpreter_ModuleNamesChanged(object sender, EventArgs e) { - Analyzer.Modules.ReInit(); - foreach (var entry in Analyzer.ModulesByFilename) { - AnalysisQueue.Enqueue(entry.Value.ProjectEntry, AnalysisPriority.Normal); - } - } - private T ActivateObject(string assemblyName, string typeName, Dictionary properties) { if (string.IsNullOrEmpty(assemblyName) || string.IsNullOrEmpty(typeName)) { return default(T); From 4170b3bd7e04d83a6c7af31a6f54ed80f3edbd0d Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 29 Sep 2018 22:45:20 -0700 Subject: [PATCH 2/8] Modules reload --- src/LanguageServer/Impl/Implementation/Server.Completion.cs | 2 -- src/LanguageServer/Impl/Implementation/Server.cs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/LanguageServer/Impl/Implementation/Server.Completion.cs b/src/LanguageServer/Impl/Implementation/Server.Completion.cs index 7c7170235..922692933 100644 --- a/src/LanguageServer/Impl/Implementation/Server.Completion.cs +++ b/src/LanguageServer/Impl/Implementation/Server.Completion.cs @@ -28,8 +28,6 @@ namespace Microsoft.Python.LanguageServer.Implementation { public sealed partial class Server { public override async Task Completion(CompletionParams @params, CancellationToken cancellationToken) { - await ReloadModulesAsync(CancellationToken.None); - var uri = @params.textDocument.uri; ProjectFiles.GetEntry(@params.textDocument, @params._version, out var entry, out var tree); diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 96b10974c..c73f3f127 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -379,7 +379,7 @@ private async Task DoInitializeAsync(InitializeParams @params, CancellationToken _analysisUpdates = @params.initializationOptions.analysisUpdates; Analyzer.EnableDiagnostics = _clientCaps?.python?.liveLinting ?? false; - _reloadModulesQueueItem = new ReloadModulesQueueItem(Analyzer); + _reloadModulesQueueItem = new ReloadModulesQueueItem(this); if (@params.initializationOptions.displayOptions != null) { DisplayOptions = @params.initializationOptions.displayOptions; From 49affa2b5918876406f086c22bbc864166f484dc Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Wed, 3 Oct 2018 10:34:03 -0700 Subject: [PATCH 3/8] Change error back to message --- src/.editorconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/.editorconfig b/src/.editorconfig index 25bc515da..d2a6ae077 100644 --- a/src/.editorconfig +++ b/src/.editorconfig @@ -29,7 +29,7 @@ csharp_space_between_parentheses = None csharp_space_after_cast = false -csharp_style_inlined_variable_declaration = true:error +csharp_style_inlined_variable_declaration = true:message csharp_space_before_open_square_brackets = false From d7c06d9d98fed33bd3f5ab8ee480c71414bf4a01 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 5 Oct 2018 10:52:21 -0700 Subject: [PATCH 4/8] Fix protocol type id --- src/Analysis/Engine/Impl/Values/Protocols.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Analysis/Engine/Impl/Values/Protocols.cs b/src/Analysis/Engine/Impl/Values/Protocols.cs index def71b7d7..090f301c2 100644 --- a/src/Analysis/Engine/Impl/Values/Protocols.cs +++ b/src/Analysis/Engine/Impl/Values/Protocols.cs @@ -389,9 +389,7 @@ class TupleProtocol : IterableProtocol { public TupleProtocol(ProtocolInfo self, IEnumerable values) : base(self, AnalysisSet.UnionAll(values)) { _values = values.Select(s => s.AsUnion(1)).ToArray(); - - var argTypes = _values.SelectMany(e => e.Select(v => v is IHasQualifiedName qn ? qn.FullyQualifiedName : v.ShortDescription)); - Name = "tuple[{0}]".FormatInvariant(string.Join(", ", argTypes)); + Name = "tuple[{0}]".FormatInvariant(string.Join(", ", _values.Select(v => v.GetShortDescriptions()))); } protected override void EnsureMembers(IDictionary members) { @@ -419,6 +417,7 @@ public override IAnalysisSet GetIndex(Node node, AnalysisUnit unit, IAnalysisSet } public override string Name { get; } + public override BuiltinTypeId TypeId => BuiltinTypeId.Tuple; public override IEnumerable> GetRichDescription() { if (_values.Any()) { From da98c14761ced5d8fbd13b6dd1bac89e9561db85 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 5 Oct 2018 10:54:12 -0700 Subject: [PATCH 5/8] Fix type ids --- src/Analysis/Engine/Impl/Values/Protocols.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Analysis/Engine/Impl/Values/Protocols.cs b/src/Analysis/Engine/Impl/Values/Protocols.cs index 090f301c2..91dd8c131 100644 --- a/src/Analysis/Engine/Impl/Values/Protocols.cs +++ b/src/Analysis/Engine/Impl/Values/Protocols.cs @@ -436,7 +436,7 @@ public override IEnumerable> GetRichDescription() { } } - protected override bool Equals(Protocol other) => + protected override bool Equals(Protocol other) => other is TupleProtocol tp && _values.Zip(tp._values, (x, y) => x.SetEquals(y)).All(b => b); @@ -523,6 +523,7 @@ public override IAnalysisSet GetIndex(Node node, AnalysisUnit unit, IAnalysisSet } public override string Name => "dict"; + public override BuiltinTypeId TypeId => BuiltinTypeId.Dict; public override IEnumerable> GetRichDescription() { if (_valueType.Any()) { @@ -575,14 +576,13 @@ protected override void EnsureMembers(IDictionary members) } public override string Name => "generator"; - + public override BuiltinTypeId TypeId => BuiltinTypeId.Generator; + public IAnalysisSet Yielded => _yielded; public IAnalysisSet Sent { get; } public IAnalysisSet Returned { get; } - public override IAnalysisSet GetReturnForYieldFrom(Node node, AnalysisUnit unit) { - return Returned; - } + public override IAnalysisSet GetReturnForYieldFrom(Node node, AnalysisUnit unit) => Returned; public override IEnumerable> GetRichDescription() { if (_yielded.Any() || Sent.Any() || Returned.Any()) { From b84d9a5c682a691ad2bf1c01f922e76565a17b4a Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 5 Oct 2018 12:35:18 -0700 Subject: [PATCH 6/8] Don't ignore empty value sets (unknown types) --- src/Analysis/Engine/Impl/Values/Protocols.cs | 37 +++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/Analysis/Engine/Impl/Values/Protocols.cs b/src/Analysis/Engine/Impl/Values/Protocols.cs index 91dd8c131..897dbd1b4 100644 --- a/src/Analysis/Engine/Impl/Values/Protocols.cs +++ b/src/Analysis/Engine/Impl/Values/Protocols.cs @@ -17,6 +17,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using Microsoft.PythonTools.Analysis.Infrastructure; using Microsoft.PythonTools.Interpreter; using Microsoft.PythonTools.Parsing; @@ -389,7 +390,41 @@ class TupleProtocol : IterableProtocol { public TupleProtocol(ProtocolInfo self, IEnumerable values) : base(self, AnalysisSet.UnionAll(values)) { _values = values.Select(s => s.AsUnion(1)).ToArray(); - Name = "tuple[{0}]".FormatInvariant(string.Join(", ", _values.Select(v => v.GetShortDescriptions()))); + Name = GetNameFromValues(); + } + + private string GetNameFromValues() { + // Enumerate manually since SelectMany drops empty/unknown values + var sb = new StringBuilder("tuple["); + for (var i = 0; i < _values.Length; i++) { + if (i > 0) { + sb.Append(", "); + } + sb.Append(GetParameterString(_values[i].ToArray())); + } + sb.Append(']'); + return sb.ToString(); + } + + private string GetParameterString(AnalysisValue[] sets) { + if (sets.Length == 0) { + return "?"; + } + var sb = new StringBuilder(); + if (sets.Length > 1) { + sb.Append('['); + } + for (var i = 0; i < sets.Length; i++) { + if (i > 0) { + sb.Append(", "); + } + var desc = sets[i] is IHasQualifiedName qn ? qn.FullyQualifiedName : sets[i].ShortDescription; + sb.Append(desc); + } + if (sets.Length > 1) { + sb.Append(']'); + } + return sb.ToString(); } protected override void EnsureMembers(IDictionary members) { From ca6ee8aca57d5d295eb22135abeb60e651a379d0 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 5 Oct 2018 13:03:37 -0700 Subject: [PATCH 7/8] PR feedback --- .../Extensions/StringBuilderExtensions.cs | 10 +++++- src/Analysis/Engine/Impl/Values/Protocols.cs | 32 ++++++++----------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringBuilderExtensions.cs b/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringBuilderExtensions.cs index 8c22853b0..11b9f2fde 100644 --- a/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringBuilderExtensions.cs +++ b/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringBuilderExtensions.cs @@ -37,7 +37,15 @@ public static StringBuilder EnsureEndsWithWhiteSpace(this StringBuilder sb, int if (whiteSpaceCount > 0) { sb.Append(' ', whiteSpaceCount); } - + + return sb; + } + + public static StringBuilder AppendIf(this StringBuilder sb, bool condition, string value) { + if (condition) { + sb.Append(value); + } + return sb; } } diff --git a/src/Analysis/Engine/Impl/Values/Protocols.cs b/src/Analysis/Engine/Impl/Values/Protocols.cs index 897dbd1b4..90c50f59f 100644 --- a/src/Analysis/Engine/Impl/Values/Protocols.cs +++ b/src/Analysis/Engine/Impl/Values/Protocols.cs @@ -397,34 +397,30 @@ private string GetNameFromValues() { // Enumerate manually since SelectMany drops empty/unknown values var sb = new StringBuilder("tuple["); for (var i = 0; i < _values.Length; i++) { - if (i > 0) { - sb.Append(", "); - } - sb.Append(GetParameterString(_values[i].ToArray())); + sb.AppendIf(i > 0, ", "); + AppendParameterString(sb, _values[i].ToArray()); } sb.Append(']'); return sb.ToString(); } - private string GetParameterString(AnalysisValue[] sets) { + private void AppendParameterString(StringBuilder sb, AnalysisValue[] sets) { if (sets.Length == 0) { - return "?"; + sb.Append('?'); + return; } - var sb = new StringBuilder(); - if (sets.Length > 1) { - sb.Append('['); + + if (sets.Length == 1) { + sb.Append(sets[0] is IHasQualifiedName qn ? qn.FullyQualifiedName : sets[0].ShortDescription); + return; } + + sb.Append('['); for (var i = 0; i < sets.Length; i++) { - if (i > 0) { - sb.Append(", "); - } - var desc = sets[i] is IHasQualifiedName qn ? qn.FullyQualifiedName : sets[i].ShortDescription; - sb.Append(desc); + sb.AppendIf(i > 0, ", "); + sb.Append(sets[i] is IHasQualifiedName qn ? qn.FullyQualifiedName : sets[i].ShortDescription); } - if (sets.Length > 1) { - sb.Append(']'); - } - return sb.ToString(); + sb.Append(']'); } protected override void EnsureMembers(IDictionary members) { From 96ae47905745bca79eb70c5af65ca1b18de725f0 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 5 Oct 2018 13:07:09 -0700 Subject: [PATCH 8/8] Even simpler --- src/Analysis/Engine/Impl/Values/Protocols.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Analysis/Engine/Impl/Values/Protocols.cs b/src/Analysis/Engine/Impl/Values/Protocols.cs index 90c50f59f..7808f11b4 100644 --- a/src/Analysis/Engine/Impl/Values/Protocols.cs +++ b/src/Analysis/Engine/Impl/Values/Protocols.cs @@ -410,17 +410,12 @@ private void AppendParameterString(StringBuilder sb, AnalysisValue[] sets) { return; } - if (sets.Length == 1) { - sb.Append(sets[0] is IHasQualifiedName qn ? qn.FullyQualifiedName : sets[0].ShortDescription); - return; - } - - sb.Append('['); + sb.AppendIf(sets.Length > 1, "["); for (var i = 0; i < sets.Length; i++) { sb.AppendIf(i > 0, ", "); sb.Append(sets[i] is IHasQualifiedName qn ? qn.FullyQualifiedName : sets[i].ShortDescription); } - sb.Append(']'); + sb.AppendIf(sets.Length > 1, "]"); } protected override void EnsureMembers(IDictionary members) {