mirror of
https://github.com/python/cpython.git
synced 2026-10-28 10:25:05 +08:00
gh-153568: Skip the expression precedence chain for single-token atoms (#153575)
A bare name or number followed by a token that cannot extend an expression now produces its AST node directly. This avoids the expression precedence chain for common arguments, assignments, and subscripts. Rules declare the hook with a fastpath flag. The Rule constructor exposes memo and fastpath attributes, and the C generator emits the hook call. Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
This commit is contained in:
1 parent
40c8edd2ca
commit
b77cb891e9
15 files changed
+202
-10
No files matched your search
+2
-2
@@ -747,7 +747,7 @@ named_expression[expr_ty]:
|
||||
| invalid_named_expression
|
||||
| expression !':='
|
||||
|
||||
disjunction[expr_ty] (memo):
|
||||
disjunction[expr_ty] (memo, fastpath=_PyPegen_atom_fast_path):
|
||||
| a=conjunction b=('or' c=conjunction { c })+ { _PyAST_BoolOp(
|
||||
Or,
|
||||
CHECK(asdl_expr_seq*, _PyPegen_seq_insert_in_front(p, a, b)),
|
||||
@@ -761,7 +761,7 @@ conjunction[expr_ty] (memo):
|
||||
EXTRA) }
|
||||
| inversion
|
||||
|
||||
inversion[expr_ty] (memo):
|
||||
inversion[expr_ty] (memo, fastpath=_PyPegen_atom_fast_path):
|
||||
| 'not' a=inversion { _PyAST_UnaryOp(Not, a, EXTRA) }
|
||||
| comparison
|
||||
|
||||
|
||||
@@ -616,6 +616,51 @@ actions. As with memoization, actions must not invalidate cached results. In
|
||||
particular, suffix actions must not move the parser before their starting mark,
|
||||
rewrite buffered input, or replace memo entries for earlier positions.
|
||||
|
||||
Fast-path hooks
|
||||
---------------
|
||||
|
||||
Some rules are entered so often that even the bookkeeping of trying their
|
||||
alternatives is expensive. Consider parsing the argument ``a`` in a call
|
||||
like ``f(a, b)``: the argument is a full expression, so the parser descends
|
||||
the whole operator precedence chain
|
||||
|
||||
```
|
||||
expression -> disjunction -> conjunction -> inversion -> comparison
|
||||
-> bitwise_or -> bitwise_xor -> bitwise_and -> shift_expr -> sum
|
||||
-> term -> factor -> power -> await_primary -> primary -> atom
|
||||
```
|
||||
|
||||
before it can produce the ``Name`` node for ``a``: around fourteen rule
|
||||
invocations, each paying its own C-stack check and memoization lookups, to
|
||||
consume a single token. But since the following token is ``,``, which
|
||||
cannot continue any binary or postfix expression, that outcome is already
|
||||
known after peeking at two tokens.
|
||||
|
||||
For cases like this a rule can declare a hand-written C hook with the
|
||||
``fastpath`` flag, next to where ``memo`` goes:
|
||||
|
||||
```
|
||||
disjunction[expr_ty] (memo, fastpath=_PyPegen_atom_fast_path):
|
||||
```
|
||||
|
||||
The generator calls the hook on rule entry, before any alternative is tried:
|
||||
|
||||
```c
|
||||
if (_PyPegen_atom_fast_path(p, &_res)) {
|
||||
p->level--;
|
||||
return _res;
|
||||
}
|
||||
```
|
||||
|
||||
The hook receives the parser and a pointer to the rule's result variable and
|
||||
returns 1 if it handled the parse (storing its result, which can be ``NULL``
|
||||
to signal failure with ``p->error_indicator`` set) or 0 to fall through to
|
||||
the rule's normal alternatives. The generator knows nothing about what the
|
||||
hook does: all parsing logic lives in the hook itself, next to the other
|
||||
token helpers in [`Parser/pegen.c`](../Parser/pegen.c). A hook must behave
|
||||
exactly like the rule it accelerates, including which tokens it fills,
|
||||
because error reporting depends on the number of tokens read (``p->fill``).
|
||||
|
||||
Automatic variables
|
||||
-------------------
|
||||
|
||||
|
||||
@@ -63,7 +63,7 @@ class TestCGenerator(unittest.TestCase):
|
||||
@modulename 'sample'
|
||||
@bytecode '1'
|
||||
start[mod_ty]: expr_without_invalid 'pass' "zsoft" "asoft" ('bb' | 'aa')* ENDMARKER
|
||||
expr_without_invalid[expr_ty] (memo): name=expr [NUMBER] { name }
|
||||
expr_without_invalid[expr_ty] (memo, fastpath=custom_fastpath): name=expr [NUMBER] { name }
|
||||
expr[expr_ty]: expr '+' NAME | NAME
|
||||
""")
|
||||
generator.debug = True
|
||||
@@ -86,6 +86,8 @@ class TestCGenerator(unittest.TestCase):
|
||||
rule.name = "changed"
|
||||
rule.type = "changed_type"
|
||||
rule.flags = frozenset()
|
||||
rule.memo = False
|
||||
rule.fastpath = None
|
||||
rule.rhs.alts[0].action = "changed_action"
|
||||
rule.rhs.alts.clear()
|
||||
generator.rules.clear()
|
||||
|
||||
@@ -464,6 +464,47 @@ class TestCParser(unittest.TestCase):
|
||||
"""
|
||||
self.run_test(grammar_source, test_source)
|
||||
|
||||
def test_rule_fastpath(self) -> None:
|
||||
grammar_source = """
|
||||
@subheader '''
|
||||
#define CHECK_INVALID(expected) \\
|
||||
(assert(p->call_invalid_rules == (expected)), _PyPegen_dummy_name(p))
|
||||
|
||||
static int
|
||||
number_fastpath(Parser *p, void *result)
|
||||
{
|
||||
assert(p->call_invalid_rules == 0);
|
||||
expr_ty number = _PyPegen_number_token(p);
|
||||
if (number != NULL) {
|
||||
*(void **)result = number;
|
||||
return 1;
|
||||
}
|
||||
if (_PyPegen_expect_token(p, LPAR)) {
|
||||
p->error_indicator = 1;
|
||||
PyErr_SetString(PyExc_ValueError, "fastpath error");
|
||||
}
|
||||
if (p->error_indicator) {
|
||||
*(void **)result = NULL;
|
||||
return 1;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
'''
|
||||
start: enable value_without_invalid after NEWLINE ENDMARKER
|
||||
enable: 'enable' { (p->call_invalid_rules = 1, _PyPegen_dummy_name(p)) }
|
||||
value_without_invalid (memo, fastpath=number_fastpath): NAME { CHECK_INVALID(0) }
|
||||
after: NAME { CHECK_INVALID(1) }
|
||||
"""
|
||||
test_source = """
|
||||
self.check_input_strings_for_grammar(
|
||||
valid_cases=['enable 42 tail', 'enable name tail'],
|
||||
invalid_cases=['enable + tail', 'enable 42'],
|
||||
)
|
||||
with self.assertRaisesRegex(ValueError, 'fastpath error'):
|
||||
parse.parse_string('enable ( tail', mode=0)
|
||||
"""
|
||||
self.run_test(grammar_source, test_source)
|
||||
|
||||
def test_left_recursive_rule_cleanup(self) -> None:
|
||||
grammar_source = """
|
||||
@subheader '''
|
||||
|
||||
@@ -1181,32 +1181,53 @@ class TestGrammarVisualizer(unittest.TestCase):
|
||||
|
||||
# Test memo-only rule
|
||||
simple_rule = rules['simple_rule']
|
||||
self.assertTrue('memo' in simple_rule.flags,
|
||||
self.assertTrue(simple_rule.memo,
|
||||
"simple_rule should have memo")
|
||||
self.assertEqual(simple_rule.flags, frozenset(['memo']),
|
||||
f"simple_rule flags should be {'memo'}, got {simple_rule.flags}")
|
||||
|
||||
# Test multi-flag rule
|
||||
multi_flag_rule = rules['multi_flag_rule']
|
||||
self.assertTrue('memo' in simple_rule.flags,
|
||||
self.assertTrue(multi_flag_rule.memo,
|
||||
"multi_flag_rule should have memo")
|
||||
self.assertEqual(multi_flag_rule.flags, frozenset({'memo', 'custom', 'test'}),
|
||||
f"multi_flag_rule flags should contain memo, custom, test, got {multi_flag_rule.flags}")
|
||||
|
||||
# Test single custom flag rule
|
||||
single_custom_rule = rules['single_custom_flag']
|
||||
self.assertFalse('memo' not in simple_rule.flags,
|
||||
self.assertFalse(single_custom_rule.memo,
|
||||
"single_custom_flag should not have memo")
|
||||
self.assertEqual(single_custom_rule.flags, frozenset(['custom']),
|
||||
f"single_custom_flag flags should be {'custom'}, got {single_custom_rule.flags}")
|
||||
|
||||
# Test no flags rule
|
||||
no_flags_rule = rules['no_flags_rule']
|
||||
self.assertFalse('memo' not in simple_rule.flags,
|
||||
self.assertFalse(no_flags_rule.memo,
|
||||
"no_flags_rule should not have memo")
|
||||
self.assertEqual(no_flags_rule.flags, frozenset(),
|
||||
f"no_flags_rule flags should be the empty set, got {no_flags_rule.flags}")
|
||||
|
||||
for rule in rules.values():
|
||||
self.assertIsNone(rule.fastpath)
|
||||
|
||||
def test_rule_fastpath_flags(self) -> None:
|
||||
for rule_type in ("", "[expr_ty]"):
|
||||
for memo in (False, True):
|
||||
with self.subTest(rule_type=rule_type, memo=memo):
|
||||
flags = "fastpath=custom_fastpath"
|
||||
if memo:
|
||||
flags = "memo, " + flags
|
||||
grammar_source = f"start{rule_type} ({flags}): NAME\n"
|
||||
rule = parse_string(grammar_source, GrammarParser).rules["start"]
|
||||
self.assertEqual(rule.fastpath, "custom_fastpath")
|
||||
self.assertIs(rule.memo, memo)
|
||||
|
||||
def test_rule_fastpath_requires_function(self) -> None:
|
||||
with self.assertRaisesRegex(
|
||||
ValueError, "rule 'start': fastpath flag needs a function"
|
||||
):
|
||||
parse_string("start (fastpath): NAME\n", GrammarParser)
|
||||
|
||||
|
||||
class TestGrammarIncludes(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
Speed up parsing of simple expressions by recognizing single-token atoms
|
||||
without descending the full precedence rule chain.
|
||||
Generated
+8
@@ -12299,6 +12299,10 @@ disjunction_rule(Parser *p)
|
||||
return NULL;
|
||||
}
|
||||
expr_ty _res = NULL;
|
||||
if (_PyPegen_atom_fast_path(p, &_res)) {
|
||||
p->level--;
|
||||
return _res;
|
||||
}
|
||||
if (_PyPegen_is_memoized(p, disjunction_type, &_res)) {
|
||||
p->level--;
|
||||
return _res;
|
||||
@@ -12479,6 +12483,10 @@ inversion_rule(Parser *p)
|
||||
return NULL;
|
||||
}
|
||||
expr_ty _res = NULL;
|
||||
if (_PyPegen_atom_fast_path(p, &_res)) {
|
||||
p->level--;
|
||||
return _res;
|
||||
}
|
||||
if (_PyPegen_is_memoized(p, inversion_type, &_res)) {
|
||||
p->level--;
|
||||
return _res;
|
||||
|
||||
@@ -328,6 +328,50 @@ _resize_tokens_array(Parser *p) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
// Fast path attached to the expression precedence chain's entry rules.
|
||||
// A bare NAME/NUMBER atom followed by a token that cannot start or
|
||||
// continue a binary/postfix expression is a complete expression, so it is
|
||||
// built directly instead of descending the whole chain.
|
||||
//
|
||||
// Returns 1 if it produced a result (stored in *result), 0 to fall through to
|
||||
// the rule's alternatives.
|
||||
int
|
||||
_PyPegen_atom_fast_path(Parser *p, void *result)
|
||||
{
|
||||
if (p->mark == p->fill && _PyPegen_fill_token(p) < 0) {
|
||||
p->error_indicator = 1;
|
||||
*(void **)result = NULL;
|
||||
return 1;
|
||||
}
|
||||
Token *t = p->tokens[p->mark];
|
||||
if (t->type != NAME && t->type != NUMBER) {
|
||||
return 0;
|
||||
}
|
||||
if (p->mark + 1 == p->fill && _PyPegen_fill_token(p) < 0) {
|
||||
p->error_indicator = 1;
|
||||
*(void **)result = NULL;
|
||||
return 1;
|
||||
}
|
||||
switch (p->tokens[p->mark + 1]->type) {
|
||||
case COMMA:
|
||||
case RPAR:
|
||||
case RSQB:
|
||||
case RBRACE:
|
||||
case COLON:
|
||||
case NEWLINE:
|
||||
case SEMI:
|
||||
case EQUAL:
|
||||
case ENDMARKER:
|
||||
break;
|
||||
default:
|
||||
return 0;
|
||||
}
|
||||
expr_ty res = (t->type == NAME)
|
||||
? _PyPegen_name_token(p) : _PyPegen_number_token(p);
|
||||
*(void **)result = res;
|
||||
return 1;
|
||||
}
|
||||
|
||||
int
|
||||
_PyPegen_fill_token(Parser *p)
|
||||
{
|
||||
|
||||
@@ -196,6 +196,7 @@ Token *_PyPegen_get_last_nonnwhitespace_token(Parser *);
|
||||
int _PyPegen_fill_token(Parser *p);
|
||||
// Return -1 on failure, leaving the buffer and capacity unchanged.
|
||||
int _PyPegen_grow_loop_buffer(void ***buffer, Py_ssize_t *capacity);
|
||||
int _PyPegen_atom_fast_path(Parser *p, void *result);
|
||||
expr_ty _PyPegen_name_token(Parser *p);
|
||||
expr_ty _PyPegen_number_token(Parser *p);
|
||||
void *_PyPegen_string_token(Parser *p);
|
||||
|
||||
@@ -381,7 +381,7 @@ class CCallLowerer:
|
||||
rule = original_rules.get(alt.items[0].item.value)
|
||||
if rule is None or rule.name not in consuming:
|
||||
return None
|
||||
if ("memo" in rule.flags and not rule.left_recursive) or (
|
||||
if (rule.memo and not rule.left_recursive) or (
|
||||
rule.left_recursive and rule.leader
|
||||
):
|
||||
return rule
|
||||
@@ -426,7 +426,8 @@ class CCallLowerer:
|
||||
),
|
||||
left_recursive=rule.left_recursive,
|
||||
leader=rule.leader,
|
||||
memoize="memo" in rule.flags and not rule.left_recursive,
|
||||
memoize=rule.memo and not rule.left_recursive,
|
||||
fastpath=rule.fastpath,
|
||||
disable_invalid_rules=rule.name.endswith("without_invalid"),
|
||||
prefixes=self._prefixes.get(rule.name, ()),
|
||||
source=rule.source,
|
||||
|
||||
@@ -123,6 +123,7 @@ class CRule:
|
||||
left_recursive: bool
|
||||
leader: bool
|
||||
memoize: bool
|
||||
fastpath: str | None
|
||||
disable_invalid_rules: bool
|
||||
prefixes: tuple[CPrefix, ...] = ()
|
||||
source: pathlib.Path | None = None
|
||||
|
||||
@@ -176,6 +176,11 @@ class CRuleEmitter:
|
||||
self._emit_recursion_check()
|
||||
self._emit_error_check()
|
||||
self._print(f"{result_type} _res = NULL;")
|
||||
if fastpath := self._rule.fastpath:
|
||||
self._print(f"if ({fastpath}(p, &_res)) {{")
|
||||
with self._indent():
|
||||
self._returns.emit("_res")
|
||||
self._print("}")
|
||||
if memoize:
|
||||
self._print(f"if (_PyPegen_is_memoized(p, {signature.name}_type, &_res)) {{")
|
||||
with self._indent():
|
||||
|
||||
@@ -90,6 +90,17 @@ class Rule:
|
||||
self.type = type
|
||||
self.rhs = rhs
|
||||
self.flags = flags or frozenset()
|
||||
self.memo = "memo" in self.flags
|
||||
self.fastpath: str | None = None
|
||||
for flag in self.flags:
|
||||
name, _, function = flag.partition("=")
|
||||
if name == "fastpath":
|
||||
if not function:
|
||||
raise ValueError(
|
||||
f"rule {self.name!r}: fastpath flag needs a function"
|
||||
)
|
||||
self.fastpath = function
|
||||
break
|
||||
self.kind = kind
|
||||
# The included grammar file that defines this rule, relative to the
|
||||
# root grammar file's directory, or None for the root grammar file itself.
|
||||
|
||||
+10
-1
@@ -235,8 +235,17 @@ class GeneratedParser(Parser):
|
||||
|
||||
@memoize
|
||||
def flag(self) -> Optional[str]:
|
||||
# flag: NAME
|
||||
# flag: NAME '=' NAME | NAME
|
||||
mark = self._mark()
|
||||
if (
|
||||
(a := self.name())
|
||||
and
|
||||
(literal := self.expect('='))
|
||||
and
|
||||
(b := self.name())
|
||||
):
|
||||
return a . string + "=" + b . string
|
||||
self._reset(mark)
|
||||
if (
|
||||
(name := self.name())
|
||||
):
|
||||
|
||||
@@ -64,6 +64,7 @@ flags[frozenset[str]]:
|
||||
| '(' a=','.flag+ ')' { frozenset(a) }
|
||||
|
||||
flag[str]:
|
||||
| a=NAME '=' b=NAME { a.string + "=" + b.string }
|
||||
| NAME { name.string }
|
||||
|
||||
alts[Rhs]:
|
||||
|
||||
Reference in new issue
Block a user