Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Commit 352fecc

Browse files
jakebaileyjakebailey
authored
Remove all references to dropped modules and force GC on reload (#1402)
* First attempt at ensuring moduled dropped during reload are GC'd * Make sure AST is set after content is cleared * More tests passing * ModuleWalker updates * Also don't merge against specialized modules, do builtin _astMap reset before reanalyzing instead of after any analysis * Don't call dispose directly, it's done elsewhere and has no effect * Force GC via normal session instead of waiting for a hardcoded time * Un-refactor function since it's not being called externally anymore * Formatting/usings * PR feedback * Undo IsBuiltin, some things continued to be referenced with that changed * Move reload logic down into PythonAnalyzer, remove added public inferface items * Mode AddReference to PythonType as override * Remove dead code, make all ResetAnalyzer calls full
1 parent b423ed8 commit 352fecc

File tree

7 files changed

+154
-99
lines changed

7 files changed

+154
-99
lines changed

‎src/Analysis/Ast/Impl/Analyzer/Definitions/IPythonAnalyzer.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,11 @@ public interface IPythonAnalyzer {
5656
/// </summary>
5757
IReadOnlyList<DiagnosticsEntry> LintModule(IPythonModule module);
5858

59+
5960
/// <summary>
60-
/// Removes all the modules from the analysis, except Typeshed and builtin
61+
/// Removes all the modules from the analysis and restarts it, including stubs.
6162
/// </summary>
62-
void ResetAnalyzer();
63+
Task ResetAnalyzer();
6364

6465
/// <summary>
6566
/// Returns list of currently loaded modules.

‎src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs‎

Lines changed: 84 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ private void MergeStub() {
228228
return;
229229
}
230230

231+
var builtins = Module.Interpreter.ModuleResolution.BuiltinsModule;
232+
231233
// Note that scrape can pick up more functions than the stub contains
232234
// Or the stub can have definitions that scraping had missed. Therefore
233235
// merge is the combination of the two with the documentation coming
@@ -240,53 +242,101 @@ private void MergeStub() {
240242

241243
var sourceVar = Eval.GlobalScope.Variables[v.Name];
242244
var sourceType = sourceVar?.Value.GetPythonType();
243-
245+
244246
// If stub says 'Any' but we have better type, keep the current type.
245247
if (!IsStubBetterType(sourceType, stubType)) {
246-
continue;;
248+
continue;
247249
}
248250

249-
// If types are the classes, merge members.
250-
// Otherwise, replace type from one from the stub.
251-
if (sourceType is PythonClassType cls && Module.Equals(cls.DeclaringModule)) {
252-
// If class exists and belongs to this module, add or replace
253-
// its members with ones from the stub, preserving documentation.
254-
// Don't augment types that do not come from this module.
255-
foreach (var name in stubType.GetMemberNames()) {
256-
var stubMember = stubType.GetMember(name);
257-
var member = cls.GetMember(name);
258-
259-
var memberType = member?.GetPythonType();
260-
var stubMemberType = stubMember.GetPythonType();
261-
if (!IsStubBetterType(memberType, stubMemberType)) {
262-
continue;
251+
// If type does not exist in module, but exists in stub, declare it unless it is an import.
252+
// If types are the classes, merge members. Otherwise, replace type from one from the stub.
253+
switch (sourceType) {
254+
case null:
255+
// Nothing in sources, but there is type in the stub. Declare it.
256+
if (v.Source == VariableSource.Declaration) {
257+
Eval.DeclareVariable(v.Name, v.Value, v.Source);
263258
}
264-
265-
// Get documentation from the current type, if any, since stubs
266-
// typically do not contain documentation while scraped code does.
267-
memberType?.TransferDocumentationAndLocation(stubMemberType);
268-
cls.AddMember(name, stubMember, overwrite: true);
269-
}
270-
} else {
271-
// Re-declare variable with the data from the stub unless member is a module.
272-
// Modules members that are modules should remain as they are, i.e. os.path
273-
// should remain library with its own stub attached.
274-
if (!(stubType is IPythonModule)) {
275-
sourceType.TransferDocumentationAndLocation(stubType);
276-
// TODO: choose best type between the scrape and the stub. Stub probably should always win.
277-
var source = Eval.CurrentScope.Variables[v.Name]?.Source ?? VariableSource.Declaration;
278-
Eval.DeclareVariable(v.Name, v.Value, source);
279-
}
259+
break;
260+
261+
case PythonClassType cls when Module.Equals(cls.DeclaringModule):
262+
// If class exists and belongs to this module, add or replace
263+
// its members with ones from the stub, preserving documentation.
264+
// Don't augment types that do not come from this module.
265+
// Do not replace __class__ since it has to match class type and we are not
266+
// replacing class type, we are only merging members.
267+
foreach (var name in stubType.GetMemberNames().Except(new[] { "__class__", "__base__", "__bases__", "__mro__", "mro" })) {
268+
var stubMember = stubType.GetMember(name);
269+
var member = cls.GetMember(name);
270+
271+
var memberType = member?.GetPythonType();
272+
var stubMemberType = stubMember.GetPythonType();
273+
274+
if (builtins.Equals(memberType?.DeclaringModule) || builtins.Equals(stubMemberType?.DeclaringModule)) {
275+
continue; // Leave builtins alone.
276+
}
277+
278+
if (memberType?.DeclaringModule is SpecializedModule || stubMemberType?.DeclaringModule is SpecializedModule) {
279+
continue; // Leave specialized modules like typing alone.
280+
}
281+
282+
if (!IsStubBetterType(memberType, stubMemberType)) {
283+
continue;
284+
}
285+
286+
// Get documentation from the current type, if any, since stubs
287+
// typically do not contain documentation while scraped code does.
288+
TransferDocumentationAndLocation(memberType, stubMemberType);
289+
cls.AddMember(name, stubMember, overwrite: true);
290+
}
291+
break;
292+
293+
case IPythonModule _:
294+
// We do not re-declare modules.
295+
break;
296+
297+
default:
298+
// Re-declare variable with the data from the stub unless member is a module.
299+
// Modules members that are modules should remain as they are, i.e. os.path
300+
// should remain library with its own stub attached.
301+
var stubModule = stubType.DeclaringModule;
302+
if (!(stubType is IPythonModule) && !builtins.Equals(stubModule)) {
303+
TransferDocumentationAndLocation(sourceType, stubType);
304+
// TODO: choose best type between the scrape and the stub. Stub probably should always win.
305+
var source = Eval.CurrentScope.Variables[v.Name]?.Source ?? v.Source;
306+
Eval.DeclareVariable(v.Name, v.Value, source);
307+
}
308+
break;
280309
}
281310
}
282311
}
283312

284313
private static bool IsStubBetterType(IPythonType sourceType, IPythonType stubType) {
285-
// If stub says 'Any' but we have better type, keep the current type.
286314
if (stubType.IsUnknown()) {
315+
// Do not use worse types than what is in the module.
287316
return false;
288317
}
289-
return sourceType.IsUnknown() || !(stubType.DeclaringModule is TypingModule) || stubType.Name != "Any";
318+
if (sourceType.IsUnknown()) {
319+
return true; // Anything is better than unknowns.
320+
}
321+
// If stub says 'Any' but we have better type, keep the current type.
322+
return !(stubType.DeclaringModule is TypingModule) || stubType.Name != "Any";
323+
}
324+
325+
private static void TransferDocumentationAndLocation(IPythonType s, IPythonType d) {
326+
if (s.IsUnknown()) {
327+
return; // Do not transfer location of unknowns
328+
}
329+
// Documentation and location are always get transferred from module type
330+
// to the stub type and never the other way around. This makes sure that
331+
// we show documentation from the original module and goto definition
332+
// navigates to the module source and not to the stub.
333+
if (s != d && s is PythonType src && d is PythonType dst) {
334+
var documentation = src.Documentation;
335+
if (!string.IsNullOrEmpty(documentation)) {
336+
dst.SetDocumentation(documentation);
337+
}
338+
dst.Location = src.Location;
339+
}
290340
}
291341
}
292342
}

‎src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
using System.Collections.Generic;
1818
using System.Diagnostics;
1919
using System.Linq;
20-
using System.Text;
2120
using System.Threading;
2221
using System.Threading.Tasks;
2322
using Microsoft.Python.Analysis.Caching;
@@ -49,6 +48,7 @@ public sealed class PythonAnalyzer : IPythonAnalyzer, IDisposable {
4948
private int _version;
5049
private PythonAnalyzerSession _currentSession;
5150
private PythonAnalyzerSession _nextSession;
51+
private bool _forceGCOnNextSession;
5252

5353
public PythonAnalyzer(IServiceManager services, string cacheFolderPath = null) {
5454
_services = services;
@@ -196,16 +196,27 @@ public IReadOnlyList<DiagnosticsEntry> LintModule(IPythonModule module) {
196196
return optionsProvider?.Options?.LintingEnabled == false ? Array.Empty<DiagnosticsEntry>() : result;
197197
}
198198

199-
public void ResetAnalyzer() {
199+
public async Task ResetAnalyzer() {
200+
var interpreter = _services.GetService<IPythonInterpreter>();
201+
var builtins = interpreter.ModuleResolution.BuiltinsModule;
202+
builtins.SetAst(builtins.Analysis.Ast);
203+
204+
await interpreter.TypeshedResolution.ReloadAsync();
205+
await interpreter.ModuleResolution.ReloadAsync();
206+
200207
lock (_syncObj) {
201-
_analysisEntries.Split(kvp => kvp.Key.IsTypeshed || kvp.Value.Module is IBuiltinsPythonModule, out var entriesToPreserve, out var entriesToRemove);
208+
_forceGCOnNextSession = true;
209+
210+
_analysisEntries.Split(kvp => kvp.Value.Module is IBuiltinsPythonModule, out var entriesToPreserve, out var entriesToRemove);
202211
_analysisEntries.Clear();
203212
foreach (var (key, entry) in entriesToPreserve) {
204213
_analysisEntries.Add(key, entry);
205214
}
206215

207216
_dependencyResolver.RemoveKeys(entriesToRemove.Select(e => e.Key));
208217
}
218+
219+
_services.GetService<IRunningDocumentTable>().ReloadAll();
209220
}
210221

211222
public IReadOnlyList<IPythonModule> LoadedModules {
@@ -218,7 +229,7 @@ public IReadOnlyList<IPythonModule> LoadedModules {
218229

219230
public event EventHandler<AnalysisCompleteEventArgs> AnalysisComplete;
220231

221-
internal void RaiseAnalysisComplete(int moduleCount, double msElapsed)
232+
internal void RaiseAnalysisComplete(int moduleCount, double msElapsed)
222233
=> AnalysisComplete?.Invoke(this, new AnalysisCompleteEventArgs(moduleCount, msElapsed));
223234

224235
private void AnalyzeDocument(in AnalysisModuleKey key, in PythonAnalyzerEntry entry, in ImmutableArray<AnalysisModuleKey> dependencies) {
@@ -241,7 +252,7 @@ private void AnalyzeDocument(in AnalysisModuleKey key, in PythonAnalyzerEntry en
241252
session.Start(true);
242253
}
243254
}
244-
255+
245256
private bool TryCreateSession(in int graphVersion, in PythonAnalyzerEntry entry, out PythonAnalyzerSession session) {
246257
var analyzeUserModuleOutOfOrder = false;
247258
lock (_syncObj) {
@@ -276,7 +287,7 @@ private bool TryCreateSession(in int graphVersion, in PythonAnalyzerEntry entry,
276287
session = null;
277288
return false;
278289
}
279-
290+
280291
if (_currentSession.IsCompleted) {
281292
_currentSession = session = CreateSession(walker, null);
282293
return true;
@@ -301,14 +312,21 @@ private void StartNextSession(Task task) {
301312
session.Start(false);
302313
}
303314

304-
private PythonAnalyzerSession CreateSession(in IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker, in PythonAnalyzerEntry entry)
305-
=> new PythonAnalyzerSession(_services, _progress, _analysisCompleteEvent, _startNextSession, _disposeToken.CancellationToken, walker, _version, entry);
315+
private PythonAnalyzerSession CreateSession(in IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker, in PythonAnalyzerEntry entry) {
316+
bool forceGC;
317+
lock (_syncObj) {
318+
forceGC = _forceGCOnNextSession;
319+
_forceGCOnNextSession = false;
320+
}
321+
322+
return new PythonAnalyzerSession(_services, _progress, _analysisCompleteEvent, _startNextSession, _disposeToken.CancellationToken, walker, _version, entry, forceGC: forceGC);
323+
}
306324

307325
private void LoadMissingDocuments(IPythonInterpreter interpreter, ImmutableArray<AnalysisModuleKey> missingKeys) {
308326
if (missingKeys.Count == 0) {
309327
return;
310328
}
311-
329+
312330
var foundKeys = ImmutableArray<AnalysisModuleKey>.Empty;
313331
foreach (var missingKey in missingKeys) {
314332
lock (_syncObj) {

‎src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ internal sealed class PythonAnalyzerSession {
4545
private readonly IProgressReporter _progress;
4646
private readonly IPythonAnalyzer _analyzer;
4747
private readonly ILogger _log;
48-
private readonly ITelemetryService _telemetry;
48+
private readonly bool _forceGC;
4949

5050
private State _state;
5151
private bool _isCanceled;
@@ -69,7 +69,8 @@ public PythonAnalyzerSession(IServiceManager services,
6969
CancellationToken analyzerCancellationToken,
7070
IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker,
7171
int version,
72-
PythonAnalyzerEntry entry) {
72+
PythonAnalyzerEntry entry,
73+
bool forceGC = false) {
7374

7475
_services = services;
7576
_analysisCompleteEvent = analysisCompleteEvent;
@@ -80,11 +81,11 @@ public PythonAnalyzerSession(IServiceManager services,
8081
_walker = walker;
8182
_entry = entry;
8283
_state = State.NotStarted;
84+
_forceGC = forceGC;
8385

8486
_diagnosticsService = _services.GetService<IDiagnosticsService>();
8587
_analyzer = _services.GetService<IPythonAnalyzer>();
8688
_log = _services.GetService<ILogger>();
87-
_telemetry = _services.GetService<ITelemetryService>();
8889
_progress = progress;
8990
}
9091

@@ -158,11 +159,12 @@ private async Task StartAsync() {
158159

159160
var elapsed = stopWatch.Elapsed.TotalMilliseconds;
160161
LogResults(_log, elapsed, originalRemaining, remaining, Version);
161-
ForceGCIfNeeded(originalRemaining, remaining);
162+
ForceGCIfNeeded(_log, originalRemaining, remaining, _forceGC);
162163
}
163164

164-
private static void ForceGCIfNeeded(int originalRemaining, int remaining) {
165-
if (originalRemaining - remaining > 1000) {
165+
private static void ForceGCIfNeeded(ILogger logger, int originalRemaining, int remaining, bool force) {
166+
if (force || originalRemaining - remaining > 1000) {
167+
logger?.Log(TraceEventType.Verbose, "Forcing full garbage collection and heap compaction.");
166168
GCSettings.LargeObjectHeapCompactionMode = GCLargeObjectHeapCompactionMode.CompactOnce;
167169
GC.Collect();
168170
}
@@ -229,7 +231,7 @@ private async Task<int> AnalyzeAffectedEntriesAsync(Stopwatch stopWatch) {
229231
}
230232

231233

232-
private bool IsAnalyzedLibraryInLoop(IDependencyChainNode<PythonAnalyzerEntry> node)
234+
private bool IsAnalyzedLibraryInLoop(IDependencyChainNode<PythonAnalyzerEntry> node)
233235
=> !node.HasMissingDependencies && node.Value.IsAnalyzedLibrary(_walker.Version) && node.IsWalkedWithDependencies && node.IsValidVersion;
234236

235237
private Task StartAnalysis(IDependencyChainNode<PythonAnalyzerEntry> node, AsyncCountdownEvent ace, Stopwatch stopWatch)
@@ -368,10 +370,22 @@ private void LogException(IPythonModule module, Exception exception) {
368370
}
369371

370372
private IDocumentAnalysis CreateAnalysis(IDependencyChainNode<PythonAnalyzerEntry> node, IDocument document, PythonAst ast, int version, ModuleWalker walker, bool isCanceled) {
373+
var canHaveLibraryAnalysis = false;
374+
375+
// Don't try to drop builtins; it causes issues elsewhere.
376+
// We probably want the builtin module's AST and other info for evaluation.
377+
switch (document.ModuleType) {
378+
case ModuleType.Library:
379+
case ModuleType.Stub:
380+
case ModuleType.Compiled:
381+
canHaveLibraryAnalysis = true;
382+
break;
383+
}
384+
371385
var createLibraryAnalysis = !isCanceled &&
372386
node != null &&
373387
!node.HasMissingDependencies &&
374-
document.ModuleType == ModuleType.Library &&
388+
canHaveLibraryAnalysis &&
375389
!document.IsOpen &&
376390
node.HasOnlyWalkedDependencies &&
377391
node.IsValidVersion;

‎src/Analysis/Ast/Impl/Modules/Definitions/ModuleType.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@
1313
// See the Apache Version 2.0 License for specific language governing
1414
// permissions and limitations under the License.
1515

16-
using System;
17-
1816
namespace Microsoft.Python.Analysis.Modules {
19-
[Flags]
2017
public enum ModuleType {
2118
/// <summary>
2219
/// Module is user file in the workspace.

‎src/Analysis/Ast/Impl/Types/PythonType.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
using System.Collections.Generic;
1818
using System.Diagnostics;
1919
using System.Linq;
20+
using Microsoft.Python.Analysis.Modules;
2021
using Microsoft.Python.Analysis.Values;
2122
using Microsoft.Python.Core.Diagnostics;
2223

@@ -51,6 +52,14 @@ private PythonType(string name, Location location, BuiltinTypeId typeId) : base(
5152

5253
#region ILocatedMember
5354
public override PythonMemberType MemberType => _typeId.GetMemberId();
55+
56+
public override void AddReference(Location location) {
57+
if (DeclaringModule == null || DeclaringModule.ModuleType == ModuleType.Builtins) {
58+
return;
59+
}
60+
61+
base.AddReference(location);
62+
}
5463
#endregion
5564

5665
#region IPythonType

0 commit comments

Comments
 (0)