Skip to content

perf: reduce per-call overhead in gin_helper hot paths#51596

Merged
MarshallOfSound merged 1 commit into
mainfrom
perf/gin-helper-hot-paths
May 13, 2026
Merged

perf: reduce per-call overhead in gin_helper hot paths#51596
MarshallOfSound merged 1 commit into
mainfrom
perf/gin-helper-hot-paths

Conversation

@MarshallOfSound

@MarshallOfSound MarshallOfSound commented May 13, 2026

Copy link
Copy Markdown
Member

Description of Change

Removes a handful of avoidable per-call costs from gin_helper paths that run on every native event emit, IPC message, option-dictionary parse, and C++ → JS callback.

  • EmitEvent / CallMethodWithArgs — internalize the event name and "emit" (gin::StringToSymbol); reuse the emitter as the node::CallbackScope resource instead of allocating a throwaway v8::Object; drop the redundant inner EscapableHandleScope.
  • gin_helper::Dictionary — internalize property keys; replace Has() + Get() in Get() with a single GetRealNamedProperty() (one V8 boundary crossing instead of two), guarded by a zero-cost HasNamedLookupInterceptor() flag check that routes native-interceptor objects (process.env, etc.) to the original path.
  • gin_helper::Locker — store v8::Locker inline in std::optional instead of heap-allocating per construction.
  • Converter<char16_t>::FromV8 — read the UTF-16 code unit directly instead of round-tripping through UTF-8.

Benchmarks

Linux x86-64 testing build, microbenchmark medians:

hot path before after Δ
EmitEvent (1 listener) 2806 ns 1816 ns −35%
EmitEvent (no listeners) 2788 ns 1800 ns −35%
Dictionary::Get (per key, all hit) 82.2 ns 26.9 ns −67%
Dictionary::Get (per key, all miss) 55.6 ns 18.5 ns −67%
Dictionary::Set (per key) 121 ns 82.6 ns −32%
gin_helper::Locker ctor/dtor 94.3 ns 9.85 ns −90%
C++ → JS callback (base::RepeatingClosure) 463 ns 301 ns −35%

Checklist

Release Notes

Notes: Improved performance of native event emission, IPC dispatch, and option-dictionary parsing.

@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label May 13, 2026
@MarshallOfSound MarshallOfSound requested a review from ckerr May 13, 2026 06:52
@MarshallOfSound MarshallOfSound enabled auto-merge (squash) May 13, 2026 18:32

@ckerr ckerr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • The blink converter change is a nice shortcut.

  • Locker is a good change. I know I've done that same change in another piece of code in the past, kinda surprised I'd left this one on the table. Good catch.

  • Dictionary::Get() is >2x bigger but worth it IMO. Get(std::string_view) is the most common call by far.

  • I haven't crunched the numbers but I'll take your word for it about the value in symbolizing dictionary keys? My intuition is that we probably use the same 100ish keys over and over again.

  • One minor concern: it would be easy for a future contributor to miss the code comment in the .cc file and accidentally add a CallMethodWithArgs() caller w/o an EscapableHandleScope. The compiler would allow it and CI might even be green. Let's another code comment before the declaration at shell/common/gin_helper/event_emitter_caller.h:21.

Several gin_helper code paths run on every native event emit, every IPC
message, every option-dictionary parse, and every C++ -> JS callback.
This change removes a handful of avoidable per-call costs from those
paths.

EmitEvent / CallMethodWithArgs:
- Internalize the event name and the "emit" method name with
  gin::StringToSymbol so V8 dedupes them against its string table and
  property lookups can compare by pointer identity instead of hashing a
  fresh kNormal string on every emit.
- Reuse the emitter object as the node::CallbackScope async resource
  instead of allocating a throwaway v8::Object per emit.
- Drop the redundant inner EscapableHandleScope; EmitEvent / CustomEmit
  already open one immediately around the call.

gin_helper::Dictionary:
- Internalize property keys via gin::StringToSymbol (matching what
  gin::DataObjectBuilder and gin::ObjectTemplateBuilder already do).
- Replace the unconditional Has() + Get() pair in Get() with a single
  GetRealNamedProperty() lookup — one V8 boundary crossing per key
  instead of two, with the same missing-vs-undefined disambiguation.
  Native interceptor objects (process.env, etc.) are detected with the
  zero-cost HasNamedLookupInterceptor() hidden-class flag and routed to
  the Has()/Get() path that drives the interceptor.

gin_helper::Locker:
- Store v8::Locker inline in std::optional instead of heap-allocating it
  per construction; this is on the path of every C++ -> JS callback.

blink converters:
- char16_t::FromV8: read the UTF-16 code unit directly instead of
  round-tripping V8's internal UTF-16 through UTF-8 and back.

Microbenchmark medians (Linux x86-64 testing build):

  EmitEvent (1 listener)              2806 ns -> 1816 ns  (-35%)
  EmitEvent (no listeners)            2788 ns -> 1800 ns  (-35%)
  Dictionary::Get (per key, all hit)  82.2 ns -> 26.9 ns  (-67%)
  Dictionary::Get (per key, all miss) 55.6 ns -> 18.5 ns  (-67%)
  Dictionary::Set (per key)           121  ns -> 82.6 ns  (-32%)
  gin_helper::Locker ctor/dtor        94.3 ns -> 9.85 ns  (-90%)
  C++ -> JS callback (RepeatingCb)    463  ns -> 301  ns  (-35%)
@MarshallOfSound MarshallOfSound force-pushed the perf/gin-helper-hot-paths branch from 1882fb0 to 80a52aa Compare May 13, 2026 19:42
@ckerr

ckerr commented May 13, 2026

Copy link
Copy Markdown
Member

needs backport labels

@MarshallOfSound MarshallOfSound added target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels May 13, 2026
@MarshallOfSound MarshallOfSound merged commit fe439c7 into main May 13, 2026
81 checks passed
@MarshallOfSound MarshallOfSound deleted the perf/gin-helper-hot-paths branch May 13, 2026 21:48
@release-clerk

release-clerk Bot commented May 13, 2026

Copy link
Copy Markdown

Release Notes Persisted

Improved performance of native event emission, IPC dispatch, and option-dictionary parsing.

@trop

trop Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "41-x-y", please check out #51613

@trop trop Bot added in-flight/41-x-y and removed target/41-x-y PR should also be added to the "41-x-y" branch. labels May 13, 2026
@trop

trop Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "42-x-y", please check out #51614

@trop

trop Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "43-x-y", please check out #51615

@trop trop Bot added in-flight/42-x-y in-flight/43-x-y and removed target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels May 13, 2026
@trop trop Bot added merged/42-x-y PR was merged to the "42-x-y" branch. and removed in-flight/42-x-y labels May 13, 2026
@trop trop Bot added merged/41-x-y PR was merged to the "41-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. and removed in-flight/41-x-y in-flight/43-x-y labels May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged/41-x-y PR was merged to the "41-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. new-pr 🌱 PR opened recently semver/none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants