Decide whether to lock function arguments at compile time#720
Decide whether to lock function arguments at compile time#720wjakob merged 5 commits intowjakob:free-threadedfrom
Conversation
70622d1 to
8155d6c
Compare
70a9493 to
20ae577
Compare
20ae577 to
0ad9042
Compare
c3b0558 to
0b1feea
Compare
|
This is great! It is certainly better than my runtime solution. Two thoughts:
template <bool IsLocked, bool HasValue> struct arg {
const char *name = nullptr;
PyObject *value = nullptr;
// ...
arg</* IsLocked = */ true, HasValue> lock() { return { name, value, ... }; }
template <typename T>
arg<IsLocked, /* HasValue = */ true> operator=(T&&x) {... }
};All the code processing the various |
|
After thinking more about it, perhaps having non-template |
wjakob
left a comment
There was a problem hiding this comment.
OOps, I forgot to send off these comments.
Correct, that's separate. This PR makes that improvement possible, but doesn't implement it.
If we had another axis of variation I would definitely want to go for the template. I think the current case is borderline, and don't mind changing it if you prefer the template, but I think the duplication still winds up a little easier to understand. For backcompat we would need to keep the |
82f76aa to
0bbb9dc
Compare
0bbb9dc to
babe47d
Compare
|
Thanks a lot! |
This commit refactors argument the locking locking so that it occurs at compile-time without imposing runtime overheads. The change applies to free-threaded extensions. Behavior differences compared to the prior approach: - it is no longer possible to do ``nb::arg().lock(false)`` or ``.lock(runtime_determined_value)`` - we no longer prohibit locking self in ``__init__``; changing this would also require restoring ``cast_flags::lock``, and it's not clear that the benefit outweighs the complexity.
This commit refactors argument the locking locking so that it occurs at compile-time without imposing runtime overheads. The change applies to free-threaded extensions. Behavior differences compared to the prior approach: - it is no longer possible to do ``nb::arg().lock(false)`` or ``.lock(runtime_determined_value)`` - we no longer prohibit locking self in ``__init__``; changing this would also require restoring ``cast_flags::lock``, and it's not clear that the benefit outweighs the complexity.
This commit refactors argument the locking locking so that it occurs at compile-time without imposing runtime overheads. The change applies to free-threaded extensions. Behavior differences compared to the prior approach: - it is no longer possible to do ``nb::arg().lock(false)`` or ``.lock(runtime_determined_value)`` - we no longer prohibit locking self in ``__init__``; changing this would also require restoring ``cast_flags::lock``, and it's not clear that the benefit outweighs the complexity.
This commit refactors argument the locking locking so that it occurs at compile-time without imposing runtime overheads. The change applies to free-threaded extensions. Behavior differences compared to the prior approach: - it is no longer possible to do ``nb::arg().lock(false)`` or ``.lock(runtime_determined_value)`` - we no longer prohibit locking self in ``__init__``; changing this would also require restoring ``cast_flags::lock``, and it's not clear that the benefit outweighs the complexity.
This commit refactors argument the locking locking so that it occurs at compile-time without imposing runtime overheads. The change applies to free-threaded extensions. Behavior differences compared to the prior approach: - it is no longer possible to do ``nb::arg().lock(false)`` or ``.lock(runtime_determined_value)`` - we no longer prohibit locking self in ``__init__``; changing this would also require restoring ``cast_flags::lock``, and it's not clear that the benefit outweighs the complexity.
An attempt at the static locking determination that I suggested in #695 (comment). Note this PR is against the free-threaded branch.
Behavior differences from the free-threaded branch without this change:
nb::arg().lock(false)or.lock(runtime_determined_value); this could be re-added by restoringcast_flags::lockand checking thearg_flagsat runtime, but I didn't think it was worth the complexity__init__; changing this would also require restoringcast_flags::lock, and it's not clear what benefit it would have (sure the lock is somewhat superfluous but do we really care?)