-
Notifications
You must be signed in to change notification settings - Fork 1
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
Comments
This is similar to adopting rust vectors https://doc.rust-lang.org/std/vec/struct.Vec.html |
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. |
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. |
Closing is favor of #132. As the consensus is that we like the property of immutable lists and we already have |
I don't think the
List
type is being used correctly in ecma-sl. Currently, in ecmaref6, there are only 19 uses ofhd
and 22 uses oftl
, whereasl_nth
has 382 uses. Additionally,l_add
has 105 uses, whereasl_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 theDynarray
type from OCaml 5.2 to address performance bottlenecks. Additionally, we could eliminate theArray
value from ecma-sl, which is rarely used in practice.Tagging for discussion @andreffnascimento @j3fsantos
The text was updated successfully, but these errors were encountered: