From 8a8ebe34bc1b4e04e36adeb8ec4f0541024a3cbf Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Tue, 29 Jun 2021 18:00:48 +0000 Subject: [PATCH] Revert "Revert "Keep libraries after linking, don't reload from bytes."" This reverts commit b3a688a9c692fa769f48472e850a258e6cec152d. Initial: https://dart-review.googlesource.com/c/sdk/+/201661 Reverted: https://dart-review.googlesource.com/c/sdk/+/204263 Change-Id: I9505962caf6ce0244ae68c3d97e6f6629f95fadc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205142 Reviewed-by: Brian Wilkerson Commit-Queue: Konstantin Shcheglov --- .../src/dart/analysis/library_context.dart | 17 ++++---- .../lib/src/dart/micro/resolve_file.dart | 15 ++++--- pkg/analyzer/lib/src/summary2/link.dart | 8 ---- .../src/summary/resynthesize_ast2_test.dart | 42 ++++++++++++++----- 4 files changed, 46 insertions(+), 36 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/analysis/library_context.dart b/pkg/analyzer/lib/src/dart/analysis/library_context.dart index c9295afd4b0..df060fa23fa 100644 --- a/pkg/analyzer/lib/src/dart/analysis/library_context.dart +++ b/pkg/analyzer/lib/src/dart/analysis/library_context.dart @@ -183,21 +183,18 @@ class LibraryContext { counterUnlinkedLinkedBytes += resolutionBytes.length; librariesLinkedTimer.stop(); - // TODO(scheglov) Uncomment to keep linking elements. - // return; } else { // TODO(scheglov) Take / clear parsed units in files. bytesGet += resolutionBytes.length; librariesLoaded += cycle.libraries.length; + elementFactory.addBundle( + BundleReader( + elementFactory: elementFactory, + unitsInformativeBytes: unitsInformativeBytes, + resolutionBytes: resolutionBytes, + ), + ); } - - elementFactory.addBundle( - BundleReader( - elementFactory: elementFactory, - unitsInformativeBytes: unitsInformativeBytes, - resolutionBytes: resolutionBytes, - ), - ); } logger.run('Prepare linked bundles', () { diff --git a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart index d98f3e44654..26085886908 100644 --- a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart +++ b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart @@ -908,17 +908,16 @@ class _LibraryContext { } else { performance.getDataInt('bytesGet').add(resolutionBytes.length); performance.getDataInt('libraryLoadCount').add(cycle.libraries.length); + elementFactory.addBundle( + BundleReader( + elementFactory: elementFactory, + unitsInformativeBytes: unitsInformativeBytes, + resolutionBytes: resolutionBytes as Uint8List, + ), + ); } cycle.resolutionId = resolutionData!.id; - elementFactory.addBundle( - BundleReader( - elementFactory: elementFactory, - unitsInformativeBytes: unitsInformativeBytes, - resolutionBytes: resolutionBytes as Uint8List, - ), - ); - // We might have just linked dart:core, ensure the type provider. _createElementFactoryTypeProvider(); } diff --git a/pkg/analyzer/lib/src/summary2/link.dart b/pkg/analyzer/lib/src/summary2/link.dart index 5f82cce6891..5156b61b6b3 100644 --- a/pkg/analyzer/lib/src/summary2/link.dart +++ b/pkg/analyzer/lib/src/summary2/link.dart @@ -24,7 +24,6 @@ import 'package:analyzer/src/summary2/types_builder.dart'; import 'package:analyzer/src/summary2/variance_builder.dart'; var timerLinkingLinkingBundle = Stopwatch(); -var timerLinkingRemoveBundle = Stopwatch(); /// Note that AST units and tokens of [inputLibraries] will be damaged. /// @@ -81,13 +80,6 @@ class Linker { timerLinkingLinkingBundle.start(); _writeLibraries(); timerLinkingLinkingBundle.stop(); - - // TODO(scheglov) Remove to keep linking elements. - timerLinkingRemoveBundle.start(); - elementFactory.removeBundle( - inputLibraries.map((e) => e.uriStr).toSet(), - ); - timerLinkingRemoveBundle.stop(); } void _buildEnumChildren() { diff --git a/pkg/analyzer/test/src/summary/resynthesize_ast2_test.dart b/pkg/analyzer/test/src/summary/resynthesize_ast2_test.dart index 738afb87bfb..3a95b465c0c 100644 --- a/pkg/analyzer/test/src/summary/resynthesize_ast2_test.dart +++ b/pkg/analyzer/test/src/summary/resynthesize_ast2_test.dart @@ -26,7 +26,8 @@ import 'test_strategies.dart'; main() { defineReflectiveSuite(() { - defineReflectiveTests(ResynthesizeAst2Test); + defineReflectiveTests(ResynthesizeAstKeepLinkingTest); + defineReflectiveTests(ResynthesizeAstFromBytesTest); // defineReflectiveTests(ApplyCheckElementTextReplacements); }); } @@ -39,11 +40,16 @@ class ApplyCheckElementTextReplacements { } @reflectiveTest -class ResynthesizeAst2Test extends AbstractResynthesizeTest +abstract class ResynthesizeAst2Test extends AbstractResynthesizeTest with ResynthesizeTestCases { /// The shared SDK bundle, computed once and shared among test invocations. static _SdkBundle? _sdkBundle; + /// We need to test both cases - when we keep linking libraries (happens for + /// new or invalidated libraries), and when we load libraries from bytes + /// (happens internally in Blaze or when we have cached summaries). + bool get keepLinkingLibraries; + _SdkBundle get sdkBundle { if (_sdkBundle != null) { return _sdkBundle!; @@ -124,14 +130,18 @@ class ResynthesizeAst2Test extends AbstractResynthesizeTest var linkResult = link(elementFactory, inputLibraries, true); - // TODO(scheglov) Remove to keep linking elements. - elementFactory.addBundle( - BundleReader( - elementFactory: elementFactory, - unitsInformativeBytes: unitsInformativeBytes, - resolutionBytes: linkResult.resolutionBytes, - ), - ); + if (!keepLinkingLibraries) { + elementFactory.removeBundle( + inputLibraries.map((e) => e.uriStr).toSet(), + ); + elementFactory.addBundle( + BundleReader( + elementFactory: elementFactory, + unitsInformativeBytes: unitsInformativeBytes, + resolutionBytes: linkResult.resolutionBytes, + ), + ); + } return elementFactory.libraryOfUri('${source.uri}')!; } @@ -232,6 +242,18 @@ class ResynthesizeAst2Test extends AbstractResynthesizeTest } } +@reflectiveTest +class ResynthesizeAstFromBytesTest extends ResynthesizeAst2Test { + @override + bool get keepLinkingLibraries => false; +} + +@reflectiveTest +class ResynthesizeAstKeepLinkingTest extends ResynthesizeAst2Test { + @override + bool get keepLinkingLibraries => true; +} + class _AnalysisSessionForLinking implements AnalysisSessionImpl { @override final ClassHierarchy classHierarchy = ClassHierarchy();