perf(nif): cache reply and dtype atoms at load - #136
Merged
Conversation
Atoms on the async-reply path (`:ok` on every op, plus
`:error`/`:stopped`/`:argument`/`:runtime`/`:unknown`) and the Nx dtype
"kind" atoms (`:f`/`:bf`/`:s`/`:u`/`:c`/`:pred`) were built at point of
use with enif_make_atom / fine::Atom("..."), redoing the atom-table
lookup on each call.
Define them once as static fine::Atom objects (emily/atoms.hpp). fine
registers namespace-scope atoms and creates their terms in the NIF load
callback, and Encoder<Atom> then returns the cached term directly. The
reply builders and from_mlx_dtype now reference these cached atoms.
Correctness-neutral (verified by the dtype and async tests); removes the
per-reply atom-table lookup from the hot path.
Closes #135
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #135. Defines the reply atoms (
:ok/:error/:stopped/:argument/:runtime/:unknown) and dtype-kind atoms (:f/:bf/:s/:u/:c/:pred) as staticfine::Atoms in a newc_src/emily/atoms.hpp. FINE registers namespace-scope atoms and builds their terms once in the load callback (init_atoms), andEncoder<Atom>returns the cached term — so the reply builders (async.hpp,emily_nif.cpp) andfrom_mlx_dtypeno longer do a per-call atom-table lookup. Covers all atom creation in Emily's own NIF code;fine::Ok<>etc. already use FINE's internal cached atoms.Correctness-neutral — the dtype assertions (
Native.dtype/1→{:f, 32}…) and async:ok/:error/:stoppedreply tests pass unchanged; the NIF loading cleanly confirms the static-atom registration has no init-order problem.mix precommitgreen (543 tests). Micro-optimization (the lookup is ~tens of ns, noise next to a GPU op), done for the idiomatic cached-atom pattern.