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

Reland elide long tree walks #49891

Merged
merged 6 commits into from
Feb 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
223 changes: 199 additions & 24 deletions packages/flutter/lib/src/foundation/assertions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,130 @@ typedef DiagnosticPropertiesTransformer = Iterable<DiagnosticsNode> Function(Ite
/// and other callbacks that collect information describing an error.
typedef InformationCollector = Iterable<DiagnosticsNode> Function();

/// Partial information from a stack frame for stack filtering purposes.
///
/// See also:
///
/// * [RepetitiveStackFrameFilter], which uses this class to compare against [StackFrame]s.
@immutable
class PartialStackFrame {
/// Creates a new [PartialStackFrame] instance. All arguments are required and
/// must not be null.
const PartialStackFrame({
@required this.package,
@required this.className,
@required this.method,
}) : assert(className != null),
assert(method != null),
assert(package != null);

/// An `<asynchronous suspension>` line in a stack trace.
static const PartialStackFrame asynchronousSuspension = PartialStackFrame(
package: '',
className: '',
method: 'asynchronous suspension',
);

/// The package to match, e.g. `package:flutter/src/foundation/assertions.dart`,
/// or `dart:ui/window.dart`.
final Pattern package;

/// The class name for the method.
///
/// On web, this is ignored, since class names are not available.
///
/// On all platforms, top level methods should use the empty string.
final String className;

/// The method name for this frame line.
///
/// On web, private methods are wrapped with `[]`.
final String method;

/// Tests whether the [StackFrame] matches the information in this
/// [PartialStackFrame].
bool matches(StackFrame stackFrame) {
final String stackFramePackage = '${stackFrame.packageScheme}:${stackFrame.package}/${stackFrame.packagePath}';
// Ideally this wouldn't be necessary.
// TODO(dnfield): https://github.com/dart-lang/sdk/issues/40117
if (kIsWeb) {
return package.allMatches(stackFramePackage).isNotEmpty
&& stackFrame.method == (method.startsWith('_') ? '[$method]' : method);
}
return package.allMatches(stackFramePackage).isNotEmpty
&& stackFrame.method == method
&& stackFrame.className == className;
}
}

/// A class that filters stack frames for additional filtering on
/// [FlutterError.defaultStackFilter].
abstract class StackFilter {
/// A const constructor to allow subclasses to be const.
const StackFilter();

/// Filters the list of [StackFrame]s by updating corrresponding indices in
/// `reasons`.
///
/// To elide a frame or number of frames, set the string
void filter(List<StackFrame> stackFrames, List<String> reasons);
}


/// A [StackFilter] that filters based on repeating lists of
/// [PartialStackFrame]s.
///
/// See also:
///
/// * [FlutterError.addDefaultStackFilter], a method to register additional
/// stack filters for [FlutterError.defaultStackFilter].
/// * [StackFrame], a class that can help with parsing stack frames.
/// * [PartialStackFrame], a class that helps match partial method information
/// to a stack frame.
class RepetitiveStackFrameFilter extends StackFilter {
/// Creates a new RepetitiveStackFrameFilter. All parameters are required and must not be
/// null.
const RepetitiveStackFrameFilter({
@required this.frames,
@required this.replacement,
}) : assert(frames != null),
assert(replacement != null);

/// The shape of this repetative stack pattern.
final List<PartialStackFrame> frames;

/// The number of frames in this pattern.
int get numFrames => frames.length;

/// The string to replace the frames with.
///
/// If the same replacement string is used multiple times in a row, the
/// [FlutterError.defaultStackFilter] will simply update a counter after this
/// line rather than repeating it.
final String replacement;

List<String> get _replacements => List<String>.filled(numFrames, replacement);

@override
void filter(List<StackFrame> stackFrames, List<String> reasons) {
for (int index = 0; index < stackFrames.length; index += 1) {
if (_matchesFrames(stackFrames.skip(index).take(numFrames).toList())) {
reasons.setRange(index, index + numFrames, _replacements);
index += numFrames - 1;
}
}
}

bool _matchesFrames(List<StackFrame> stackFrames) {
for (int index = 0; index < stackFrames.length; index++) {
if (!frames[index].matches(stackFrames[index])) {
return false;
}
}
return true;
}
}

abstract class _ErrorDiagnostic extends DiagnosticsProperty<List<Object>> {
/// This constructor provides a reliable hook for a kernel transformer to find
/// error messages that need to be rewritten to include object references for
Expand Down Expand Up @@ -653,6 +777,19 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti
_errorCount += 1;
}

static final List<StackFilter> _stackFilters = <StackFilter>[];

/// Adds a stack filtering function to [defaultStackFilter].
///
/// For example, the framework adds common patterns of element building to
/// elide tree-walking patterns in the stacktrace.
///
/// Added filters are checked in order of addition. The first matching filter
/// wins, and subsequent filters will not be checked.
static void addDefaultStackFilter(StackFilter filter) {
_stackFilters.add(filter);
}

/// Converts a stack to a string that is more readable by omitting stack
/// frames that correspond to Dart internals.
///
Expand All @@ -665,38 +802,76 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti
/// format but the frame numbers will not be consecutive (frames are elided)
/// and the final line may be prose rather than a stack frame.
static Iterable<String> defaultStackFilter(Iterable<String> frames) {
const Set<String> filteredPackages = <String>{
'dart:async-patch',
'dart:async',
'package:stack_trace',
};
const Set<String> filteredClasses = <String>{
'_AssertionError',
'_FakeAsync',
'_FrameCallbackEntry',
final Map<String, int> removedPackagesAndClasses = <String, int>{
'dart:async-patch': 0,
'dart:async': 0,
'package:stack_trace': 0,
'class _AssertionError': 0,
'class _FakeAsync': 0,
'class _FrameCallbackEntry': 0,
'class _Timer': 0,
'class _RawReceivePortImpl': 0,
};
int skipped = 0;

final List<StackFrame> parsedFrames = StackFrame.fromStackString(frames.join('\n'));

for (int index = 0; index < parsedFrames.length; index += 1) {
final StackFrame frame = parsedFrames[index];
final String className = 'class ${frame.className}';
final String package = '${frame.packageScheme}:${frame.package}';
if (removedPackagesAndClasses.containsKey(className)) {
skipped += 1;
removedPackagesAndClasses[className] += 1;
parsedFrames.removeAt(index);
index -= 1;
} else if (removedPackagesAndClasses.containsKey(package)) {
skipped += 1;
removedPackagesAndClasses[package] += 1;
parsedFrames.removeAt(index);
index -= 1;
}
}
final List<String> reasons = List<String>(parsedFrames.length);
for (final StackFilter filter in _stackFilters) {
filter.filter(parsedFrames, reasons);
}

final List<String> result = <String>[];
final List<String> skipped = <String>[];
for (final String line in frames) {
final StackFrame frameLine = StackFrame.fromStackTraceLine(line);
if (filteredClasses.contains(frameLine.className)) {
skipped.add('class ${frameLine.className}');
} else if (filteredPackages.contains(frameLine.packageScheme + ':' + frameLine.package)) {
skipped.add('package ${frameLine.packageScheme == 'dart' ? 'dart:' : ''}${frameLine.package}');
} else {
result.add(line);

// Collapse duplicated reasons.
for (int index = 0; index < parsedFrames.length; index += 1) {
final int start = index;
while (index < reasons.length - 1 && reasons[index] != null && reasons[index + 1] == reasons[index]) {
index++;
}
String suffix = '';
if (reasons[index] != null) {
if (index != start) {
suffix = ' (${index - start + 2} frames)';
} else {
suffix = ' (1 frame)';
}
}
final String resultLine = '${reasons[index] ?? parsedFrames[index].source}$suffix';
result.add(resultLine);
}
if (skipped.length == 1) {
result.add('(elided one frame from ${skipped.single})');
} else if (skipped.length > 1) {
final List<String> where = Set<String>.from(skipped).toList()..sort();

// Only include packages we actually elided from.
final List<String> where = <String>[
for (MapEntry<String, int> entry in removedPackagesAndClasses.entries)
if (entry.value > 0)
entry.key
]..sort();
if (skipped == 1) {
result.add('(elided one frame from ${where.single})');
} else if (skipped > 1) {
if (where.length > 1)
where[where.length - 1] = 'and ${where.last}';
if (where.length > 2) {
result.add('(elided ${skipped.length} frames from ${where.join(", ")})');
result.add('(elided $skipped frames from ${where.join(", ")})');
} else {
result.add('(elided ${skipped.length} frames from ${where.join(" ")})');
result.add('(elided $skipped frames from ${where.join(" ")})');
}
}
return result;
Expand Down
84 changes: 84 additions & 0 deletions packages/flutter/lib/src/widgets/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,12 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB
void initInstances() {
super.initInstances();
_instance = this;

assert(() {
_debugAddStackFilters();
return true;
}());

// Initialization of [_buildOwner] has to be done after
// [super.initInstances] is called, as it requires [ServicesBinding] to
// properly setup the [defaultBinaryMessenger] instance.
Expand All @@ -265,6 +271,84 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB
FlutterErrorDetails.propertiesTransformers.add(transformDebugCreator);
}

void _debugAddStackFilters() {
const PartialStackFrame elementInflateWidget = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'Element', method: 'inflateWidget');
const PartialStackFrame elementUpdateChild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'Element', method: 'updateChild');
const PartialStackFrame elementRebuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'Element', method: 'rebuild');
const PartialStackFrame componentElementPerformRebuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'ComponentElement', method: 'performRebuild');
const PartialStackFrame componentElementFristBuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'ComponentElement', method: '_firstBuild');
const PartialStackFrame componentElementMount = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'ComponentElement', method: 'mount');
const PartialStackFrame statefulElementFristBuild = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'StatefulElement', method: '_firstBuild');
const PartialStackFrame singleChildMount = PartialStackFrame(package: 'package:flutter/src/widgets/framework.dart', className: 'SingleChildRenderObjectElement', method: 'mount');

