1847761 - pre-calculate length and hash for names in xptdata

Status: ASSIGNED

(Core :: XPConnect, task, P3)

Description

[Tooru Fujisawa [:arai]]

XPCNativeInterface::NewInstance calls JS_AtomizeString with const char*, but the content is known at compile-time and we can pre-calculate length and hash, either in codegen or constexpr.

https://searchfox.org/mozilla-central/rev/0058dbdd1c2eb9164e195082d18f3a657f291824/xpcom/reflect/xptinfo/xptinfo.cpp#99

JSString* str = JS_AtomizeString(aCx, Name());

https://searchfox.org/mozilla-central/rev/0058dbdd1c2eb9164e195082d18f3a657f291824/js/xpconnect/src/XPCWrappedNativeInfo.cpp#285

RootedString str(cx, JS_AtomizeString(cx, namestr.get()));

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Can we use the Gecko static atoms?

Comment 2

[Nika Layzell [:nika] (ni? for response)]

We also atomize strings every time we create things for other more-performance-sensitive parts of the codebase, like in WebIDL (e.g. through interfaces like JS_DefineFunctions and JS_DefineAtoms: https://searchfox.org/mozilla-central/rev/778df10b835ea87def2bfdbb83810ee36bc22c94/dom/bindings/BindingUtils.cpp#718,722), because we don't have a way to do something better. If the JS engine exposes a system to allow us to provide static atoms to be used directly in the JS engine, we should probably focus on using it within WebIDL before we do optimizations in XPConnect, which is less performance-sensitive.

Comment 3

[Tooru Fujisawa [:arai]]

for JSPropertySpec and JSFunctionSpec, there are bug 1847672 and bug 1847677.
bug 1847672 is to add length and hash to them to skip some steps during atomization,
and bug 1847677 is to add fast-path for well-known atoms.

bug 1847677 way can be extended if we merge the gecko static atoms into SpiderMonkey's well-known atoms,
so that the conversion becomes a simple pointer operation.

I'll look into exposing the well-known atoms mechanism, so that it can directly be used and extended by Gecko.

Comment 4

[Tooru Fujisawa [:arai]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)

Can we use the Gecko static atoms?

Filed bug 1848278 to extend the well-known atom with gecko static atoms.