Version 2.18.0-240.0.dev
Merge commit '1c461e06c1d53cf60e4458bb89b2a1543266bd95' into 'dev'
diff --git a/pkg/analyzer/lib/src/dart/analysis/driver.dart b/pkg/analyzer/lib/src/dart/analysis/driver.dart
index ba502a6..d19cc50 100644
--- a/pkg/analyzer/lib/src/dart/analysis/driver.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/driver.dart
@@ -1363,7 +1363,7 @@
return result;
} catch (exception, stackTrace) {
String? contextKey =
- _storeExceptionContext(path, library.file, exception, stackTrace);
+ _storeExceptionContext(path, library, exception, stackTrace);
throw _ExceptionState(exception, stackTrace, contextKey);
}
});
@@ -1500,7 +1500,6 @@
/// When we look at a part that has a `part of name;` directive, we
/// usually don't know the library (in contrast to `part of uri;`).
/// So, we have no choice than to resolve this part as its own library.
- /// TODO(scheglov) Maybe just return an error result instead?
///
/// But parts of `dart:xyz` libraries are special. The reason is that
/// `dart:core` is always implicitly imported. So, when we start building
@@ -1519,7 +1518,8 @@
_fsState.getFileForUri(Uri.parse('dart:core')).map(
(file) {
- file?.transitiveFiles;
+ final kind = file?.kind as LibraryFileStateKind;
+ kind.discoverReferencedFiles();
},
(externalLibrary) {},
);
@@ -1810,7 +1810,7 @@
.toBuffer();
}
- String? _storeExceptionContext(String path, FileState libraryFile,
+ String? _storeExceptionContext(String path, LibraryFileStateKind library,
Object exception, StackTrace stackTrace) {
if (allowedNumberOfContextsToWrite <= 0) {
return null;
@@ -1818,8 +1818,7 @@
allowedNumberOfContextsToWrite--;
}
try {
- List<AnalysisDriverExceptionFileBuilder> contextFiles = libraryFile
- .transitiveFiles
+ final contextFiles = library.files
.map((file) => AnalysisDriverExceptionFileBuilder(
path: file.path, content: file.content))
.toList();
diff --git a/pkg/analyzer/lib/src/dart/analysis/file_state.dart b/pkg/analyzer/lib/src/dart/analysis/file_state.dart
index ffd31fe..8e11c437 100644
--- a/pkg/analyzer/lib/src/dart/analysis/file_state.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/file_state.dart
@@ -124,8 +124,15 @@
bool isAugmentationOf(LibraryOrAugmentationFileKind container) => false;
}
+/// Information about a directive that "includes" a file - `import`, `export`,
+/// or `part`. But not `part of` or `library augment` - these are modelled as
+/// kinds.
+class DirectiveState {
+ void dispose() {}
+}
+
/// Information about a single `import` directive.
-class ExportDirectiveState {
+class ExportDirectiveState extends DirectiveState {
final UnlinkedNamespaceDirective directive;
ExportDirectiveState({
@@ -145,12 +152,16 @@
/// [ExportDirectiveState] that has a valid URI that references a file.
class ExportDirectiveWithFile extends ExportDirectiveState {
+ final LibraryOrAugmentationFileKind container;
final FileState exportedFile;
ExportDirectiveWithFile({
required super.directive,
+ required this.container,
required this.exportedFile,
- });
+ }) {
+ exportedFile.referencingFiles.add(container.file);
+ }
/// Returns [exportedFile] if it is a library.
LibraryFileStateKind? get exportedLibrary {
@@ -171,6 +182,11 @@
@override
Source get exportedSource => exportedFile.source;
+
+ @override
+ void dispose() {
+ exportedFile.referencingFiles.remove(container.file);
+ }
}
/// [ExportDirectiveState] with a URI that resolves to [InSummarySource].
@@ -420,21 +436,6 @@
@visibleForTesting
FileStateTestView get test => FileStateTestView(this);
- /// Return the set of transitive files - the file itself and all of the
- /// directly or indirectly referenced files.
- Set<FileState> get transitiveFiles {
- var transitiveFiles = <FileState>{};
-
- void appendReferenced(FileState file) {
- if (transitiveFiles.add(file)) {
- file.directReferencedFiles.forEach(appendReferenced);
- }
- }
-
- appendReferenced(this);
- return transitiveFiles;
- }
-
/// The [UnlinkedUnit] of the file.
UnlinkedUnit get unlinked2 => _unlinked2!;
@@ -551,10 +552,6 @@
!_equalByteLists(_apiSignature, newApiSignature);
_apiSignature = newApiSignature;
- // It is possible that this file does not reference these files.
- // TODO(scheglov) This also could be moved, almost.
- _stopReferencingByThisFile();
-
// Read imports/exports on demand.
_importedFiles = null;
_exportedFiles = null;
@@ -706,19 +703,6 @@
return directive.uri;
}
- void _stopReferencingByThisFile() {
- void removeForOne(List<FileState?>? referencedFiles) {
- if (referencedFiles != null) {
- for (var referenced in referencedFiles) {
- referenced?.referencingFiles.remove(this);
- }
- }
- }
-
- removeForOne(_exportedFiles);
- removeForOne(_importedFiles);
- }
-
void _updateKind() {
_kind?.dispose();
@@ -1460,12 +1444,10 @@
}
/// Information about a single `import augment` directive.
-class ImportAugmentationDirectiveState {
- final LibraryOrAugmentationFileKind container;
+class ImportAugmentationDirectiveState extends DirectiveState {
final UnlinkedImportAugmentationDirective directive;
ImportAugmentationDirectiveState({
- required this.container,
required this.directive,
});
@@ -1478,13 +1460,16 @@
/// [PartDirectiveState] that has a valid URI that references a file.
class ImportAugmentationDirectiveWithFile
extends ImportAugmentationDirectiveState {
+ final LibraryOrAugmentationFileKind container;
final FileState importedFile;
ImportAugmentationDirectiveWithFile({
- required super.container,
+ required this.container,
required super.directive,
required this.importedFile,
- });
+ }) {
+ importedFile.referencingFiles.add(container.file);
+ }
/// If [importedFile] is a [AugmentationFileStateKind], and it confirms that
/// it is an augmentation of the [container], returns the [importedFile].
@@ -1498,10 +1483,15 @@
@override
Source? get importedSource => importedFile.source;
+
+ @override
+ void dispose() {
+ importedFile.referencingFiles.remove(container.file);
+ }
}
/// Information about a single `import` directive.
-class ImportDirectiveState {
+class ImportDirectiveState extends DirectiveState {
final UnlinkedNamespaceDirective directive;
ImportDirectiveState({
@@ -1523,12 +1513,16 @@
/// [ImportDirectiveState] that has a valid URI that references a file.
class ImportDirectiveWithFile extends ImportDirectiveState {
+ final LibraryOrAugmentationFileKind container;
final FileState importedFile;
ImportDirectiveWithFile({
+ required this.container,
required super.directive,
required this.importedFile,
- });
+ }) {
+ importedFile.referencingFiles.add(container.file);
+ }
/// Returns [importedFile] if it is a library.
LibraryFileStateKind? get importedLibrary {
@@ -1549,6 +1543,11 @@
@override
Source get importedSource => importedFile.source;
+
+ @override
+ void dispose() {
+ importedFile.referencingFiles.remove(container.file);
+ }
}
/// [ImportDirectiveState] with a URI that resolves to [InSummarySource].
@@ -1630,7 +1629,6 @@
return file._fileForRelativeUri(directive.uri).map(
(refFile) {
if (refFile != null) {
- refFile.referencingFiles.add(file);
return PartDirectiveWithFile(
library: this,
directive: directive,
@@ -1663,16 +1661,7 @@
void dispose() {
invalidateLibraryCycle();
file._fsState._libraryNameToFiles.remove(this);
-
- final parts = _parts;
- if (parts != null) {
- for (final part in parts) {
- if (part is PartDirectiveWithFile) {
- part.includedFile.referencingFiles.remove(file);
- }
- }
- }
-
+ _parts?.disposeAll();
super.dispose();
}
@@ -1711,7 +1700,6 @@
return file._fileForRelativeUri(directive.uri).map(
(refFile) {
if (refFile != null) {
- refFile.referencingFiles.add(file);
return ImportAugmentationDirectiveWithFile(
container: this,
directive: directive,
@@ -1719,14 +1707,12 @@
);
} else {
return ImportAugmentationDirectiveState(
- container: this,
directive: directive,
);
}
},
(externalLibrary) {
return ImportAugmentationDirectiveState(
- container: this,
directive: directive,
);
},
@@ -1740,8 +1726,8 @@
return file._fileForRelativeUri(uriStr).map(
(refFile) {
if (refFile != null) {
- refFile.referencingFiles.add(file);
return ExportDirectiveWithFile(
+ container: this,
directive: directive,
exportedFile: refFile,
);
@@ -1767,8 +1753,8 @@
return file._fileForRelativeUri(uriStr).map(
(refFile) {
if (refFile != null) {
- refFile.referencingFiles.add(file);
return ImportDirectiveWithFile(
+ container: this,
directive: directive,
importedFile: refFile,
);
@@ -1806,33 +1792,9 @@
@override
void dispose() {
- final augmentations = _augmentations;
- if (augmentations != null) {
- for (final import in augmentations) {
- if (import is ImportAugmentationDirectiveWithFile) {
- import.importedFile.referencingFiles.remove(file);
- }
- }
- }
-
- final exports = _exports;
- if (exports != null) {
- for (final export in exports) {
- if (export is ExportDirectiveWithFile) {
- export.exportedFile.referencingFiles.remove(file);
- }
- }
- }
-
- final imports = _imports;
- if (imports != null) {
- for (final import in imports) {
- if (import is ImportDirectiveWithFile) {
- import.importedFile.referencingFiles.remove(file);
- }
- }
- }
-
+ _augmentations?.disposeAll();
+ _exports?.disposeAll();
+ _imports?.disposeAll();
super.dispose();
}
@@ -1849,7 +1811,7 @@
}
/// Information about a single `part` directive.
-class PartDirectiveState {
+class PartDirectiveState extends DirectiveState {
final LibraryFileStateKind library;
final UnlinkedPartDirective directive;
@@ -1872,7 +1834,9 @@
required super.library,
required super.directive,
required this.includedFile,
- });
+ }) {
+ includedFile.referencingFiles.add(library.file);
+ }
/// If [includedFile] is a [PartFileStateKind], and it confirms that it
/// is a part of the [library], returns the [includedFile].
@@ -1886,6 +1850,11 @@
@override
Source? get includedSource => includedFile.source;
+
+ @override
+ void dispose() {
+ includedFile.referencingFiles.remove(library.file);
+ }
}
/// The file has `part of` directive.
@@ -2103,3 +2072,11 @@
}
}
}
+
+extension on List<DirectiveState> {
+ void disposeAll() {
+ for (final directive in this) {
+ directive.dispose();
+ }
+ }
+}
diff --git a/pkg/analyzer/test/src/dart/analysis/analyzer_state_printer.dart b/pkg/analyzer/test/src/dart/analysis/analyzer_state_printer.dart
index 242801d..1c43336 100644
--- a/pkg/analyzer/test/src/dart/analysis/analyzer_state_printer.dart
+++ b/pkg/analyzer/test/src/dart/analysis/analyzer_state_printer.dart
@@ -96,13 +96,13 @@
_writeElements<ImportAugmentationDirectiveState>(
'augmentations',
container.augmentations,
- (augmentation) {
- expect(augmentation.container, same(container));
- if (augmentation is ImportAugmentationDirectiveWithFile) {
- final file = augmentation.importedFile;
+ (import) {
+ if (import is ImportAugmentationDirectiveWithFile) {
+ expect(import.container, same(container));
+ final file = import.importedFile;
sink.write(_indent);
- final importedAugmentation = augmentation.importedAugmentation;
+ final importedAugmentation = import.importedAugmentation;
if (importedAugmentation != null) {
expect(importedAugmentation.file, file);
sink.write(idProvider.fileStateKind(importedAugmentation));
@@ -112,7 +112,7 @@
sink.writeln();
} else {
sink.write(_indent);
- sink.write('uri: ${_stringOfUriStr(augmentation.directive.uri)}');
+ sink.write('uri: ${_stringOfUriStr(import.directive.uri)}');
sink.writeln();
}
},
@@ -169,89 +169,99 @@
});
}
- void _writeFileExports(LibraryOrAugmentationFileKind file) {
- _writeElements<ExportDirectiveState>('exports', file.exports, (export) {
- if (export is ExportDirectiveWithFile) {
- final file = export.exportedFile;
- sink.write(_indent);
+ void _writeFileExports(LibraryOrAugmentationFileKind container) {
+ _writeElements<ExportDirectiveState>(
+ 'exports',
+ container.exports,
+ (export) {
+ if (export is ExportDirectiveWithFile) {
+ expect(export.container, same(container));
+ final file = export.exportedFile;
+ sink.write(_indent);
- final exportedLibrary = export.exportedLibrary;
- if (exportedLibrary != null) {
- expect(exportedLibrary.file, file);
- sink.write(idProvider.fileStateKind(exportedLibrary));
+ final exportedLibrary = export.exportedLibrary;
+ if (exportedLibrary != null) {
+ expect(exportedLibrary.file, file);
+ sink.write(idProvider.fileStateKind(exportedLibrary));
+ } else {
+ sink.write('notLibrary ${idProvider.fileState(file)}');
+ }
+
+ if (omitSdkFiles && file.uri.isScheme('dart')) {
+ sink.write(' ${file.uri}');
+ }
+ sink.writeln();
+ } else if (export is ExportDirectiveWithInSummarySource) {
+ sink.write(_indent);
+ sink.write('inSummary ${export.exportedSource.uri}');
+
+ final librarySource = export.exportedLibrarySource;
+ if (librarySource != null) {
+ expect(librarySource, same(export.exportedSource));
+ } else {
+ sink.write(' notLibrary');
+ }
+ sink.writeln();
} else {
- sink.write('notLibrary ${idProvider.fileState(file)}');
+ sink.write(_indent);
+ sink.write('uri: ${_stringOfUriStr(export.directive.uri)}');
+ sink.writeln();
}
-
- if (omitSdkFiles && file.uri.isScheme('dart')) {
- sink.write(' ${file.uri}');
- }
- sink.writeln();
- } else if (export is ExportDirectiveWithInSummarySource) {
- sink.write(_indent);
- sink.write('inSummary ${export.exportedSource.uri}');
-
- final librarySource = export.exportedLibrarySource;
- if (librarySource != null) {
- expect(librarySource, same(export.exportedSource));
- } else {
- sink.write(' notLibrary');
- }
- sink.writeln();
- } else {
- sink.write(_indent);
- sink.write('uri: ${_stringOfUriStr(export.directive.uri)}');
- sink.writeln();
- }
- });
+ },
+ );
}
- void _writeFileImports(LibraryOrAugmentationFileKind file) {
- _writeElements<ImportDirectiveState>('imports', file.imports, (import) {
- if (import is ImportDirectiveWithFile) {
- final file = import.importedFile;
- sink.write(_indent);
+ void _writeFileImports(LibraryOrAugmentationFileKind container) {
+ _writeElements<ImportDirectiveState>(
+ 'imports',
+ container.imports,
+ (import) {
+ if (import is ImportDirectiveWithFile) {
+ expect(import.container, same(container));
+ final file = import.importedFile;
+ sink.write(_indent);
- final importedLibrary = import.importedLibrary;
- if (importedLibrary != null) {
- expect(importedLibrary.file, file);
- sink.write(idProvider.fileStateKind(importedLibrary));
+ final importedLibrary = import.importedLibrary;
+ if (importedLibrary != null) {
+ expect(importedLibrary.file, file);
+ sink.write(idProvider.fileStateKind(importedLibrary));
+ } else {
+ sink.write('notLibrary ${idProvider.fileState(file)}');
+ }
+
+ if (omitSdkFiles && file.uri.isScheme('dart')) {
+ sink.write(' ${file.uri}');
+ }
+
+ if (import.isSyntheticDartCoreImport) {
+ sink.write(' synthetic');
+ }
+ sink.writeln();
+ } else if (import is ImportDirectiveWithInSummarySource) {
+ sink.write(_indent);
+ sink.write('inSummary ${import.importedSource.uri}');
+
+ final librarySource = import.importedLibrarySource;
+ if (librarySource != null) {
+ expect(librarySource, same(import.importedSource));
+ } else {
+ sink.write(' notLibrary');
+ }
+
+ if (import.isSyntheticDartCoreImport) {
+ sink.write(' synthetic');
+ }
+ sink.writeln();
} else {
- sink.write('notLibrary ${idProvider.fileState(file)}');
+ sink.write(_indent);
+ sink.write('uri: ${_stringOfUriStr(import.directive.uri)}');
+ if (import.isSyntheticDartCoreImport) {
+ sink.write(' synthetic');
+ }
+ sink.writeln();
}
-
- if (omitSdkFiles && file.uri.isScheme('dart')) {
- sink.write(' ${file.uri}');
- }
-
- if (import.isSyntheticDartCoreImport) {
- sink.write(' synthetic');
- }
- sink.writeln();
- } else if (import is ImportDirectiveWithInSummarySource) {
- sink.write(_indent);
- sink.write('inSummary ${import.importedSource.uri}');
-
- final librarySource = import.importedLibrarySource;
- if (librarySource != null) {
- expect(librarySource, same(import.importedSource));
- } else {
- sink.write(' notLibrary');
- }
-
- if (import.isSyntheticDartCoreImport) {
- sink.write(' synthetic');
- }
- sink.writeln();
- } else {
- sink.write(_indent);
- sink.write('uri: ${_stringOfUriStr(import.directive.uri)}');
- if (import.isSyntheticDartCoreImport) {
- sink.write(' synthetic');
- }
- sink.writeln();
- }
- });
+ },
+ );
}
void _writeFileKind(FileState file) {
diff --git a/runtime/vm/dart.cc b/runtime/vm/dart.cc
index 65aea3a..9d66e4d 100644
--- a/runtime/vm/dart.cc
+++ b/runtime/vm/dart.cc
@@ -1059,9 +1059,6 @@
return error.ptr();
}
- if (!was_child_cloned_into_existing_isolate) {
- IG->heap()->InitGrowthControl();
- }
I->set_init_callback_data(isolate_data);
if (FLAG_print_class_table) {
IG->class_table()->Print();
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index eb33322..adac74b 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -1313,9 +1313,6 @@
}
if (success) {
- if (is_new_group) {
- group->heap()->InitGrowthControl();
- }
// A Thread structure has been associated to the thread, we do the
// safepoint transition explicitly here instead of using the
// TransitionXXX scope objects as the reverse transition happens
diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc
index 4d7f959..0e7442a 100644
--- a/runtime/vm/dart_api_impl_test.cc
+++ b/runtime/vm/dart_api_impl_test.cc
@@ -3280,7 +3280,7 @@
{
// GCs due to allocations or weak handle creation can cause early
// promotion and interfere with the scenario this test is verifying.
- NoHeapGrowthControlScope force_growth;
+ ForceGrowthScope force_growth(thread);
// Create an object in new space.
new_ref = AllocateNewString("new string");
@@ -3425,7 +3425,7 @@
{
// GCs due to allocations or weak handle creation can cause early
// promotion and interfere with the scenario this test is verifying.
- NoHeapGrowthControlScope force_growth;
+ ForceGrowthScope force_growth(thread);
// Create an object in new space.
new_ref = AllocateNewString("new string");
diff --git a/runtime/vm/heap/heap.cc b/runtime/vm/heap/heap.cc
index d8b0c3f..c218c50 100644
--- a/runtime/vm/heap/heap.cc
+++ b/runtime/vm/heap/heap.cc
@@ -74,7 +74,7 @@
if (LIKELY(addr != 0)) {
return addr;
}
- if (!assume_scavenge_will_fail_ && new_space_.GrowthControlState()) {
+ if (!assume_scavenge_will_fail_ && !thread->force_growth()) {
// This call to CollectGarbage might end up "reusing" a collection spawned
// from a different thread and will be racing to allocate the requested
// memory with other threads being released after the collection.
@@ -93,7 +93,7 @@
uword Heap::AllocateOld(Thread* thread, intptr_t size, OldPage::PageType type) {
ASSERT(thread->no_safepoint_scope_depth() == 0);
- if (old_space_.GrowthControlState()) {
+ if (!thread->force_growth()) {
CollectForDebugging(thread);
uword addr = old_space_.TryAllocate(size, type);
if (addr != 0) {
@@ -132,7 +132,7 @@
return addr;
}
- if (old_space_.GrowthControlState()) {
+ if (!thread->force_growth()) {
WaitForSweeperTasks(thread);
old_space_.TryReleaseReservation();
} else {
@@ -155,10 +155,10 @@
}
Thread* thread = Thread::Current();
- if (thread->no_callback_scope_depth() == 0) {
+ if ((thread->no_callback_scope_depth() == 0) && !thread->force_growth()) {
CheckExternalGC(thread);
} else {
- // Check delayed until Dart_TypedDataRelease.
+ // Check delayed until Dart_TypedDataRelease/~ForceGrowthScope.
}
}
@@ -179,6 +179,7 @@
void Heap::CheckExternalGC(Thread* thread) {
ASSERT(thread->no_safepoint_scope_depth() == 0);
ASSERT(thread->no_callback_scope_depth() == 0);
+ ASSERT(!thread->force_growth());
if (new_space_.ExternalInWords() >= (4 * new_space_.CapacityInWords())) {
// Attempt to free some external allocation by a scavenge. (If the total
// remains above the limit, next external alloc will trigger another.)
@@ -566,6 +567,8 @@
void Heap::CheckConcurrentMarking(Thread* thread,
GCReason reason,
intptr_t size) {
+ ASSERT(!thread->force_growth());
+
PageSpace::Phase phase;
{
MonitorLocker ml(old_space_.tasks_lock());
@@ -662,21 +665,6 @@
(UsedInWords(Heap::kOld) * kWordSize));
}
-void Heap::InitGrowthControl() {
- new_space_.InitGrowthControl();
- old_space_.InitGrowthControl();
-}
-
-void Heap::SetGrowthControlState(bool state) {
- new_space_.SetGrowthControlState(state);
- old_space_.SetGrowthControlState(state);
-}
-
-bool Heap::GrowthControlState() {
- ASSERT(new_space_.GrowthControlState() == old_space_.GrowthControlState());
- return old_space_.GrowthControlState();
-}
-
void Heap::WriteProtect(bool read_only) {
read_only_ = read_only;
new_space_.WriteProtect(read_only);
@@ -1187,16 +1175,13 @@
}
}
-NoHeapGrowthControlScope::NoHeapGrowthControlScope()
- : ThreadStackResource(Thread::Current()) {
- Heap* heap = isolate_group()->heap();
- current_growth_controller_state_ = heap->GrowthControlState();
- heap->DisableGrowthControl();
+ForceGrowthScope::ForceGrowthScope(Thread* thread)
+ : ThreadStackResource(thread) {
+ thread->IncrementForceGrowthScopeDepth();
}
-NoHeapGrowthControlScope::~NoHeapGrowthControlScope() {
- Heap* heap = isolate_group()->heap();
- heap->SetGrowthControlState(current_growth_controller_state_);
+ForceGrowthScope::~ForceGrowthScope() {
+ thread()->DecrementForceGrowthScopeDepth();
}
WritableVMIsolateScope::WritableVMIsolateScope(Thread* thread)
diff --git a/runtime/vm/heap/heap.h b/runtime/vm/heap/heap.h
index 3ceab79..3408cb2 100644
--- a/runtime/vm/heap/heap.h
+++ b/runtime/vm/heap/heap.h
@@ -131,13 +131,6 @@
void WaitForSweeperTasks(Thread* thread);
void WaitForSweeperTasksAtSafepoint(Thread* thread);
- // Enables growth control on the page space heaps. This should be
- // called before any user code is executed.
- void InitGrowthControl();
- void DisableGrowthControl() { SetGrowthControlState(false); }
- void SetGrowthControlState(bool state);
- bool GrowthControlState();
-
// Protect access to the heap. Note: Code pages are made
// executable/non-executable when 'read_only' is true/false, respectively.
void WriteProtect(bool read_only);
@@ -424,14 +417,13 @@
DISALLOW_COPY_AND_ASSIGN(HeapIterationScope);
};
-class NoHeapGrowthControlScope : public ThreadStackResource {
+class ForceGrowthScope : public ThreadStackResource {
public:
- NoHeapGrowthControlScope();
- ~NoHeapGrowthControlScope();
+ explicit ForceGrowthScope(Thread* thread);
+ ~ForceGrowthScope();
private:
- bool current_growth_controller_state_;
- DISALLOW_COPY_AND_ASSIGN(NoHeapGrowthControlScope);
+ DISALLOW_COPY_AND_ASSIGN(ForceGrowthScope);
};
// Note: During this scope all pages are writable and the code pages are
diff --git a/runtime/vm/heap/heap_test.cc b/runtime/vm/heap/heap_test.cc
index d2e2c41..51a139b 100644
--- a/runtime/vm/heap/heap_test.cc
+++ b/runtime/vm/heap/heap_test.cc
@@ -721,4 +721,65 @@
}
}
+class ConcurrentForceGrowthScopeTask : public ThreadPool::Task {
+ public:
+ ConcurrentForceGrowthScopeTask(Isolate* isolate,
+ Monitor* monitor,
+ intptr_t* done_count)
+ : isolate_(isolate), monitor_(monitor), done_count_(done_count) {}
+
+ virtual void Run() {
+ Thread::EnterIsolateAsHelper(isolate_, Thread::kUnknownTask);
+ {
+ Thread* thread = Thread::Current();
+ StackZone stack_zone(thread);
+
+ GrowableObjectArray& accumulate =
+ GrowableObjectArray::Handle(GrowableObjectArray::New());
+ Object& element = Object::Handle();
+ for (intptr_t i = 0; i < 1000; i++) {
+ // Lots of entering and leaving ForceGrowth scopes. Previously, this
+ // would have been data races on the per-Heap force-growth flag.
+ {
+ ForceGrowthScope force_growth(thread);
+ GrowableObjectArrayPtr unsafe_accumulate = accumulate.ptr();
+ element = Array::New(0);
+ accumulate = unsafe_accumulate;
+ }
+ accumulate.Add(element);
+ }
+ }
+ Thread::ExitIsolateAsHelper();
+ // Notify the main thread that this thread has exited.
+ {
+ MonitorLocker ml(monitor_);
+ *done_count_ += 1;
+ ml.Notify();
+ }
+ }
+
+ private:
+ Isolate* isolate_;
+ Monitor* monitor_;
+ intptr_t* done_count_;
+};
+
+ISOLATE_UNIT_TEST_CASE(ConcurrentForceGrowthScope) {
+ intptr_t task_count = 8;
+ Monitor monitor;
+ intptr_t done_count = 0;
+
+ for (intptr_t i = 0; i < task_count; i++) {
+ Dart::thread_pool()->Run<ConcurrentForceGrowthScopeTask>(
+ thread->isolate(), &monitor, &done_count);
+ }
+
+ {
+ MonitorLocker ml(&monitor);
+ while (done_count < task_count) {
+ ml.WaitWithSafepointCheck(thread);
+ }
+ }
+}
+
} // namespace dart
diff --git a/runtime/vm/heap/pages.cc b/runtime/vm/heap/pages.cc
index 1d66e97..e207185 100644
--- a/runtime/vm/heap/pages.cc
+++ b/runtime/vm/heap/pages.cc
@@ -471,7 +471,7 @@
ASSERT(Heap::IsAllocatableViaFreeLists(size));
if (growth_policy != kForceGrowth) {
- ASSERT(GrowthControlState());
+ ASSERT(!Thread::Current()->force_growth());
if (heap_ != nullptr) { // Some unit tests.
heap_->CheckConcurrentMarking(Thread::Current(), GCReason::kOldSpace,
kOldPageSize);
@@ -513,7 +513,7 @@
ASSERT(!Heap::IsAllocatableViaFreeLists(size));
if (growth_policy != kForceGrowth) {
- ASSERT(GrowthControlState());
+ ASSERT(!Thread::Current()->force_growth());
if (heap_ != nullptr) { // Some unit tests.
heap_->CheckConcurrentMarking(Thread::Current(), GCReason::kOldSpace,
size);
@@ -1110,7 +1110,7 @@
}
void PageSpace::CollectGarbage(Thread* thread, bool compact, bool finalize) {
- ASSERT(GrowthControlState());
+ ASSERT(!Thread::Current()->force_growth());
if (!finalize) {
#if defined(TARGET_ARCH_IA32)
@@ -1509,7 +1509,6 @@
int heap_growth_max,
int garbage_collection_time_ratio)
: heap_(heap),
- is_enabled_(false),
heap_growth_ratio_(heap_growth_ratio),
desired_utilization_((100.0 - heap_growth_ratio) / 100.0),
heap_growth_max_(heap_growth_max),
@@ -1522,9 +1521,6 @@
PageSpaceController::~PageSpaceController() {}
bool PageSpaceController::ReachedHardThreshold(SpaceUsage after) const {
- if (!is_enabled_) {
- return false;
- }
if (heap_growth_ratio_ == 100) {
return false;
}
@@ -1532,9 +1528,6 @@
}
bool PageSpaceController::ReachedSoftThreshold(SpaceUsage after) const {
- if (!is_enabled_) {
- return false;
- }
if (heap_growth_ratio_ == 100) {
return false;
}
@@ -1542,9 +1535,6 @@
}
bool PageSpaceController::ReachedIdleThreshold(SpaceUsage current) const {
- if (!is_enabled_) {
- return false;
- }
if (heap_growth_ratio_ == 100) {
return false;
}
diff --git a/runtime/vm/heap/pages.h b/runtime/vm/heap/pages.h
index aff20f3..36e8ab2 100644
--- a/runtime/vm/heap/pages.h
+++ b/runtime/vm/heap/pages.h
@@ -270,10 +270,6 @@
void set_last_usage(SpaceUsage current) { last_usage_ = current; }
- void Enable() { is_enabled_ = true; }
- void Disable() { is_enabled_ = false; }
- bool is_enabled() { return is_enabled_; }
-
private:
friend class PageSpace; // For MergeOtherPageSpaceController
@@ -285,8 +281,6 @@
Heap* heap_;
- bool is_enabled_;
-
// Usage after last evaluated GC or last enabled.
SpaceUsage last_usage_;
@@ -415,21 +409,6 @@
void AddRegionsToObjectSet(ObjectSet* set) const;
- void InitGrowthControl() {
- page_space_controller_.set_last_usage(usage_);
- page_space_controller_.Enable();
- }
-
- void SetGrowthControlState(bool state) {
- if (state) {
- page_space_controller_.Enable();
- } else {
- page_space_controller_.Disable();
- }
- }
-
- bool GrowthControlState() { return page_space_controller_.is_enabled(); }
-
// Note: Code pages are made executable/non-executable when 'read_only' is
// true/false, respectively.
void WriteProtect(bool read_only);
diff --git a/runtime/vm/heap/pages_test.cc b/runtime/vm/heap/pages_test.cc
index fed0a88..1db6113 100644
--- a/runtime/vm/heap/pages_test.cc
+++ b/runtime/vm/heap/pages_test.cc
@@ -10,7 +10,6 @@
TEST_CASE(Pages) {
PageSpace* space = new PageSpace(NULL, 4 * MBInWords);
- space->InitGrowthControl();
EXPECT(!space->Contains(reinterpret_cast<uword>(&space)));
uword block = space->TryAllocate(8 * kWordSize);
EXPECT(block != 0);
diff --git a/runtime/vm/heap/safepoint.cc b/runtime/vm/heap/safepoint.cc
index d75d06a..3853e9c 100644
--- a/runtime/vm/heap/safepoint.cc
+++ b/runtime/vm/heap/safepoint.cc
@@ -37,13 +37,10 @@
IsolateGroup* IG = T->isolate_group();
ASSERT(IG != NULL);
+ T->IncrementForceGrowthScopeDepth();
+
auto handler = IG->safepoint_handler();
handler->SafepointThreads(T, level_);
-
- // N.B.: Change growth policy inside the safepoint to prevent racy access.
- Heap* heap = IG->heap();
- current_growth_controller_state_ = heap->GrowthControlState();
- heap->DisableGrowthControl();
}
ForceGrowthSafepointOperationScope::~ForceGrowthSafepointOperationScope() {
@@ -52,16 +49,14 @@
IsolateGroup* IG = T->isolate_group();
ASSERT(IG != NULL);
- // N.B.: Change growth policy inside the safepoint to prevent racy access.
- Heap* heap = IG->heap();
- heap->SetGrowthControlState(current_growth_controller_state_);
-
auto handler = IG->safepoint_handler();
handler->ResumeThreads(T, level_);
- if (current_growth_controller_state_) {
+ T->DecrementForceGrowthScopeDepth();
+ if (!T->force_growth()) {
ASSERT(T->CanCollectGarbage());
// Check if we passed the growth limit during the scope.
+ Heap* heap = T->heap();
if (heap->old_space()->ReachedHardThreshold()) {
heap->CollectGarbage(T, GCType::kMarkSweep, GCReason::kOldSpace);
} else {
diff --git a/runtime/vm/heap/scavenger.cc b/runtime/vm/heap/scavenger.cc
index 5bce434..8dc0cfd 100644
--- a/runtime/vm/heap/scavenger.cc
+++ b/runtime/vm/heap/scavenger.cc
@@ -1678,7 +1678,7 @@
AbandonRemainingTLAB(thread);
- if (can_safepoint) {
+ if (can_safepoint && !thread->force_growth()) {
ASSERT(thread->no_safepoint_scope_depth() == 0);
heap_->CheckConcurrentMarking(thread, GCReason::kNewSpace, kNewPageSize);
}
diff --git a/runtime/vm/heap/scavenger.h b/runtime/vm/heap/scavenger.h
index 3c1051c..43eac30 100644
--- a/runtime/vm/heap/scavenger.h
+++ b/runtime/vm/heap/scavenger.h
@@ -349,16 +349,6 @@
void MakeNewSpaceIterable();
int64_t FreeSpaceInWords(Isolate* isolate) const;
- void InitGrowthControl() {
- growth_control_ = true;
- }
-
- void SetGrowthControlState(bool state) {
- growth_control_ = state;
- }
-
- bool GrowthControlState() { return growth_control_; }
-
bool scavenging() const { return scavenging_; }
// The maximum number of Dart mutator threads we allow to execute at the same
@@ -457,10 +447,6 @@
RelaxedAtomic<bool> failed_to_promote_;
RelaxedAtomic<bool> abort_;
- // When the isolate group is ready it will enable growth control via
- // InitGrowthControl.
- bool growth_control_ = false;
-
// Protects new space during the allocation of new TLABs
mutable Mutex space_lock_;
diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc
index f182dce..f40c12b 100644
--- a/runtime/vm/isolate_reload.cc
+++ b/runtime/vm/isolate_reload.cc
@@ -753,7 +753,7 @@
// "become" operation below replaces all the instances of the old
// size with forwarding corpses. Force heap growth to prevent size
// confusion during this period.
- NoHeapGrowthControlScope scope;
+ ForceGrowthScope force_growth(thread);
// The HeapIterationScope above ensures no other GC tasks can be
// active.
ASSERT(HasNoTasks(heap));
diff --git a/runtime/vm/thread.h b/runtime/vm/thread.h
index 906efbf..5f9660d 100644
--- a/runtime/vm/thread.h
+++ b/runtime/vm/thread.h
@@ -586,17 +586,25 @@
}
int32_t no_callback_scope_depth() const { return no_callback_scope_depth_; }
-
void IncrementNoCallbackScopeDepth() {
ASSERT(no_callback_scope_depth_ < INT_MAX);
no_callback_scope_depth_ += 1;
}
-
void DecrementNoCallbackScopeDepth() {
ASSERT(no_callback_scope_depth_ > 0);
no_callback_scope_depth_ -= 1;
}
+ bool force_growth() const { return force_growth_scope_depth_ != 0; }
+ void IncrementForceGrowthScopeDepth() {
+ ASSERT(force_growth_scope_depth_ < INT_MAX);
+ force_growth_scope_depth_ += 1;
+ }
+ void DecrementForceGrowthScopeDepth() {
+ ASSERT(force_growth_scope_depth_ > 0);
+ force_growth_scope_depth_ -= 1;
+ }
+
bool is_unwind_in_progress() const { return is_unwind_in_progress_; }
void StartUnwindError() {
@@ -1219,6 +1227,7 @@
mutable Monitor thread_lock_;
ApiLocalScope* api_reusable_scope_;
int32_t no_callback_scope_depth_;
+ int32_t force_growth_scope_depth_ = 0;
intptr_t no_reload_scope_depth_ = 0;
intptr_t stopped_mutators_scope_depth_ = 0;
#if defined(DEBUG)
diff --git a/runtime/vm/thread_test.cc b/runtime/vm/thread_test.cc
index 5faa11b..796faa2 100644
--- a/runtime/vm/thread_test.cc
+++ b/runtime/vm/thread_test.cc
@@ -303,9 +303,9 @@
intptr_t done_count = 0;
bool wait = true;
- EXPECT(isolate->group()->heap()->GrowthControlState());
+ EXPECT(!thread->force_growth());
- NoHeapGrowthControlScope no_heap_growth_scope;
+ ForceGrowthScope no_heap_growth_scope(thread);
for (intptr_t i = 0; i < kTaskCount; i++) {
Dart::thread_pool()->Run<SimpleTaskWithZoneAllocation>(
diff --git a/tools/VERSION b/tools/VERSION
index 702aecd..706b141 100644
--- a/tools/VERSION
+++ b/tools/VERSION
@@ -27,5 +27,5 @@
MAJOR 2
MINOR 18
PATCH 0
-PRERELEASE 239
+PRERELEASE 240
PRERELEASE_PATCH 0
\ No newline at end of file
