From 0b6099fb49e8f1afe810f000f31032b2cac3db85 Mon Sep 17 00:00:00 2001 From: Tom Duncalf Date: Mon, 5 Sep 2022 11:22:04 +0100 Subject: [PATCH 1/5] Statically cache the String value of g_internal_field for a small performance gain --- CHANGELOG.md | 1 + src/jsi/jsi_class.hpp | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2097509082..88dbd2fded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## vNext (TBD) ### Enhancements +* Small improvement to performance by caching JSI property String object [#4863](https://github.com/realm/realm-js/pull/4863) * None. ### Fixed diff --git a/src/jsi/jsi_class.hpp b/src/jsi/jsi_class.hpp index ccd5f22787..5bf6f46411 100644 --- a/src/jsi/jsi_class.hpp +++ b/src/jsi/jsi_class.hpp @@ -240,6 +240,10 @@ class ObjectWrap { // Also, may need to suppress destruction. inline static std::optional s_ctor; + // Cache the JSI String instance representing the field name of the internal + // C++ object for the lifetime of the current env, as this is a hot path + inline static std::optional s_js_internal_field_name; + /** * @brief callback for invalid access to index setters * Throws an error when a users attemps to write to an index on a type that @@ -310,9 +314,10 @@ class ObjectWrap { .asFunction(env)); js::Context::register_invalidator([] { - // Ensure the static constructor is destructed when the runtime goes away. - // This is to avoid the reassignment of s_ctor throwing because the runtime has disappeared. + // Ensure the static constructor and JSI String reference are destructed when the runtime goes away. + // This is to avoid the reassignment throwing because the runtime has disappeared. s_ctor.reset(); + s_js_internal_field_name.reset(); }); for (auto&& [name, prop] : s_type.static_properties) { @@ -490,7 +495,11 @@ class ObjectWrap { static Internal* get_internal(JsiEnv env, const JsiObj& object) { - auto internal = object->getProperty(env, g_internal_field); + if (!s_js_internal_field_name) { + s_js_internal_field_name = fbjsi::String::createFromAscii(env, g_internal_field); + } + + auto internal = object->getProperty(env, *s_js_internal_field_name); if (internal.isUndefined()) { // In the case of a user opening a Realm with a class-based model, // the user defined constructor will get called before the "internal" property has been set. @@ -663,7 +672,8 @@ class ObjectWrap { } // namespace realmjsi template -class ObjectWrap : public realm::js::realmjsi::ObjectWrap {}; +class ObjectWrap : public realm::js::realmjsi::ObjectWrap { +}; template fbjsi::Value wrap(fbjsi::Runtime& rt, const fbjsi::Value& thisVal, const fbjsi::Value* args, size_t count) From 3f9548399fbd689a24562d72e4b3cbdd2584ad4a Mon Sep 17 00:00:00 2001 From: Tom Duncalf Date: Fri, 9 Sep 2022 10:01:03 +0100 Subject: [PATCH 2/5] Fix lint --- src/jsi/jsi_class.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/jsi/jsi_class.hpp b/src/jsi/jsi_class.hpp index 5bf6f46411..e6ee771ae9 100644 --- a/src/jsi/jsi_class.hpp +++ b/src/jsi/jsi_class.hpp @@ -672,8 +672,7 @@ class ObjectWrap { } // namespace realmjsi template -class ObjectWrap : public realm::js::realmjsi::ObjectWrap { -}; +class ObjectWrap : public realm::js::realmjsi::ObjectWrap {}; template fbjsi::Value wrap(fbjsi::Runtime& rt, const fbjsi::Value& thisVal, const fbjsi::Value* args, size_t count) From 2d9ed2ed5e75c140863bd5df754345b4e3df0ba2 Mon Sep 17 00:00:00 2001 From: Tom Duncalf Date: Fri, 9 Sep 2022 11:13:27 +0100 Subject: [PATCH 3/5] Update src/jsi/jsi_class.hpp Co-authored-by: Mathias Stearn --- src/jsi/jsi_class.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jsi/jsi_class.hpp b/src/jsi/jsi_class.hpp index e6ee771ae9..a8bf18bef3 100644 --- a/src/jsi/jsi_class.hpp +++ b/src/jsi/jsi_class.hpp @@ -315,7 +315,7 @@ class ObjectWrap { js::Context::register_invalidator([] { // Ensure the static constructor and JSI String reference are destructed when the runtime goes away. - // This is to avoid the reassignment throwing because the runtime has disappeared. + // This is to avoid reassignment and destruction throwing because the runtime has disappeared. s_ctor.reset(); s_js_internal_field_name.reset(); }); From c256c31b026b64bc60e02bef05482e85b5d8cd7c Mon Sep 17 00:00:00 2001 From: Tom Duncalf Date: Fri, 9 Sep 2022 11:13:33 +0100 Subject: [PATCH 4/5] Update src/jsi/jsi_class.hpp Co-authored-by: Mathias Stearn --- src/jsi/jsi_class.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jsi/jsi_class.hpp b/src/jsi/jsi_class.hpp index a8bf18bef3..aaace099fa 100644 --- a/src/jsi/jsi_class.hpp +++ b/src/jsi/jsi_class.hpp @@ -495,7 +495,7 @@ class ObjectWrap { static Internal* get_internal(JsiEnv env, const JsiObj& object) { - if (!s_js_internal_field_name) { + if (REALM_UNLIKELY(!s_js_internal_field_name)) { s_js_internal_field_name = fbjsi::String::createFromAscii(env, g_internal_field); } From 1ed1d1ef4b26dbeacb2386f754b30d8e4dcb30af Mon Sep 17 00:00:00 2001 From: Tom Duncalf Date: Fri, 9 Sep 2022 12:42:07 +0100 Subject: [PATCH 5/5] Change to empty string for internal field name --- src/jsi/jsi_class.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/jsi/jsi_class.hpp b/src/jsi/jsi_class.hpp index aaace099fa..253234a717 100644 --- a/src/jsi/jsi_class.hpp +++ b/src/jsi/jsi_class.hpp @@ -145,7 +145,10 @@ inline void copyProperty(JsiEnv env, const fbjsi::Object& from, const fbjsi::Obj defineProperty(env, to, name, *prop); } -inline constexpr const char g_internal_field[] = "__Realm_internal"; +// The name used for the property on the JS object which stores the reference to the corresponding C++ object. +// We use an empty string as testing showed it was 1% faster with JSC and 4% faster with Hermes than using +// an actual string, and also has the benefit that it is not a valid Realm object key name. +inline constexpr const char g_internal_field[] = ""; template using ClassDefinition = js::ClassDefinition;