| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
# Conflicts: # Lib/test/test_capi/test_opt.py # Python/specialize.c
|
This looks good overall, but I've not done a detailed review. I have a couple of general concerns about the BINARY_OP_EXTEND optimization in general, not in this PR, but something to keep in mind:
|
Sorry, something went wrong.
|
I note that you had to add a lot of new guard functions, most of which just check that the operands types are the same as those specified in the new lhs_type and rhs_type fields. In other words, the check becomes: This looks slower and more complex but it is more robust, since we don't need to worry about the guard function and the types being out of sync. The additional specializations should more than compensate in the interpreter, and we can easily eliminate the additional check in the JIT. If the guard function is not NULL, then both lhs_type and rhs_type should be NULL. |
Sorry, something went wrong.
There was a problem hiding this comment.
A few minor suggestions
Sorry, something went wrong.
…timizer The descriptor pointer was not being passed as an operand when adding _GUARD_BINARY_OP_EXTEND_LHS and _GUARD_BINARY_OP_EXTEND_RHS operations in the tier2 optimizer. This caused the executor to read garbage/NULL values from the inline cache, leading to assertion failures and crashes in JIT-compiled code. Fixed by passing the descriptor as a uintptr_t operand, following the same pattern used for other pointer-valued operations like _CALL_METHOD_DESCRIPTOR_*. Fixes CI failures where multiple test platforms were failing.
That was a nice suggestion. Less guards, and faster for most cases (no function call needed). Updated benchmarks:
|
Sorry, something went wrong.
|
Would you check if #148384 helps in this? |
Sorry, something went wrong.
| static PyObject * | ||
| str_int_multiply(PyObject *lhs, PyObject *rhs) | ||
| { | ||
| return seq_int_multiply(lhs, rhs, PyUnicode_Type.tp_as_sequence->sq_repeat); |
There was a problem hiding this comment.
This would still lookup the function pointer each time this gets called, have you tried exposing the function and using that directly?
Sorry, something went wrong.
There was a problem hiding this comment.
I considered it, but did not do it yet. With the current PR we get a performance increase and better type information in tier2 and I did not want to make too many changes. For some other ops exposing the function meant not having to create an extra method in specialize.c. Even when exposing PyUnicode_Type.tp_as_sequence->sq_repeat (which is unicode_repeat from unicodeobject.c) we would still need the str_int_multiply as unicode_repeat takes an int.
Exposing it and using it here would be another minor performance improvement though. So this let me know if you want me to make the change.
Sorry, something went wrong.
There was a problem hiding this comment.
It would make sense to expose the functions, like you have for _PyBytes_Concat for example, but it can wait for a future PR.
Sorry, something went wrong.
I'll run benchmarks later (will have to be on a stable machine, I suspect the difference will be small) |
Sorry, something went wrong.
| sym_set_type(right, d->rhs_type); | ||
| } | ||
|
|
||
| op(_GUARD_BINARY_OP_EXTEND, (descr/4, left, right -- left, right)) { |
There was a problem hiding this comment.
There is potential to optimize out the float/compact int and compact int/float guards.
Can be done in another PR.
Sorry, something went wrong.
There was a problem hiding this comment.
This looks good now.
There more optimizations we can do, but let's get this in first.
Can you fix the merge conflicts, and then I can merge it.
Sorry, something went wrong.
|
Further optimizations that can be done (in future PRs):
|
Sorry, something went wrong.
|
The aarch64-apple-darwin machine seems to be generally flaky lately. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Benchmark are performance neutral (in the +- 1% range) is seems.
Benchmark script"""Benchmark for BINARY_OP_EXTEND type propagation. Tests whether the tier 2 optimizer can eliminate guards when types are known from previous BINARY_OP_EXTEND results. Usage: ./python bench_binary_op_extend.py ./python bench_binary_op_extend.py --save result.json ./python bench_binary_op_extend.py --compare a.json b.json """ import sys import pyperf INNER = 2000 def bench_list_concat_subscr(n): """list + list followed by subscript — tests list type propagation.""" a = [1, 2, 3] b = [4, 5, 6] total = 0 for _ in range(n): c = a + b total += c[0] + c[3] return total def bench_tuple_concat_unpack(n): """tuple + tuple followed by unpack — tests tuple type propagation.""" t1 = (1, 2) t2 = (3, 4) total = 0 for _ in range(n): a, b, c, d = t1 + t2 total += a + d return total def bench_str_repeat(n): """str * int in a loop — tests str type propagation.""" s = "ab" total = 0 for i in range(n): r = s * (i % 5) total += len(r) return total def bench_bytes_concat(n): """bytes + bytes in a loop — tests bytes type propagation.""" a = b"hello" b_ = b" world" total = 0 for _ in range(n): c = a + b_ total += len(c) return total def bench_bytes_repeat(n): """bytes * int in a loop — tests bytes type propagation.""" b = b"ab" total = 0 for i in range(n): r = b * (i % 3) total += len(r) return total def bench_tuple_repeat(n): """tuple * int in a loop — tests tuple type propagation.""" t = (1, 2, 3) total = 0 for i in range(n): r = t * (i % 3) total += len(r) return total def bench_dict_merge(n): """dict | dict in a loop — tests dict type propagation.""" d1 = {"a": 1, "b": 2} d2 = {"c": 3, "d": 4} total = 0 for _ in range(n): d = d1 | d2 total += len(d) return total def bench_chained_list_ops(n): """Multiple list ops chained — tests guard elimination across ops.""" a = [1, 2] b = [3, 4] total = 0 for _ in range(n): c = a + b d = c + a total += d[0] + d[4] return total def bench_mixed_float_int(n): """float + int and int + float — existing EXTEND specializations.""" x = 1.5 total = 0.0 for i in range(n): a = x + i total += a return total def float_mix_mul(n): """float + int then float * float — tests unique flag for inplace mul.""" x = 1.5 total = 0.0 for i in range(n): a = (x + i) * 2.0 # result of x+i should be unique -> inplace multiply total += a return total BENCHMARKS = [ ("list_concat_subscr", bench_list_concat_subscr), ("tuple_concat_unpack", bench_tuple_concat_unpack), ("str_repeat", bench_str_repeat), ("bytes_concat", bench_bytes_concat), ("bytes_repeat", bench_bytes_repeat), ("tuple_repeat", bench_tuple_repeat), ("dict_merge", bench_dict_merge), ("chained_list_ops", bench_chained_list_ops), ("mixed_float_int", bench_mixed_float_int), ("float_mix_mul", float_mix_mul), ] def main(): args = sys.argv[1:] if "--compare" in args: idx = args.index("--compare") file_a = args[idx + 1] file_b = args[idx + 2] import subprocess subprocess.run([sys.executable, "-m", "pyperf", "compare_to", file_a, file_b, "--table"]) return save_file = None if "--save" in args: idx = args.index("--save") save_file = args[idx + 1] runner = pyperf.Runner() for name, func in BENCHMARKS: # Warm up func(INNER) runner.bench_func(name, func, INNER) if save_file and runner.args.output: import shutil shutil.copy(runner.args.output, save_file) if __name__ == "__main__": main()