Skip to content

Fix sys::Cache for ref-counted values which caused memory leaks in SourceKit #10822

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

Merged
merged 1 commit into from
Jul 10, 2017

Conversation

benlangmuir
Copy link
Contributor

  • Explanation: Our libcache implementation of swift::sys::Cache was broken for ref-counted values (which are used by e.g. the SourceKit ASTManager). This was causing us to never release ASTs cached by SourceKit even when the underlying libcache purged itself under memory pressure.
  • Scope: Affects memory usage under high system load on macOS.
  • Radar: rdar://problem/21619189
  • Testing: Unit tests added.

…urceKit

Our libcache implementation of swift::sys::Cache was broken for
ref-counted values (which are used by e.g. the SourceKit ASTManager).
It would always `retain(value)` in `set(key, value)`, but under the hood
libcache shares values, so we would only get one `release(value)` if the
same value was used across multiple keys, or if the same value *and* key
were set multiple times.

This was causing us to never release ASTs cached by SourceKit even when
the underlying libcache purged itself under memory pressure.

rdar://problem/21619189
@benlangmuir
Copy link
Contributor Author

@swift-ci please test

@benlangmuir benlangmuir merged commit 732f90d into swiftlang:swift-4.0-branch Jul 10, 2017
@benlangmuir benlangmuir deleted the cache-release-4 branch July 10, 2017 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant