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

Specialize Js.Dict.fromArray to compile to direct JS object creation #6538

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

Conversation

zth
Copy link
Collaborator

@zth zth commented Dec 21, 2023

…when possible.

A Dict is obviously a JS object at runtime, but there has been no way to create a dict in a way that compiles to a regular JS object creation. This can be a bit confusing when looking at the generated code, as well as hurt performance (albeit slightly in most cases) as creating dict objects are done dynamically via an array.

This changes so that any Js.Dict.fromArray call that is an array with static string keys can get compiled to an actual JS object creation.

@cristianoc no idea if this is the right way to go about it, feedback very welcome.

Related issues:

@cknitt
Copy link
Member

cknitt commented Jan 4, 2024

What about RescriptCore.Dict.fromArray?

@zth
Copy link
Collaborator Author

zth commented Jan 4, 2024

Yeah that's a good point, we should make sure this PR handles both.

@zth
Copy link
Collaborator Author

zth commented Jan 25, 2024

Dict should be promoted to a built in type and specialized that way instead.

@zth zth added this to the v12 milestone Jan 25, 2024
This was referenced Jan 29, 2024
@zth zth mentioned this pull request Mar 5, 2024
2 tasks
@zth zth force-pushed the specialize-js-dict-fromArray branch from 45b4d6a to f67c121 Compare May 26, 2024 14:12
@zth zth marked this pull request as ready for review May 26, 2024 19:09
@zth
Copy link
Collaborator Author

zth commented May 26, 2024

@cristianoc please have a look at whether this approach is good and if the optimization is in the correct place. Maybe it could be put somewhere else?

When Core moves into the compiler (#6761) then we'll change this to account for Dict.fromArray as well.

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.

None yet

2 participants