Skip to content

fix: move model construction off the JS thread#944

Open
yocontra wants to merge 4 commits intosoftware-mansion:mainfrom
yocontra:fix/async-model-loading
Open

fix: move model construction off the JS thread#944
yocontra wants to merge 4 commits intosoftware-mansion:mainfrom
yocontra:fix/async-model-loading

Conversation

@yocontra
Copy link
Contributor

@yocontra yocontra commented Mar 7, 2026

Problem

The loadModel template in RnExecutorchInstaller.h constructs model objects synchronously on the JS thread. For models like Kokoro TTS, the constructor loads .pte files, initializes the phonemizer, and reads voice data — blocking the JS thread for several seconds.

This prevents React from rendering any pending state updates (loading spinners, progress indicators, etc.) until construction completes. For example, calling setIsSynthesizing(true) followed by tts.load() will not show the loading UI for 3-4 seconds because the JS thread is blocked by the synchronous native constructor.

Solution

Make loadModel return a Promise and dispatch the heavy model construction to GlobalThreadPool::detach — matching the pattern already used by promiseHostFunction for inference calls like generate().

C++ (RnExecutorchInstaller.h)

  • JSI arguments are still parsed on the JS thread (required for jsi::Value access)
  • Model construction is dispatched to GlobalThreadPool::detach
  • The Promise resolves on the JS thread via callInvoker->invokeAsync with the host object
  • Error handling follows the same pattern as promiseHostFunction in ModelHostObject.h

TypeScript

  • All global.load*() call sites updated to await the now-async result
  • Global type declarations updated to return Promise<any>
  • SpeechToText already used await, so no change needed there

Breaking Change

This is a breaking change for any consumers calling global.load*() directly and expecting a synchronous return value. However:

  • The public module APIs (load() methods) are already async — no user-facing API changes needed
  • The global.load*() functions are internal/undocumented

Test Plan

  • Verify model loading still works for all model types (TTS, LLM, OCR, etc.)
  • Verify that React state updates (loading spinners) render immediately when load() is called, rather than being delayed until construction completes
  • Verify error handling still propagates correctly from native construction failures

@yocontra yocontra marked this pull request as draft March 7, 2026 15:52
@yocontra yocontra force-pushed the fix/async-model-loading branch from 1877fe7 to f293cb2 Compare March 7, 2026 16:19
@yocontra yocontra marked this pull request as ready for review March 7, 2026 16:22
@msluszniak
Copy link
Member

There is an error in CI build, specifically in VerticalOCR, I guess we just need to mark it as promise like for the other models.

@yocontra
Copy link
Contributor Author

yocontra commented Mar 8, 2026

@msluszniak Fixed! I can't see the CI but pretty sure I made the correct changes.

@msluszniak
Copy link
Member

Cool now all important checks work. Now, we just need to fix lint, I will fix this for you later today and then proceed with the actual review ;)

@yocontra
Copy link
Contributor Author

yocontra commented Mar 8, 2026

@msluszniak Fixed the lint errors - my bad

@msluszniak msluszniak added the bug fix PRs that are fixing bugs label Mar 9, 2026
@msluszniak msluszniak self-requested a review March 9, 2026 10:35
yocontra and others added 4 commits March 9, 2026 15:04
The `loadModel` template in `RnExecutorchInstaller.h` constructs model
objects synchronously on the JS thread. For models like Kokoro TTS, the
constructor loads .pte files, initializes the phonemizer, and reads
voice data — blocking the JS thread for several seconds. This prevents
React from rendering loading states (spinners, progress indicators)
until construction completes.

This change makes `loadModel` return a Promise and dispatches the model
construction to `GlobalThreadPool::detach`, matching the pattern already
used by `promiseHostFunction` for inference calls like `generate()`.

On the JS side, all `global.load*()` call sites are updated to `await`
the now-async result, and the global type declarations are updated to
return `Promise<any>`.

This is a breaking change for any consumers calling `global.load*()`
directly and expecting a synchronous return value. The public module
APIs (`load()` methods) are already async, so no user-facing API changes
are needed.
The `await` keyword was used in loadNativeModule without the method
being marked async, causing a Babel parse error (UnexpectedReservedWord).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@msluszniak
Copy link
Member

FYI @yocontra, I will push changes to the API reference so CI will pass. Other then that, I tested changes and everything was correct. :))

@msluszniak msluszniak force-pushed the fix/async-model-loading branch from ff67697 to 32620fd Compare March 9, 2026 14:10
Copy link
Member

@msluszniak msluszniak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@chmjkb
Copy link
Collaborator

chmjkb commented Mar 9, 2026

Thanks for another contribution @yocontra 🫶, I will perform some quality checks tomorrow and then I think we're ready to merge :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix PRs that are fixing bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants