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

[RFC] Remove List and Array in favour of Dynarray #97

Open
filipeom opened this issue Apr 6, 2024 · 3 comments
Open

[RFC] Remove List and Array in favour of Dynarray #97

filipeom opened this issue Apr 6, 2024 · 3 comments

Comments

@filipeom
Copy link
Member

filipeom commented Apr 6, 2024

I don't think the List type is being used correctly in ecma-sl. Currently, in ecmaref6, there are only 19 uses of hd and 22 uses of tl, whereas l_nth has 382 uses. Additionally, l_add has 105 uses, whereas l_prepend has only 6 uses. This indicates that we are consistently iterating over linked lists to access and add values to this type.

Alternatively, we could use the ecma-sl arrays more. However, having a fixed-size array in ecma-sl doesn't seem to be very useful, as array operations only appear to have 15 uses in total in ecmaref6.

For this reason, I think we should consider implementing Lists as dynamic arrays using the Dynarray type from OCaml 5.2 to address performance bottlenecks. Additionally, we could eliminate the Array value from ecma-sl, which is rarely used in practice.

Tagging for discussion @andreffnascimento @j3fsantos

@filipeom
Copy link
Member Author

filipeom commented Apr 6, 2024

This is similar to adopting rust vectors https://doc.rust-lang.org/std/vec/struct.Vec.html

@andreffnascimento
Copy link
Contributor

I've always had a problem with ECMA-SL arrays, as they rarely seemed to be used (primarily for byte operations, if memory serves me correctly??). I think there was even a point back when I stated to work in this repo where arrays weren't even accepted by the ECMA-SL parser. I'm ok with this change specially since performance is a big issue, but we should check Array/List uses within the JS standard so that we don't start deviating too much from it.

@filipeom
Copy link
Member Author

filipeom commented Apr 6, 2024

I think so, arrays are so rarely used I don't even know 😅

Yeah, I'd also like to do this in a way that there is minimal impact to the current ecmaref's interpreter code so that we remain as close to the standard as possible.

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

No branches or pull requests

2 participants