Created attachment 442791 [details] Field Initialization Benchmark I benchmarked ES2022 class field initialization time. https://gist.github.com/robpalme/a9ff3e4764b764d7dc24743816d8a624#gistcomment-3944128 In Chrome and Firefox, public field initialization has been optimized to have similar performance to private fields. Performance is also similar to assigning to `this` inside the constructor. In latest Safari Tech Preview 134, performance of public field initialization is 12x worse than both private fields and constructor assignment. - Classic Constructor Assignments: 38ms - Public Fields: 462ms - Private Fields: 33ms
Currently, public fields are implemented in LLint as op_define_data_property and op_define_accessor_property, both of which seem to be implemented only as slow paths with no caching, and as C++ calls with no caching in each JIT. It may be worth adding some caching and inlining for these operations.
<rdar://problem/85050031>
*** Bug 198330 has been marked as a duplicate of this bug. ***
Yeah, looks like public-class-field implementation lacks this optimization.
Created attachment 445071 [details] Patch
Comment on attachment 445071 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445071&action=review > Source/JavaScriptCore/runtime/JSObject.h:1590 > + return putDirectInternal<PutModeDefineOwnPropertyIgnoringExtensibility>(vm, propertyName, value, attributes, slot); Under which conditions do we end up with the "IgnoringExtensibility" flag? Is this a type speculation to avoid unnecessary checks in DFG? Under normal circumstances, defining public class fields should be concerned with extensibility, particularly for derived classes
Comment on attachment 445071 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445071&action=review >> Source/JavaScriptCore/runtime/JSObject.h:1590 >> + return putDirectInternal<PutModeDefineOwnPropertyIgnoringExtensibility>(vm, propertyName, value, attributes, slot); > > Under which conditions do we end up with the "IgnoringExtensibility" flag? Is this a type speculation to avoid unnecessary checks in DFG? > > Under normal circumstances, defining public class fields should be concerned with extensibility, particularly for derived classes For now, I would like to keep the current putDirect semantics since this patch's intention is not changing that. Probably, I'll look into it to remove that hack for putDirect. But for now, I keep that semantics and adding the new cleaner one.
Created attachment 445119 [details] Patch
Created attachment 445120 [details] Patch
Created attachment 445121 [details] Patch
Created attachment 445163 [details] Patch
Comment on attachment 445163 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445163&action=review r=me with a tiny nit on FIXME. So nice to see generator.emitCallDefineProperty() gone! I've evaluated pretty much the same approach to replace them in ClassExprNode::emitBytecode() but that didn't result in microbenchmark progression. Also, really cool to have JSObject::putDirectInternal() return an error message, improving developer experience with JSC. > Source/JavaScriptCore/runtime/CommonSlowPaths.h:202 > + if (LIKELY(propertyName != vm.propertyNames->underscoreProto && !structure->hasReadOnlyOrGetterSetterPropertiesExcludingProto() && (isJSFunction || structure->classInfo()->methodTable.defineOwnProperty == &JSObject::defineOwnProperty))) { This handle non-reified accessors, great! > Source/JavaScriptCore/runtime/JSObjectInlines.h:285 > // FIXME: For a failure due to non-extensible structure, the error message is misleading We can remove this FIXME now.
Comment on attachment 445163 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445163&action=review Thank you! >> Source/JavaScriptCore/runtime/CommonSlowPaths.h:202 >> + if (LIKELY(propertyName != vm.propertyNames->underscoreProto && !structure->hasReadOnlyOrGetterSetterPropertiesExcludingProto() && (isJSFunction || structure->classInfo()->methodTable.defineOwnProperty == &JSObject::defineOwnProperty))) { > > This handle non-reified accessors, great! :D >> Source/JavaScriptCore/runtime/JSObjectInlines.h:285 >> // FIXME: For a failure due to non-extensible structure, the error message is misleading > > We can remove this FIXME now. NICE.
Committed r286251 (244614@main): <https://commits.webkit.org/244614@main>