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

Remove all references to dropped modules and force GC on reload #1402

Merged
merged 14 commits into from Aug 5, 2019

Conversation

@jakebailey
Copy link
Member

commented Aug 2, 2019

Fixes #1408.
For #1298.

  • Pulls ModuleWalker changes from #1372, as stub merging was adding things to places it shouldn't.
  • Drops more ASTs (compiled/stubs).
  • Prevents adding references to the builtin module (i.e. references to object).
  • Resets both the typeshed and main module resolution.
  • Clears the builtin module's AST map for a future reanalysis.
    • This needs to be reexamined; our method of modifying the scope of functions during calls to them is not right and needs to be fixed separately.
  • Forces a GC/compact after a reload (to actually lower the memory usage after reloading)
  • Drops [Flag] on ModuleType.
  • Removes some dead code that would have caused reloads (but didn't need to).

@jakebailey jakebailey merged commit 352fecc into microsoft:master Aug 5, 2019

1 check passed

license/cla All CLA requirements met.
Details

@jakebailey jakebailey deleted the jakebailey:reload-memory-usage branch Aug 5, 2019


lock (_syncObj) {
_forceGCOnNextSession = _forceGCOnNextSession || full;
_forceGCOnNextSession = true;

This comment has been minimized.

Copy link
@AlexanderSher

AlexanderSher Aug 5, 2019

Contributor

If we reload TypeshedResolution and BuiltinsModule, there is no reason for a split bleow. _analysisEntries should be completely cleared, _dependencyResolver can be thrown away.

This comment has been minimized.

Copy link
@jakebailey

jakebailey Aug 5, 2019

Author Member

Sort of; the way we create the builtins module is really contrived and currently can only be created once when the interpreter object is created. What I had to do here is only a partial fix; we don't have a way to do a complete reload including the builtin module, so I kept the existing code and allowed it to remove more.

I'd like for this to be fixed in the future so we can have a "real" reload that drops everything but file contents and does a complete restart, but I think that'd be a bigger change.

MikhailArkhipov added a commit that referenced this pull request Aug 8, 2019

Multiple fixes and additional tests for persistence (#1372)
* Remove old qualified name

* Node storage

* Class and scope to use AST map

* Library analysis

* Fix SO

* Keep small AST with imports

* AST reduction

* Final field

* Initial

* Reload

* Ignore post-final requests

* Drop AST

* Remove local variables

* Test fixes

* Fix overload match

* Tests

* Add locks

* Remove local variables

* Drop file content to save memory

* Cache PEP hints

* Recreate AST

* Fix specialization

* Fix locations

* usings

* Test fixes

* Add options to keep data in memory

* Fix test

* Fix lambda parameters

* Fix argument set
Fix global scope node

* Fix overload doc

* Fix stub merge errors

* Fix async issues

* Undo some changes

* Fix test

* Fix race condition

* Partial

* Models and views

* Restore log null checks

* Fix merge conflict

* Fix merge issue

* Null check

* Partial

* Partial

* Partial

* Fix test

* Partial

* Partial

* First test

* Baseline comparison

* Builtins

* Partial

* Type fixes

* Fix type names, part I

* Qualified name

* Properly write variables

* Partial

* Construct module from model

* Test

* Variable creations

* Factories

* Factories

* Split construction

* Restore

* Save builtins

* Test passes

* Qualified name

* Better export detection

* Test fixes

* More consistent qualified names

* Sys test

* Demo

* Complete sys write/read

* Partial

* Partial

* Test staility

* Perf bug

* Baseline, remove debug code, deactivate db

* Test fixes

* Test fix

* Simplify a bit

* Baselines and use : separator

* Baselines

* PR feedback

* Merge master

* Remove registry reference

* PR feedback

* PR feedback

* Restore persistence + update test

* Better handle persistent module in dependencies

* Undo

* Add location converter abstraction

* Store member location

* Fix merge issue

* Basic locations test

* Navigation

* Add test

* Update baselines

* Type restore - initial

* Remove debug code

* Partial

* Fix stub merge

* Various model fixes

* Improve module handling

* Qualified name improvements

* Fix unbound case

* Improve stub merge

* Fix qualified names of typing

* Handle stub-only modules

* Add tests for io, re and sys

* Better handle named tuple

* Handle module circular references

* Handle else in platform and version clauses + handle it in symbol collector

* Formatting

* Fix walk of multi-level if statement

* Unify package search in imports

* Fix tests

* Undo change

* Port changes from dbtype

* Partial

* Named tuple support

* Baseline updates

* Debug code

* Support types

* Properly compare class member declaring type

* Nested classes and functions persistence

* Undo debug

* Fix numpy restore

* Baselines

* Fix tests

* Update AnyStr test reflecting changes to AnyStr behavior

* Exclude baselines from git diff

* Fix gitattr

* Move git setting to root

* Try no path

* Add RDT count to the analysis_complete event (#1396)

* Add RDT count to the analysis_complete event

* Use property instead of method

* Test fixes

* Undo change

* Additional stub merge fixes

* Baseline updates

* Fix goto def

* Protect specific type creation

* Track documentaton source

* More reliable tests + simplification

* Prevent crashes when the AST happens to be null (#1405)

* 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

* Typo

* Cleanup

* Basic classification

* Fix merge error

* Module unique id fixes

* Stricted check to save analysis

* Revert "Fix tests"

This reverts commit 247a8c3.

* Revert "Unify package search in imports"

This reverts commit 67fed10.

* Don't clear scope variables with inner classes

* Fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.