GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-143732: Add tier2 specialization for TO_BOOL by eendebakpt · Pull Request #148271 · python/cpython Conversation
|
| Benchmark | main | branch | Speedup |
|---|---|---|---|
| to_bool_dict_recorded | 1.21 sec | 852 ms | 1.41× |
| to_bool_tuple_recorded | 1.21 sec | 850 ms | 1.42× |
| to_bool_bytes_true | 1.20 sec | 852 ms | 1.41× |
| to_bool_bytes_false | 922 ms | 566 ms | 1.63× |
| to_bool_bytearray | 1.20 sec | 851 ms | 1.41× |
| to_bool_set_recorded | 1.23 sec | 849 ms | 1.45× |
| to_bool_frozenset | 1.23 sec | 850 ms | 1.45× |
| to_bool_kwargs_nonempty | 1.26 sec | 912 ms | 1.38× |
| to_bool_kwargs_empty | 952 ms | 576 ms | 1.65× |
| to_bool_varargs_nonempty | 1.23 sec | 892 ms | 1.38× |
| to_bool_varargs_empty | 933 ms | 574 ms | 1.62× |
| to_bool_set_literal | 26.3 ms | 24.6 ms | 1.07× |
| Geometric mean | (ref) | 1.43× |
"""TO_BOOL specialization benchmark.
Covers both the static (sym_get_type) and recorded (sym_get_probable_type)
paths through the JIT optimizer's _TO_BOOL rule.
Static path:
- kwargs dict (via **kwargs slot priming)
- varargs tuple (via *args slot priming)
- set literal (via _BUILD_SET)
Recorded path (only reached via _RECORD_TOS_TYPE; bytes/bytearray/frozenset
have no static-type construction at all):
- dict / tuple / bytes / bytearray / set / frozenset passed as parameter
"""
import pyperf
INNER_ITERS = 200
# ---- recorded-type path: value comes in as a parameter, no static type ----
def _recorded_inner(v):
cnt = 0
for _ in range(INNER_ITERS):
if v:
cnt += 1
return cnt
def to_bool_dict_recorded(n):
d = {"a": 1}
for _ in range(n):
_recorded_inner(d)
def to_bool_tuple_recorded(n):
t = (1, 2, 3)
for _ in range(n):
_recorded_inner(t)
def to_bool_bytes_true(n):
b = b"hello"
for _ in range(n):
_recorded_inner(b)
def to_bool_bytes_false(n):
b = b""
for _ in range(n):
_recorded_inner(b)
def to_bool_bytearray(n):
b = bytearray(b"hello")
for _ in range(n):
_recorded_inner(b)
def to_bool_set_recorded(n):
s = {1, 2, 3}
for _ in range(n):
_recorded_inner(s)
def to_bool_frozenset(n):
s = frozenset({1, 2, 3})
for _ in range(n):
_recorded_inner(s)
# ---- static-type path: kwargs / varargs slot priming ----
def _kwargs_inner(**kwargs):
cnt = 0
for _ in range(INNER_ITERS):
if kwargs:
cnt += 1
return cnt
def _varargs_inner(*args):
cnt = 0
for _ in range(INNER_ITERS):
if args:
cnt += 1
return cnt
def to_bool_kwargs_nonempty(n):
for _ in range(n):
_kwargs_inner(x=1, y=2)
def to_bool_kwargs_empty(n):
for _ in range(n):
_kwargs_inner()
def to_bool_varargs_nonempty(n):
for _ in range(n):
_varargs_inner(1, 2, 3)
def to_bool_varargs_empty(n):
for _ in range(n):
_varargs_inner()
# ---- static-type path: set literal via _BUILD_SET ----
def to_bool_set_literal(n):
cnt = 0
for _ in range(n):
s = {1}
if s:
cnt += 1
return cnt
N = 500_000
runner = pyperf.Runner()
runner.bench_func("to_bool_dict_recorded", to_bool_dict_recorded, N)
runner.bench_func("to_bool_tuple_recorded", to_bool_tuple_recorded, N)
runner.bench_func("to_bool_bytes_true", to_bool_bytes_true, N)
runner.bench_func("to_bool_bytes_false", to_bool_bytes_false, N)
runner.bench_func("to_bool_bytearray", to_bool_bytearray, N)
runner.bench_func("to_bool_set_recorded", to_bool_set_recorded, N)
runner.bench_func("to_bool_frozenset", to_bool_frozenset, N)
runner.bench_func("to_bool_kwargs_nonempty", to_bool_kwargs_nonempty, N)
runner.bench_func("to_bool_kwargs_empty", to_bool_kwargs_empty, N)
runner.bench_func("to_bool_varargs_nonempty", to_bool_varargs_nonempty, N)
runner.bench_func("to_bool_varargs_empty", to_bool_varargs_empty, N)
runner.bench_func("to_bool_set_literal", to_bool_set_literal, N)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| _REPLACE_WITH_TRUE + | ||
| POP_TOP; | ||
|
|
||
| tier2 op(_TO_BOOL_DICT, (value -- res)) { |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
You can merge this with _TO_BOOL_SIZED by using the fact that both do a fixed offset lookup.
In tier2 optimizer you can set the offset for where the size is stored and do size = (Py_ssize_t)((char *)obj + offset) and check that directly.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean. It goes into the internals of PyDict (e.g. not using PyDict_GET_SIZE, but doing manual offset calculations) and we also need to store the offset somewhere. So I think this is too much of a complication to get rid of a tier2 opcode.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
we also need to store the offset somewhere.
You can store it in the instruction operand0.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| REPLACE_OP(this_instr, _TO_BOOL_DICT, 0, 0); | ||
| } | ||
| else if (tp == &PyTuple_Type || | ||
| tp == &PySet_Type || |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect for set as it does not uses PyObject_VAR_HEAD, this works by accident because it has fill at that offset which is incorrect if set has dummy entries.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I updated the PR to handle the set/frozenset separately.
We can also use your suggestion to fold everything into the _TO_BOOL_SIZED. That means we have to load the offset at runtime (minor cost), but it does keep the number of ops lower. I implemented this in main...eendebakpt:to_bool_specialization_v2.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
That means we have to load the offset at runtime (minor cost), but it does keep the number of ops lower.
I don't think so, in the JIT the offset would be burned into the machine code itself so the offset is fixed and not looked up at runtime.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The problem with recording uops not being allowed after specializing uops has been fixed, so you can add a recording uop to _TO_BOOL and use the recorded information for better specialization.
#148285
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
@markshannon Adding the recording uop will help to specialize for several cases. But it will slow down (a tiny bit) the unspecialized cases. Should we make the recording uop change in a followup PR? And do you have an opinion on whether to use the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Take v2's parameterized _TO_BOOL_SIZED design (with size_offset operand) which subsumes _TO_BOOL_DICT and _TO_BOOL_ANY_SET. Sets/frozensets use PySetObject.used (not Py_SIZE, which aliases the dummy-counting fill slot). Regenerated all generated files from the merged source.
# Conflicts: # Include/internal/pycore_uop_ids.h # Lib/test/test_capi/test_opt.py # Python/optimizer_bytecodes.c # Python/optimizer_cases.c.h # Python/record_functions.c.h
Earlier merge resolutions silently dropped a sizable set of tests that exist on origin/main (test_get_iter_list, test_get_iter_gen, test_get_iter_trad, test_cached_load_special, all test_binary_op_extend_*, test_contains_op_frozendict_const_fold, test_not_contains_op_frozendict_const_fold, test_load_attr_getattribute_frame, etc.). Restore the file from origin/main and re-insert the eight TO_BOOL tests that are unique to this branch (test_to_bool_kwargs_dict, _empty_dict, _varargs_tuple, _empty_tuple, _args_and_kwargs, _args_kwargs_with_regular_params, _kwargs_only_no_varargs, _set_with_dummy_entries) right after test_to_bool_always_true. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
I decided to go with the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
# Conflicts: # Include/internal/pycore_uop_ids.h
| uops = get_opnames(ex) | ||
| self.assertNotIn("_REPLACE_WITH_TRUE", uops) | ||
|
|
||
| def test_to_bool_kwargs_dict(self): |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
These tests are too verbose, I would prefer if you combine several of them into one, not each case needs a different test.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done!
I also looked into refactoring the optimizer code to use a table based approach. The result is
If you want me to pull it into the branch give me a thumbs up.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This PR is stale because it has been open for 90 days with no activity. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.