From c36cc995a9440ba86db2ab3dd4d4fda90b748b45 Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Wed, 12 Aug 2026 13:00:48 -0400 Subject: [PATCH] [build] JSG: store struct field wrappers in a flat array Now that all of a struct's field wrappers hold nothing but an interned name, they no longer need to be distinct types. Move the field type from FieldWrapper's template arguments onto wrap()/unwrap() and keep the wrappers in a kj::Array instead of a kj::Tuple. Using an array allows us to avoid templating based on the list of nested ExpandAndApplyFunc types seen so far. Field access stays direct: the `fields` pack still holds pointers to members, so `in.*fields` is a constant member offset, and the field type still comes from the pack via FieldType, so no dispatch becomes indirect. --- src/workerd/jsg/struct.h | 55 ++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/src/workerd/jsg/struct.h b/src/workerd/jsg/struct.h index f02950623ed..271e0558bb1 100644 --- a/src/workerd/jsg/struct.h +++ b/src/workerd/jsg/struct.h @@ -80,14 +80,13 @@ static_assert(!NotV8Local>); static_assert(!NotV8Local>>); static_assert(!NotV8Local>); -// Converts one field of a JSG_STRUCT to/from the corresponding JavaScript property. Note that the -// struct that declares the field, the field's location within that struct, and the field's JS- -// visible name are supplied by StructWrapper at the call site to avoid excessive templating. -template +// Converts one field of a JSG_STRUCT to/from the corresponding JavaScript property. Note that a +// field wrapper's state is just its interned JS-visible name – the struct that declares the field, +// the field's location within that struct and the field's type are supplied by StructWrapper at the +// call site. This avoids excessive templating, only wrap() and unwrap() need to be emitted once per +// field type. class FieldWrapper { public: - using Type = T; - FieldWrapper(v8::Isolate* isolate, kj::StringPtr exportedName) : exportedName(exportedName), nameHandle(isolate, v8StrIntern(isolate, exportedName)) {} @@ -95,6 +94,7 @@ class FieldWrapper { // The is the original, slow-path wrap implementation that uses Set(). Prefer the other overload // for better performance. It is, however, a breaking change to remove this overload so we // need to keep it with a compatibility flag. + template void wrap(Lock& js, TypeWrapper& wrapper, v8::Isolate* isolate, @@ -107,7 +107,7 @@ class FieldWrapper { } else if constexpr (kj::isSameType() || kj::isSameType()) { // Fields with these types are required NOT to be present, so don't try to convert them. } else { - if constexpr (webidl::OptionalType) { + if constexpr (webidl::OptionalType) { // Don't even set optional fields that aren't present. if (value == kj::none) return; } @@ -116,6 +116,7 @@ class FieldWrapper { } } + template void wrap(Lock& js, TypeWrapper& wrapper, v8::Isolate* isolate, @@ -135,12 +136,13 @@ class FieldWrapper { } // `structType` identifies the struct declaring this field and is used for type error messages. - Type unwrap(TypeWrapper& wrapper, + template + T unwrap(TypeWrapper& wrapper, v8::Isolate* isolate, v8::Local context, v8::Local in, const std::type_info& structType) { - static_assert(NotV8Local); + static_assert(NotV8Local); auto& js = Lock::from(isolate); auto fieldName = nameHandle.Get(isolate); v8::Local jsValue = v8::Undefined(isolate); @@ -158,7 +160,7 @@ class FieldWrapper { jsValue = check(in->GetRealNamedProperty(context, fieldName)); } } - return wrapper.template unwrap( + return wrapper.template unwrap( js, context, jsValue, TypeErrorContext::structField(structType, exportedName.cStr()), in); } @@ -206,16 +208,15 @@ class StructWrapper, kj::_::Indexes v8::Local wrap( Lock& js, v8::Local context, kj::Maybe> creator, T&& in) { auto isolate = js.v8Isolate; - auto& fieldWrappers = getFields(isolate); + auto fieldWrappers = getFields(isolate); // Fast path using a cached dictionary template. if (js.isUsingFastJsgStruct()) { v8::MaybeLocal values[kCountOfUsableFields]{}; size_t idx = 0; - (kj::get(fieldWrappers) - .wrap(js, static_cast(*this), isolate, context, creator, in.*fields, - values[idx], idx), + (fieldWrappers[indices].wrap( + js, static_cast(*this), isolate, context, creator, in.*fields, values[idx], idx), ...); // We use a cached dictionary template to improve performance on repeated struct wraps. @@ -236,8 +237,8 @@ class StructWrapper, kj::_::Indexes // Original slow path. v8::Local out = v8::Object::New(isolate); - (kj::get(fieldWrappers) - .wrap(js, static_cast(*this), isolate, context, creator, in.*fields, out), + (fieldWrappers[indices].wrap( + js, static_cast(*this), isolate, context, creator, in.*fields, out), ...); return out; } @@ -274,7 +275,7 @@ class StructWrapper, kj::_::Indexes if (!handle->IsObject()) return kj::none; - auto& fieldWrappers = getFields(js.v8Isolate); + auto fieldWrappers = getFields(js.v8Isolate); auto in = handle.As(); // Note: We unwrap struct members in the order in which the compiler evaluates the expressions @@ -282,8 +283,8 @@ class StructWrapper, kj::_::Indexes // it prescribes lexicographically-ordered member initialization, with base members ordered // before derived members. Objects with mutating getters might be broken by this, but it // doesn't seem worth fixing absent a compelling use case. - auto t = T{kj::get(fieldWrappers) - .unwrap(static_cast(*this), js.v8Isolate, context, in, typeid(T))...}; + auto t = T{fieldWrappers[indices].template unwrap>( + static_cast(*this), js.v8Isolate, context, in, typeid(T))...}; // Note that if a `validate` function is provided, then it will be called after the struct is // unwrapped from v8. This would be an appropriate time to throw an error. @@ -299,17 +300,21 @@ class StructWrapper, kj::_::Indexes void getTemplate() = delete; private: - using FieldWrappers = kj::Tuple>...>; - v8::Global templateHandle; - kj::Maybe lazyFields; - FieldWrappers& getFields(v8::Isolate* isolate) { + // One wrapper per field, in declaration order, so that fieldWrappers lines up with the fields + // pack and with T::jsgFieldNames. + kj::Maybe> lazyFields; + + kj::ArrayPtr getFields(v8::Isolate* isolate) { KJ_IF_SOME(f, lazyFields) { return f; } else { - return lazyFields.emplace( - kj::tuple(FieldWrapper>(isolate, T::jsgFieldNames[indices])...)); + auto builder = kj::heapArrayBuilder(kj::size(T::jsgFieldNames)); + for (auto name: T::jsgFieldNames) { + builder.add(isolate, name); + } + return lazyFields.emplace(builder.finish()); } }