lucifer1004 commented on PR #824:
URL: https://github.com/apache/tvm-ffi/pull/824#issuecomment-5879449378
Thanks, both points hold. Addressed in 8ef9c78:
**Load order.** Registration is now opt-in: a type defined in Rust declares
`#[type_register]`. A type without it binds a key its defining library
registers and, as before this PR, looks it up with `TVMFFITypeKeyToIndex` and
panics if that library is not loaded yet ("load the library that defines it
first, or add #[type_register] if this crate defines it"). A Rust binding of a
C++ object therefore never registers the key with the wrong child slots,
whatever the load order; a wrong order fails loudly instead of misclassifying
types later. `#[type_child_slots]` and `#[type_child_slots_can_overflow]`
require `#[type_register]`.
**Concurrent registration.** The type table assumes updates happen while a
library loads or are locked by the caller (`src/ffi/object.cc`). C++ meets that
with `static inline const _type_index`, initialized at load; Rust registers on
first use, possibly on any thread. Registrations from Rust now hold one
process-wide lock, `tvm_ffi::object::TYPE_REGISTRATION`, so they cannot race
each other (the parent is registered before the lock is taken, so there is no
re-entrant locking). What remains is the caller's duty the table already
documents: not registering while another thread loads a library that registers
types. The derive's docs say so, and that calling `T::type_index()` before
starting such threads registers a type eagerly. A new test registers eight
types from eight threads at once and checks their indices are distinct and
match their keys.
If you would rather close the remaining gap for C++ and Rust alike, a mutex
inside `TypeTable::GetOrAllocTypeIndex` would do it; it is a cold path. I kept
that out of this PR since it changes the C++ runtime, but I'm happy to open it
separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
