Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibJS: Make function instantiation a bit more spec compliant #1555

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mobounya
Copy link
Contributor

@mobounya mobounya commented Sep 27, 2024

We used to call ECMAScriptFunctionObject::create() to instantiate different types of JS functions with a lot of "prep" hidden in ECMAScriptFunctionObject::initialize(). This replaces that with instantiate_function_object() which matches the specs and organizes function initialization in one place.

To be safe I choose to skip the code in ECMAScriptFunctionObject::initialize() if we're instantiation from a instantiate_function_object() otherwise that code will be executed since ECMAScriptFunctionObject::create() is called from a bunch of other places that may need it. I think ideally that code should be removed and JS function initialization should be done from functions that match the specs.

test262 diff:

None

@awesomekling
Copy link
Member

Typo: seperate -> separate

A valuable optimization we should do is moving everything immutable from ECMAScriptFunctionObject that doesn't change between instantiations and caching it in a shared object, held by the AST node.

This would make instantiation much faster, since we could reuse all the shared data instead of recomputing it. :)

Implement "10.2.5 MakeConstructor" as per the ecma262 spec
Arrange the instantiation of different JS function types
into separate functions that meet the ECMA262 specs.
Use the more spec compliant instantiate_function_object instead of
ECMAScriptFunctionObject::create() where applicable.
Cache prototype shapes and use them when instantiating a new function
object instead of creating a new one on each instantiation.
@mobounya mobounya force-pushed the feat-js_function_instantiation branch from 89baaa4 to 67a0a2d Compare October 12, 2024 11:07
@mobounya
Copy link
Contributor Author

@awesomekling is there anything else that we can cache in ECMAScriptFunctionObject that I'm missing 🤔 ?

@kalenikaliaksandr
Copy link
Member

have you checked how this change affects test262?

@mobounya
Copy link
Contributor Author

@kalenikaliaksandr I've included the test262 diff

@mobounya
Copy link
Contributor Author

Apparently what I thought are improvement to the test262 results are just my machine being slow at times and fast at times, so no changes to the test262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants