| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…obTheBuidler/mypy into isinstance-tuple-of-primitives
|
Not really sure where a run test best fits. Maybe the IR test is straightforward enough? |
Sorry, something went wrong.
There was a problem hiding this comment.
What about adding a run test to run-misc.test?
Sorry, something went wrong.
| from typing import Any | ||
|
|
||
| def is_instance(x: Any) -> bool: | ||
| return isinstance(x, (str, int, bytes)) |
There was a problem hiding this comment.
Test also single-item tuple as an edge case.
Sorry, something went wrong.
|
Both of those just as run tests, or IR too? |
Sorry, something went wrong.
|
Added run tests for various cases. no additional IR tests at this time. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for adding the tests! Looks mostly good now, just one thing remains.
Sorry, something went wrong.
| is_last = i == len(descs) - 1 | ||
| next_block = fail_block if is_last else BasicBlock() | ||
| builder.add_bool_branch( | ||
| builder.primitive_op(desc, [obj], expr.line), pass_block, next_block # type: ignore [arg-type] |
There was a problem hiding this comment.
Can you avoid the # type: ignore? If there is some typing issue, I'd prefer a cast since it's more specific and tells which argument is the problematic one.
Sorry, something went wrong.
There was a problem hiding this comment.
Done. It was because descs has type list[PrimitiveDescription | None]
I'm assuming the if None in descs: return None can probably be considered as a TypeGuard of sorts by mypy but that's above my paygrade
Sorry, something went wrong.
for more information, see https://pre-commit.ci
| is_last = i == len(descs) - 1 | ||
| next_block = fail_block if is_last else BasicBlock() | ||
| builder.add_bool_branch( | ||
| builder.primitive_op(cast(PrimitiveDescription, desc), [obj], expr.line), |
There was a problem hiding this comment.
Here a slightly better way to narrow the type would be to use assert desc is not None before the call, but this is fine as well.
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks!
Sorry, something went wrong.
… memoryview (#22097) `isinstance(x, complex)` wasn't specialized, so every call looked up `builtins.complex` by name and went through `PyObject_IsInstance`. Because the tuple fast path from #19949 only applies when every member is a primitive type, `isinstance(x, (float, complex))` was worse still: it built a new tuple on every call, and `float` lost its fast path too. This is an issue I have run into in my personal projects. This adds `isinstance` primitives for `complex`, `type`, `range`, `slice` and `memoryview` (`PyComplex_Check`, `PyType_Check`, etc.), plus direct type object loads for `complex`, `slice` and `memoryview` so things like `type(x) is complex` don't go through `builtins` either. Tuples such as `(float, complex)` and `(bytes, bytearray, memoryview)` now use the existing chained checks. Also fixes a typo in the name of the `isinstance(obj, int)` primitive (`builtints.isinstance`). Benchmarks in ns, Python 3.12: | | master | this PR | | `isinstance(x, complex)` | 43.0 | 5.9 | | `isinstance(x, (float, complex))` | 106.0 | 7.1 | | `isinstance(x, (bytes, bytearray, memoryview))` | 101.8 | 5.4 | | `type(x) is complex` | 22.2 | 4.3 |
Fixes mypyc/mypyc#1227 This is just an idea for how to improve performance, for an issue that was exposed to me because of the issues in PR #22097 . `isinstance(x, (A, B))` is specialized when every class in the tuple is a primitive type (#19949) or every class is a native class (mypyc/mypyc#413). Anything else falls back to the generic path, which builds a new tuple and calls `PyObject_IsInstance` on every call, so the primitive types in the tuple lose their fast checks as well. That covers: - a primitive type mixed with a native class: `isinstance(x, (float, MyNativeClass))` - a primitive type mixed with any other class: `isinstance(x, (float, Decimal))` - nested tuples, even when every class is a primitive type: `isinstance(x, (int, (float, str)))` The chained checks added in #19949 could be generalized: flatten nested tuples, use the fast checks for primitive types and native classes, and check any other class with its own `PyObject_IsInstance` call, so no tuple is built. In a quick microbenchmark this takes `isinstance(x, (float, MyNativeClass))` from about 50 ns to about 5 ns per call. Relatedly, `isinstance(f(), ())` currently returns `False` without calling `f()`.
| Back | FazBrowse Home | New Git URL |
This PR specializes isinstance calls where the type argument is a tuple of primitive types.
We can skip tuple creation and the associated refcounting, and daisy-chain the primitive checks with an early exit option at each step.