Skip to content

Commit

Permalink
Invalidate resolution of analysisOptions/sourceFactory changes.
Browse files Browse the repository at this point in the history 
R=brianwilkerson@google.com
BUG=

Review URL: https://codereview.chromium.org//1167483004
  • Loading branch information
@scheglov
scheglov committed Jun 5, 2015
1 parent 787e9fc commit 811877d
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 79 deletions.
27 changes: 13 additions & 14 deletions pkg/analysis_server/test/domain_analysis_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ class AnalysisDomainTest extends AbstractAnalysisTest {
}
}

test_packageMapDependencies() {
test_packageMapDependencies() async {
// Prepare a source file that has errors because it refers to an unknown
// package.
String pkgFile = '/packages/pkgA/libA.dart';
Expand All @@ -305,19 +305,18 @@ f(A a) {
packageMapProvider.dependencies.add(pkgDependency);
// Create project and wait for analysis
createProject();
return waitForTasksFinished().then((_) {
expect(filesErrors[testFile], isNotEmpty);
// Add the package to the package map and tickle the package dependency.
packageMapProvider.packageMap = {
'pkgA': [resourceProvider.getResource('/packages/pkgA')]
};
resourceProvider.modifyFile(pkgDependency, 'new contents');
// Give the server time to notice the file has changed, then let
// analysis complete. There should now be no error.
return pumpEventQueue().then((_) => waitForTasksFinished()).then((_) {
expect(filesErrors[testFile], isEmpty);
});
});
await waitForTasksFinished();
expect(filesErrors[testFile], isNotEmpty);
// Add the package to the package map and tickle the package dependency.
packageMapProvider.packageMap = {
'pkgA': [resourceProvider.getResource('/packages/pkgA')]
};
resourceProvider.modifyFile(pkgDependency, 'new contents');
// Give the server time to notice the file has changed, then let
// analysis complete. There should now be no error.
await pumpEventQueue();
await waitForTasksFinished();
expect(filesErrors[testFile], isEmpty);
}

test_setRoots_packages() {
Expand Down
58 changes: 4 additions & 54 deletions pkg/analyzer/lib/src/context/context.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ class AnalysisContextImpl implements InternalAnalysisContext {
this._options.lint = options.lint;
this._options.preserveComments = options.preserveComments;
if (needsRecompute) {
_invalidateAllLocalResolutionInformation(false);
dartWorkManager.onAnalysisOptionsChanged();
}
}

Expand Down Expand Up @@ -366,7 +366,7 @@ class AnalysisContextImpl implements InternalAnalysisContext {
factory.context = this;
_sourceFactory = factory;
_cache = createCacheFromSourceFactory(factory);
_invalidateAllLocalResolutionInformation(true);
dartWorkManager.onSourceFactoryChanged();
}

@override
Expand Down Expand Up @@ -1542,36 +1542,6 @@ class AnalysisContextImpl implements InternalAnalysisContext {
}
}

/**
* Invalidate all of the resolution results computed by this context. The flag
* [invalidateUris] should be `true` if the cached results of converting URIs
* to source files should also be invalidated.
*/
void _invalidateAllLocalResolutionInformation(bool invalidateUris) {
HashMap<Source, List<Source>> oldPartMap =
new HashMap<Source, List<Source>>();
// TODO(brianwilkerson) Implement this
// MapIterator<AnalysisTarget, cache.CacheEntry> iterator =
// _privatePartition.iterator();
// while (iterator.moveNext()) {
// AnalysisTarget target = iterator.key;
// cache.CacheEntry entry = iterator.value;
// if (entry is HtmlEntry) {
// HtmlEntry htmlEntry = entry;
// htmlEntry.invalidateAllResolutionInformation(invalidateUris);
// iterator.value = htmlEntry;
// _workManager.add(target, SourcePriority.HTML);
// } else if (entry is DartEntry) {
// DartEntry dartEntry = entry;
// oldPartMap[target] = dartEntry.getValue(DartEntry.INCLUDED_PARTS);
// dartEntry.invalidateAllResolutionInformation(invalidateUris);
// iterator.value = dartEntry;
// _workManager.add(target, _computePriority(dartEntry));
// }
// }
_removeFromPartsUsingMap(oldPartMap);
}

/**
* Log the given debugging [message].
*/
Expand All @@ -1591,28 +1561,8 @@ class AnalysisContextImpl implements InternalAnalysisContext {
}
}

/**
* Remove the given libraries that are keys in the given map from the list of
* containing libraries for each of the parts in the corresponding value.
*/
void _removeFromPartsUsingMap(HashMap<Source, List<Source>> oldPartMap) {
// TODO(brianwilkerson) Figure out whether we still need this.
// oldPartMap.forEach((Source librarySource, List<Source> oldParts) {
// for (int i = 0; i < oldParts.length; i++) {
// Source partSource = oldParts[i];
// if (partSource != librarySource) {
// DartEntry partEntry = _getReadableDartEntry(partSource);
// if (partEntry != null) {
// partEntry.removeContainingLibrary(librarySource);
// if (partEntry.containingLibraries.length == 0 &&
// !exists(partSource)) {
// _cache.remove(partSource);
// }
// }
// }
// }
// });
}
@override
CachePartition get privateAnalysisCachePartition => _privatePartition;

/**
* Remove the given [source] from the priority order if it is in the list.
Expand Down
11 changes: 11 additions & 0 deletions pkg/analyzer/lib/src/generated/engine.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,9 @@ class AnalysisContextImpl implements InternalAnalysisContext {
@override
List<AnalysisTarget> get priorityTargets => prioritySources;

@override
CachePartition get privateAnalysisCachePartition => _privatePartition;

@override
SourceFactory get sourceFactory => _sourceFactory;

Expand Down Expand Up @@ -9058,6 +9061,14 @@ abstract class InternalAnalysisContext implements AnalysisContext {
*/
List<AnalysisTarget> get priorityTargets;

/**
* The partition that contains analysis results that are not shared with other
* contexts.
*
* TODO(scheglov) add the type, once we have only one cache.
*/
dynamic get privateAnalysisCachePartition;

/**
* A factory to override how [ResolverVisitor] is created.
*/
Expand Down
64 changes: 64 additions & 0 deletions pkg/analyzer/lib/src/task/dart_work_manager.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,13 @@ class DartWorkManager implements WorkManager {
*/
AnalysisCache get analysisCache => context.analysisCache;

/**
* The partition that contains analysis results that are not shared with other
* contexts.
*/
CachePartition get privateAnalysisCachePartition =>
context.privateAnalysisCachePartition;

/**
* Specifies that the client want the given [result] of the given [target]
* to be computed with priority.
Expand Down Expand Up @@ -243,6 +250,20 @@ class DartWorkManager implements WorkManager {
return WorkOrderPriority.NONE;
}

/**
* Notifies the manager about analysis options changes.
*/
void onAnalysisOptionsChanged() {
_invalidateAllLocalResolutionInformation(false);
}

/**
* Notifies the manager about [SourceFactory] changes.
*/
void onSourceFactoryChanged() {
_invalidateAllLocalResolutionInformation(true);
}

@override
void resultsComputed(
AnalysisTarget target, Map<ResultDescriptor, dynamic> outputs) {
Expand Down Expand Up @@ -310,6 +331,49 @@ class DartWorkManager implements WorkManager {
librarySourceQueue.add(librarySource);
}

/**
* Invalidate all of the resolution results computed by this context. The flag
* [invalidateUris] should be `true` if the cached results of converting URIs
* to source files should also be invalidated.
*/
void _invalidateAllLocalResolutionInformation(bool invalidateUris) {
CachePartition partition = privateAnalysisCachePartition;
// Prepare targets and values to invalidate.
List<Source> dartSources = <Source>[];
List<LibrarySpecificUnit> unitTargets = <LibrarySpecificUnit>[];
MapIterator<AnalysisTarget, CacheEntry> iterator = partition.iterator();
while (iterator.moveNext()) {
AnalysisTarget target = iterator.key;
// Optionally gather Dart sources to invalidate URIs resolution.
if (invalidateUris && _isDartSource(target)) {
dartSources.add(target);
}
// LibrarySpecificUnit(s) are roots of Dart resolution.
// When one is invalidated, invalidation is propagated to all resolution.
if (target is LibrarySpecificUnit) {
unitTargets.add(target);
Source library = target.library;
if (context.exists(library)) {
librarySourceQueue.add(library);
}
}
}
// Invalidate targets and values.
unitTargets.forEach(partition.remove);
for (Source dartSource in dartSources) {
CacheEntry entry = partition.get(dartSource);
if (dartSource != null) {
// TODO(scheglov) we invalidate too much.
// Would be nice to invalidate just URLs resolution.
entry.setState(PARSED_UNIT, CacheState.INVALID);
entry.setState(IMPORTED_LIBRARIES, CacheState.INVALID);
entry.setState(EXPLICITLY_IMPORTED_LIBRARIES, CacheState.INVALID);
entry.setState(EXPORTED_LIBRARIES, CacheState.INVALID);
entry.setState(INCLUDED_PARTS, CacheState.INVALID);
}
}
}

/**
* Invalidate [CONTAINING_LIBRARIES] for the given [source].
* [CONTAINING_LIBRARIES] does not have dependencies, so we manage it here.
Expand Down
7 changes: 6 additions & 1 deletion pkg/analyzer/test/generated/engine_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5490,13 +5490,18 @@ class TestAnalysisContext implements InternalAnalysisContext {
fail("Unexpected invocation of getPrioritySources");
return null;
}

@override
List<AnalysisTarget> get priorityTargets {
fail("Unexpected invocation of visitCacheItems");
return null;
}

@override
CachePartition get privateAnalysisCachePartition {
fail("Unexpected invocation of privateAnalysisCachePartition");
return null;
}

@override
ResolverVisitorFactory get resolverVisitorFactory {
fail("Unexpected invocation of getResolverVisitorFactory");
Expand Down
76 changes: 66 additions & 10 deletions pkg/analyzer/test/src/task/dart_work_manager_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,6 @@ class DartWorkManagerTest {
expect(cache.getState(part1, CONTAINING_LIBRARIES), CacheState.INVALID);
}

CacheEntry _getOrCreateEntry(Source source) {
CacheEntry entry = cache.get(source);
if (entry == null) {
entry = new CacheEntry(source);
cache.put(entry);
}
return entry;
}

void test_applyChange_updatePartsLibraries_removeLibrary() {
Source part1 = new TestSource('part1.dart');
Source part2 = new TestSource('part2.dart');
Expand Down Expand Up @@ -420,6 +411,58 @@ class DartWorkManagerTest {
expect(manager.getNextResultPriority(), WorkOrderPriority.NONE);
}

void test_onAnalysisOptionsChanged() {
when(context.exists(anyObject)).thenReturn(true);
// set cache values
entry1.setValue(PARSED_UNIT, AstFactory.compilationUnit(), []);
entry1.setValue(IMPORTED_LIBRARIES, <Source>[], []);
entry1.setValue(EXPLICITLY_IMPORTED_LIBRARIES, <Source>[], []);
entry1.setValue(EXPORTED_LIBRARIES, <Source>[], []);
entry1.setValue(INCLUDED_PARTS, <Source>[], []);
// configure LibrarySpecificUnit
LibrarySpecificUnit unitTarget = new LibrarySpecificUnit(source2, source3);
CacheEntry unitEntry = new CacheEntry(unitTarget);
cache.put(unitEntry);
unitEntry.setValue(BUILD_LIBRARY_ERRORS, <AnalysisError>[], []);
expect(unitEntry.getState(BUILD_LIBRARY_ERRORS), CacheState.VALID);
// notify
manager.onAnalysisOptionsChanged();
// resolution is invalidated
expect(unitEntry.getState(BUILD_LIBRARY_ERRORS), CacheState.INVALID);
// ...but URIs are still value
expect(entry1.getState(PARSED_UNIT), CacheState.VALID);
expect(entry1.getState(IMPORTED_LIBRARIES), CacheState.VALID);
expect(entry1.getState(EXPLICITLY_IMPORTED_LIBRARIES), CacheState.VALID);
expect(entry1.getState(EXPORTED_LIBRARIES), CacheState.VALID);
expect(entry1.getState(INCLUDED_PARTS), CacheState.VALID);
}

void test_onSourceFactoryChanged() {
when(context.exists(anyObject)).thenReturn(true);
// set cache values
entry1.setValue(PARSED_UNIT, AstFactory.compilationUnit(), []);
entry1.setValue(IMPORTED_LIBRARIES, <Source>[], []);
entry1.setValue(EXPLICITLY_IMPORTED_LIBRARIES, <Source>[], []);
entry1.setValue(EXPORTED_LIBRARIES, <Source>[], []);
entry1.setValue(INCLUDED_PARTS, <Source>[], []);
// configure LibrarySpecificUnit
LibrarySpecificUnit unitTarget = new LibrarySpecificUnit(source2, source3);
CacheEntry unitEntry = new CacheEntry(unitTarget);
cache.put(unitEntry);
unitEntry.setValue(BUILD_LIBRARY_ERRORS, <AnalysisError>[], []);
expect(unitEntry.getState(BUILD_LIBRARY_ERRORS), CacheState.VALID);
// notify
manager.onSourceFactoryChanged();
// resolution is invalidated
expect(unitEntry.getState(BUILD_LIBRARY_ERRORS), CacheState.INVALID);
// ...and URIs resolution too
expect(entry1.getState(PARSED_UNIT), CacheState.INVALID);
expect(entry1.getState(IMPORTED_LIBRARIES), CacheState.INVALID);
expect(entry1.getState(EXPLICITLY_IMPORTED_LIBRARIES), CacheState.INVALID);
expect(entry1.getState(EXPORTED_LIBRARIES), CacheState.INVALID);
expect(entry1.getState(INCLUDED_PARTS), CacheState.INVALID);
}

void test_resultsComputed_errors_forLibrarySpecificUnit() {
AnalysisError error1 =
new AnalysisError(source1, 1, 0, ScannerErrorCode.MISSING_DIGIT);
Expand Down Expand Up @@ -539,17 +582,30 @@ class DartWorkManagerTest {
expect_librarySourceQueue([]);
expect_unknownSourceQueue([source1, source3]);
}

CacheEntry _getOrCreateEntry(Source source) {
CacheEntry entry = cache.get(source);
if (entry == null) {
entry = new CacheEntry(source);
cache.put(entry);
}
return entry;
}
}

class _InternalAnalysisContextMock extends TypedMock
implements InternalAnalysisContext {
@override
CachePartition privateAnalysisCachePartition;

@override
AnalysisCache analysisCache;

Map<Source, ChangeNoticeImpl> _pendingNotices = <Source, ChangeNoticeImpl>{};

_InternalAnalysisContextMock() {
analysisCache = new AnalysisCache([new UniversalCachePartition(this)]);
privateAnalysisCachePartition = new UniversalCachePartition(this);
analysisCache = new AnalysisCache([privateAnalysisCachePartition]);
}

@override
Expand Down

0 comments on commit 811877d

Please sign in to comment.