const String replacementString = '... Normal element mounting';

// ComponentElement variations
FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter(
frames: <PartialStackFrame>[
elementInflateWidget,
elementUpdateChild,
componentElementPerformRebuild,
elementRebuild,
componentElementFristBuild,
componentElementMount,
],
replacement: replacementString,
));
FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter(
frames: <PartialStackFrame>[
elementUpdateChild,
componentElementPerformRebuild,
elementRebuild,
componentElementFristBuild,
componentElementMount,
],
replacement: replacementString,
));

// StatefulElement variations
FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter(
frames: <PartialStackFrame>[
elementInflateWidget,
elementUpdateChild,
componentElementPerformRebuild,
elementRebuild,
componentElementFristBuild,
statefulElementFristBuild,
componentElementMount,
],
replacement: replacementString,
));
FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter(
frames: <PartialStackFrame>[
elementUpdateChild,
componentElementPerformRebuild,
elementRebuild,
componentElementFristBuild,
statefulElementFristBuild,
componentElementMount,
],
replacement: replacementString,
));

// SingleChildRenderObjectElement variations
FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter(
frames: <PartialStackFrame>[
elementInflateWidget,
elementUpdateChild,
singleChildMount,
],
replacement: replacementString,
));
FlutterError.addDefaultStackFilter(const RepetitiveStackFrameFilter(
frames: <PartialStackFrame>[
elementUpdateChild,
singleChildMount,
],
replacement: replacementString,
));
}

/// The current [WidgetsBinding], if one has been created.
///
/// If you need the binding to be constructed before calling [runApp],
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter/test/foundation/error_reporting_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Future<void> main() async {
'#2 getSampleStack \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
'#3 main \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
'(.+\n)+' // TODO(ianh): when fixing #4021, also filter out frames from the test infrastructure below the first call to our main()
'\\(elided [0-9]+ frames from package dart:async\\)\n'
'\\(elided [0-9]+ frames from class _RawReceivePortImpl and dart:async\\)\n'
'\n'
'line 1 of extra information\n'
'line 2 of extra information\n'
Expand Down Expand Up @@ -153,7 +153,7 @@ Future<void> main() async {
'#2 getSampleStack \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
'#3 main \\([^)]+flutter/test/foundation/error_reporting_test\\.dart:[0-9]+:[0-9]+\\)\n'
'(.+\n)+' // TODO(ianh): when fixing #4021, also filter out frames from the test infrastructure below the first call to our main()
'\\(elided [0-9]+ frames from package dart:async\\)\n'
'\\(elided [0-9]+ frames from class _RawReceivePortImpl and dart:async\\)\n'
'\n'
'line 1 of extra information\n'
'line 2 of extra information\n'
Expand Down