dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[analyzer] Replace regex in ignore_info with normal code

Browse Source 
The RegEx engine in the VM was updated in
e443b89f23 which caused the analyzer
analyzing the CFE to use ~150 mio instructions more.

Part of this was an increased cost in ignore comment processing which
relied on regex. Using regex before the updated engine made
`processPrecedingComments` have a cost of ~240 mio instructions,
updating the regex engine took that to ~264 mio instructions.

This CL gets rid of the regex and takes the cost of
`processPrecedingComments` to ~74.4 mio instructions a saving of about
189 mio instructions (all then analyzing the CFE and looking at output
from `valgrind --tool=callgrind`).

Benchmarking with `perf stat` with normal GC gives:

```
task-clock:u: -2.1760% +/- 1.6063% (-265608341.80 +/- 196062087.98) (12206056036.20 -> 11940447694.40)
page-faults:u: 0.2301% +/- 0.0313% (448.20 +/- 60.88) (194764.60 -> 195212.80)
cycles:u: -2.2906% +/- 1.6090% (-1180119481.60 +/- 828994273.75) (51521138476.00 -> 50341018994.40)
instructions:u: -0.3325% +/- 0.0032% (-196547942.60 +/- 1874215.18) (59120651337.60 -> 58924103395.00)
seconds time elapsed: -2.1715% +/- 1.6011% (-0.27 +/- 0.20) (12.21 -> 11.95)
seconds user: -2.2487% +/- 1.7816% (-0.27 +/- 0.21) (11.87 -> 11.60)

Comparing GC data:
'No' GC change.
```

Note that it must push the GC - the savings isn't really 2% in time.

And with GC disabled:

```
instructions:u: -0.4562% +/- 0.0029% (-185499444.00 +/- 1189012.77) (40663084597.80 -> 40477585153.80)
```

So here a saving of ~185 mio which fits okay with the data from
valgrind.

Change-Id: Ib203baeac6a93f5e37c737080fed342dbd0740a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/487021
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Jens Johansen
2026-03-13 02:43:23 -07:00
committed by Commit Queue
parent cf2cf18751
commit 2849d34c7c
4 changed files with 138 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analysis_server/lib/src/services/correction/organize_imports.dart
+1 -1
View File
@@ -359,7 +359,7 @@ class ImportOrganizer {
/// Returns whether this token is a '// ignore:' comment (but not an
/// '// ignore_for_file:' comment).
static bool _isIgnoreComment(Token token) =>
IgnoreInfo.ignoreMatcher.matchAsPrefix(token.lexeme) != null;
IgnoreInfo.isIgnoreComment(token.lexeme);
static bool _isLibraryTargetAnnotation(Annotation annotation) =>
annotation.elementAnnotation?.targetKinds.contains(TargetKind.library) ??
pkg/analysis_server_plugin/lib/src/correction/ignore_diagnostic.dart
+2 -2
View File
@@ -171,7 +171,7 @@ class IgnoreDiagnosticInFile extends _DartIgnoreDiagnostic {
var line = source.substring(lineStart, nextLineStart);
var trimmedLine = line.trim();
if (trimmedLine.startsWith(IgnoreInfo.ignoreForFileMatcher)) {
if (IgnoreInfo.isIgnoreForFileComment(trimmedLine)) {
// Found an existing ignore; insert after `// ignore_for_file: `
// before any existing codes.
var insertOffset = lineStart + line.indexOf(':') + 1;
@@ -243,7 +243,7 @@ class IgnoreDiagnosticOnLine extends _DartIgnoreDiagnostic {
var lineStart = unitResult.lineInfo.getOffsetOfLine(lineNumber);
var line = unitResult.content.substring(previousLineStart, lineStart);
if (line.trim().startsWith(IgnoreInfo.ignoreMatcher)) {
if (IgnoreInfo.isIgnoreComment(line.trim())) {
// Add after the `// ignore: ` before any existing codes.
var insertOffset = previousLineStart + line.indexOf(':') + 1;
builder.addSimpleInsertion(insertOffset, ' $_code,');
pkg/analyzer/lib/src/ignore_comments/ignore_info.dart
+80 -12
View File
@@ -2,6 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:_fe_analyzer_shared/src/scanner/characters.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/diagnostic/diagnostic.dart';
@@ -83,16 +84,6 @@ sealed class IgnoredElement {
/// Information about analysis `//ignore:` and `//ignore_for_file:` comments
/// within a source file.
class IgnoreInfo {
/// A regular expression for matching 'ignore' comments.
///
/// Resulting codes may be in a list (e.g. 'error_code_1,error_code2').
static final RegExp ignoreMatcher = RegExp(r'//+[ ]*ignore:');
/// A regular expression for matching 'ignore_for_file' comments.
///
/// Resulting codes may be in a list (e.g. 'error_code_1,error_code2').
static final RegExp ignoreForFileMatcher = RegExp(r'//[ ]*ignore_for_file:');
/// A regular expression for matching 'ignore' comments in a .yaml file.
///
/// Resulting codes may be in a list (e.g. 'error_code_1,error_code2').
@@ -219,6 +210,83 @@ class IgnoreInfo {
(name) => name._matches(diagnosticCode, pluginName: pluginName),
);
}
/// If [s] starts as a 'ignore' comments.
///
/// Resulting codes may be in a list (e.g. 'error_code_1,error_code2').
static bool isIgnoreComment(String s) {
int end = s.length;
if (end < 9) return false;
// Require 2 slashes.
if (s.codeUnitAt(0) != $SLASH) return false;
if (s.codeUnitAt(1) != $SLASH) return false;
// Allow more slashes.
int from = 2;
for (; from < end; from++) {
if (s.codeUnitAt(from) != $SLASH) break;
}
// Skip any spaces.
for (; from < end; from++) {
if (s.codeUnitAt(from) != $SPACE) break;
}
// Does the string match 'ignore:' now?
if (end - from < 7) return false;
if (s.codeUnitAt(from++) != $i) return false;
if (s.codeUnitAt(from++) != $g) return false;
if (s.codeUnitAt(from++) != $n) return false;
if (s.codeUnitAt(from++) != $o) return false;
if (s.codeUnitAt(from++) != $r) return false;
if (s.codeUnitAt(from++) != $e) return false;
if (s.codeUnitAt(from++) != $COLON) return false;
return true;
}
/// If [s] starts as a 'ignore_for_file' comments.
///
/// Resulting codes may be in a list (e.g. 'error_code_1,error_code2').
static bool isIgnoreForFileComment(String s) {
int end = s.length;
if (end < 18) return false;
// Require 2 slashes.
if (s.codeUnitAt(0) != $SLASH) return false;
if (s.codeUnitAt(1) != $SLASH) return false;
// We don't (currently?) allow more than 2 slashes.
int from = 2;
// for (; from < end; from++) {
// if (s.codeUnitAt(from) != $SLASH) break;
// }
// Skip any spaces.
for (; from < end; from++) {
if (s.codeUnitAt(from) != $SPACE) break;
}
// Does the string match 'ignore_for_file:' now?
if (end - from < 16) return false;
if (s.codeUnitAt(from++) != $i) return false;
if (s.codeUnitAt(from++) != $g) return false;
if (s.codeUnitAt(from++) != $n) return false;
if (s.codeUnitAt(from++) != $o) return false;
if (s.codeUnitAt(from++) != $r) return false;
if (s.codeUnitAt(from++) != $e) return false;
if (s.codeUnitAt(from++) != $_) return false;
if (s.codeUnitAt(from++) != $f) return false;
if (s.codeUnitAt(from++) != $o) return false;
if (s.codeUnitAt(from++) != $r) return false;
if (s.codeUnitAt(from++) != $_) return false;
if (s.codeUnitAt(from++) != $f) return false;
if (s.codeUnitAt(from++) != $i) return false;
if (s.codeUnitAt(from++) != $l) return false;
if (s.codeUnitAt(from++) != $e) return false;
if (s.codeUnitAt(from++) != $COLON) return false;
return true;
}
}
extension CommentTokenExtension on CommentToken {
@@ -412,9 +480,9 @@ extension CompilationUnitExtension on CompilationUnit {
var comment = currentToken.precedingComments;
while (comment != null) {
var lexeme = comment.lexeme;
if (lexeme.startsWith(IgnoreInfo.ignoreMatcher)) {
if (IgnoreInfo.isIgnoreComment(lexeme)) {
result.add(comment);
} else if (lexeme.startsWith(IgnoreInfo.ignoreForFileMatcher)) {
} else if (IgnoreInfo.isIgnoreForFileComment(lexeme)) {
result.add(comment);
}
comment = comment.next as CommentToken?;
pkg/analyzer/test/src/ignore_comments/ignore_info_test.dart
+55 -3
View File
@@ -2,7 +2,6 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/src/dart/ast/token.dart';
import 'package:analyzer/src/ignore_comments/ignore_info.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -17,6 +16,55 @@ main() {
@reflectiveTest
class IgnoreInfoTest extends PubPackageResolutionTest {
test_allows_several_slashes_and_spaces() async {
var ignoredElements = await _parseIgnoredElements(
'/////// ignore: type =lint',
);
expect(ignoredElements, hasLength(1));
_expectIgnoredType(
ignoredElements[0],
type: 'lint',
offset: 19,
length: 10,
);
}
test_ignore_for_file() async {
var ignoredElements = await _parseIgnoredElements(
'// ignore_for_file: type =lint',
);
expect(ignoredElements, hasLength(1));
_expectIgnoredType(
ignoredElements[0],
type: 'lint',
offset: 20,
length: 10,
);
}
test_ignore_for_file_several_spaces() async {
var ignoredElements = await _parseIgnoredElements(
'// ignore_for_file: type =lint',
);
expect(ignoredElements, hasLength(1));
_expectIgnoredType(
ignoredElements[0],
type: 'lint',
offset: 23,
length: 10,
);
}
test_ignore_in_file_needs_exactly_two_slashes() async {
var ignoredElements = await _parseIgnoredElements(
'/// ignore_on_file: type =lint',
);
if (ignoredElements.length == 1) {
print((ignoredElements[0] as dynamic).st);
}
expect(ignoredElements, hasLength(0));
}
test_name_multiple() async {
var ignoredElements = await _parseIgnoredElements('// ignore: foo, bar');
expect(ignoredElements, hasLength(2));
@@ -237,7 +285,11 @@ class IgnoreInfoTest extends PubPackageResolutionTest {
$comment
int x = 1;
''');
var commentToken = result.unit.beginToken.precedingComments as CommentToken;
return commentToken.ignoredElements.toList();
// This corresponds roughly to what happens in `IgnoreInfo.forDart`.
List<IgnoredElement> ignoreResult = [];
for (var comment in result.unit.ignoreComments) {
ignoreResult.addAll(comment.ignoredElements);
}
return ignoreResult;
}
}