[vm/regexp] Follow-up to RegExp sharing change 438b81c4f2.

Avoid ZoneHandles unless needed, use field initializer, few more nits.

This is based on feedback on https://dart-review.googlesource.com/c/sdk/+/279747.
TEST=ci

Change-Id: I59b462ca70a912b270b089fb114d6029b7fa5856
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279903
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
This commit is contained in:
Alexander Aprelev 2023-01-27 00:43:59 +00:00 committed by Commit Queue
parent 5cbd424bdf
commit bb63927fed
6 changed files with 50 additions and 29 deletions

View file

@ -5,7 +5,9 @@
// Verifies that many isolate workers can use one RegExp.
import 'dart:isolate';
import "package:async_helper/async_helper.dart";
import 'package:async_helper/async_helper.dart';
import 'package:expect/expect.dart';
worker(List<dynamic> args) {
final re = args[0] as RegExp;
@ -14,7 +16,11 @@ worker(List<dynamic> args) {
final sendPort = args[2] as SendPort;
final sw = Stopwatch()..start();
while (sw.elapsedMilliseconds < 2000) {
re.firstMatch('h' * i * 1000);
final match = re.firstMatch('h' * i * 1000 + ' a b c ');
Expect.isNotNull(match);
Expect.equals(2, match!.groupCount);
Expect.equals('a b c ', match.group(0));
Expect.equals('a b c ', match.group(1));
}
sendPort.send(true);
}

View file

@ -7,7 +7,9 @@
// Verifies that many isolate workers can use one RegExp.
import 'dart:isolate';
import "package:async_helper/async_helper.dart";
import 'package:async_helper/async_helper.dart';
import 'package:expect/expect.dart';
worker(List<dynamic> args) {
final re = args[0] as RegExp;
@ -16,7 +18,11 @@ worker(List<dynamic> args) {
final sendPort = args[2] as SendPort;
final sw = Stopwatch()..start();
while (sw.elapsedMilliseconds < 2000) {
re.firstMatch('h' * i * 1000);
final match = re.firstMatch('h' * i * 1000 + ' a b c ');
Expect.isNotNull(match);
Expect.equals(2, match.groupCount);
Expect.equals('a b c ', match.group(0));
Expect.equals('a b c ', match.group(1));
}
sendPort.send(true);
}

View file

@ -183,23 +183,25 @@ void IRRegExpMacroAssembler::GenerateEntryBlock() {
StoreLocal(current_position_, Bind(Sub(start_index_push, length_push)));
{
const Library& lib = Library::ZoneHandle(Z, Library::CoreLibrary());
const Library& lib = Library::Handle(Library::CoreLibrary());
const Class& regexp_class =
Class::ZoneHandle(Z, lib.LookupClassAllowPrivate(Symbols::_RegExp()));
Class::Handle(lib.LookupClassAllowPrivate(Symbols::_RegExp()));
const Function& get_registers_function = Function::ZoneHandle(
Z, regexp_class.LookupFunctionAllowPrivate(Symbols::_getRegisters()));
// This will be replaced with correct constant value at Finalization.
num_registers_constant_instr = Int64Constant(0);
// The "0" placeholder constant will be replaced with correct value
// determined at the end of regexp graph construction in Finalization.
num_registers_constant_instr =
new (Z) ConstantInstr(Integer::ZoneHandle(Z, Integer::NewCanonical(0)));
StoreLocal(registers_, Bind(StaticCall(get_registers_function,
Bind(num_registers_constant_instr),
ICData::kStatic)));
const Function& get_backtracking_stack_function =
Function::ZoneHandle(Z, regexp_class.LookupFunctionAllowPrivate(
Symbols::_getBacktrackingStack()));
StoreLocal(stack_, Bind(StaticCall(get_backtracking_stack_function,
ICData::kStatic)));
const Field& backtracking_stack_field =
Field::ZoneHandle(Z, regexp_class.LookupStaticFieldAllowPrivate(
Symbols::_backtrackingStack()));
StoreLocal(stack_, Bind(LoadStaticField(backtracking_stack_field,
/*calls_initializer=*/true)));
}
ClearRegisters(0, saved_registers_count_ - 1);
@ -515,6 +517,13 @@ void IRRegExpMacroAssembler::StoreLocal(LocalVariable* local, Value* value) {
Do(new (Z) StoreLocalInstr(*local, value, InstructionSource()));
}
LoadStaticFieldInstr* IRRegExpMacroAssembler::LoadStaticField(
const Field& field,
bool calls_initializer) const {
return new (Z) LoadStaticFieldInstr(field, InstructionSource(),
calls_initializer, GetNextDeoptId());
}
void IRRegExpMacroAssembler::set_current_instruction(Instruction* instruction) {
current_instruction_ = instruction;
}
@ -1480,9 +1489,9 @@ void IRRegExpMacroAssembler::CheckStackLimit() {
void IRRegExpMacroAssembler::GrowStack() {
TAG();
const Library& lib = Library::ZoneHandle(Z, Library::CoreLibrary());
const Library& lib = Library::Handle(Library::CoreLibrary());
const Class& regexp_class =
Class::ZoneHandle(Z, lib.LookupClassAllowPrivate(Symbols::_RegExp()));
Class::Handle(lib.LookupClassAllowPrivate(Symbols::_RegExp()));
const Function& grow_backtracking_stack_function =
Function::ZoneHandle(Z, regexp_class.LookupFunctionAllowPrivate(

View file

@ -269,6 +269,9 @@ class IRRegExpMacroAssembler : public RegExpMacroAssembler {
LoadLocalInstr* LoadLocal(LocalVariable* local) const;
void StoreLocal(LocalVariable* local, Value* value);
LoadStaticFieldInstr* LoadStaticField(const Field& field,
bool calls_initializer = false) const;
Value* PushLocal(LocalVariable* local);
Value* PushRegisterIndex(intptr_t reg);

View file

@ -418,11 +418,11 @@ class ObjectPointerVisitor;
V(_WeakReference, "_WeakReference") \
V(_await, "_await") \
V(_awaitWithTypeCheck, "_awaitWithTypeCheck") \
V(_backtrackingStack, "_backtrackingStack") \
V(_classRangeCheck, "_classRangeCheck") \
V(_current, "_current") \
V(_ensureScheduleImmediate, "_ensureScheduleImmediate") \
V(_future, "_future") \
V(_getBacktrackingStack, "_getBacktrackingStack") \
V(_getRegisters, "_getRegisters") \
V(_growBacktrackingStack, "_growBacktrackingStack") \
V(_handleException, "_handleException") \

View file

@ -208,6 +208,8 @@ class _RegExpMatch implements RegExpMatch {
static const int _MATCH_PAIR = 2;
}
const _initialBacktrackingStackSize = 128;
@pragma("vm:entry-point")
class _RegExp implements RegExp {
@pragma("vm:external-name", "RegExp_factory")
@ -367,24 +369,17 @@ class _RegExp implements RegExp {
external List<int>? _ExecuteMatchSticky(String str, int start_index);
static Int32List _getRegisters(int registers_count) {
if (_registers == null || _registers!.length < registers_count) {
_registers = Int32List(registers_count);
var registers = _registers;
if (registers == null || registers.length < registers_count) {
_registers = registers = Int32List(registers_count);
}
return _registers!;
}
static Int32List _getBacktrackingStack() {
if (_backtrackingStack == null) {
const _initialBacktrackingStackSize = 128;
_backtrackingStack = Int32List(_initialBacktrackingStackSize);
}
return _backtrackingStack!;
return registers;
}
// TODO: Should we bound this to the same limit used by the irregexp interpreter
// for consistency?
static Int32List _growBacktrackingStack() {
final stack = _backtrackingStack!;
final stack = _backtrackingStack;
final newStack = Int32List(stack.length * 2);
for (int i = 0; i < stack.length; i++) {
newStack[i] = stack[i];
@ -394,7 +389,9 @@ class _RegExp implements RegExp {
}
static Int32List? _registers;
static Int32List? _backtrackingStack;
static Int32List _backtrackingStack =
Int32List(_initialBacktrackingStackSize);
}
class _AllMatchesIterable extends IterableBase<RegExpMatch> {