Skip to content

perf(napi): PersistedPerInstanceHashMap can be thread local - #2681

Merged
Brooooooklyn merged 4 commits into
napi-rs:mainfrom
SyMind:perf-PersistedPerInstanceHashMap
Jun 4, 2025
Merged

perf(napi): PersistedPerInstanceHashMap can be thread local#2681
Brooooooklyn merged 4 commits into
napi-rs:mainfrom
SyMind:perf-PersistedPerInstanceHashMap

Conversation

@SyMind

@SyMind SyMind commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

Performance analysis revealed unexpected overhead in napi::bindgen_runtime::module_register::PersistedPerInstanceHashMap<K,V>::borrow_mut.

Optimizations

  1. Lock mechanism refinement for PersistedPerInstanceHashMap
    Replaced internal RwLock with RefCell since this struct doesn't require Send + Sync implementation, eliminating thread synchronization overhead.

  2. Thread-local initialization
    Initialized the following global variables via thread_local! macro for thread safety:

    • REGISTERED_CLASSES
    • FN_REGISTER_MAP
    • THREADS_CAN_ACCESS_ENV
  3. Cross-thread sharing enhancement
    Refactored ModuleClassProperty into a struct guarded by RwLock to enable safe sharing between JS main thread and worker threads via MODULE_CLASS_PROPERTIES.

image

@SyMind
SyMind force-pushed the perf-PersistedPerInstanceHashMap branch from b5ba986 to 36d37b7 Compare June 4, 2025 00:36
@SyMind
SyMind force-pushed the perf-PersistedPerInstanceHashMap branch from 10a0aef to 74826e3 Compare June 4, 2025 01:26
@SyMind
SyMind marked this pull request as ready for review June 4, 2025 01:42
@SyMind
SyMind force-pushed the perf-PersistedPerInstanceHashMap branch from 42ce520 to 6de0cd5 Compare June 4, 2025 01:56
@Brooooooklyn Brooooooklyn changed the title perf: PersistedPerInstanceHashMap can be thread local perf(napi): PersistedPerInstanceHashMap can be thread local Jun 4, 2025
@Brooooooklyn

Copy link
Copy Markdown
Member

Notes to look back on:

I was trying to avoid thread_local! usage because of I wasn't figure out why https://github.com/napi-rs/napi-rs/blob/napi%403.0.0-beta.4/.cargo/config.toml#L4

We can use thread_local! safely now

@Brooooooklyn
Brooooooklyn merged commit a6f133c into napi-rs:main Jun 4, 2025
@SyMind
SyMind deleted the perf-PersistedPerInstanceHashMap branch June 4, 2025 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants