Improve performance of stdlib immutable Map on JavaScript

[schurhammer]

Issue gleam-lang/gleam#1262

Still WIP but creating PR for feedback.

Reference Implementation https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/PersistentHashMap.java

[schurhammer]

One question is where is isEqual and inspect implemented? I couldn't find the source files. I think there might need to be some changes to these since the same set of entries could potentially have a different internal node structure, especially if/after items had been deleted from the map.

[inoas]

inspect is implemented in the ffi parts:

Erlang: https://github.com/gleam-lang/stdlib/blob/main/src/gleam_stdlib.erl#L324-L377= JavaScript: https://github.com/gleam-lang/stdlib/blob/main/src/gleam_stdlib.mjs#L333= ... the JS part AFAIU leads into https://github.com/gleam-lang/gleam/blob/main/compiler-core/templates/prelude.js#L65-L67=

For isEqual see https://github.com/gleam-lang/gleam/blob/main/compiler-core/templates/prelude.js#L301= I think

[schurhammer]

Thanks inoas :)

We somehow need to get the new Map class into prelude or find a workaround where we don't need to do that. I'm not sure which path to take here. Some options:

1. implement the new Map in prelude instead of stdlib
2. make ther new Map a subclass of the native JS Map, then it should be handled by those code paths in theory
3. support a magic "equals" method, similar to the "inspect" method (prelude.js:283)

[lpil]

3 sounds like it would be useful for other future user implemented data structures as well as this one. Let's go for that one.

I could imagine us in future promoting the JavaScript map implementation into core, but let's be conservative and keep it in the standard library for now.

[schurhammer]

Maybe. I think it'd be nice to keep it separate for the sanity of maintainers. Probably need a better mechanism for including files?

On Mon, 15 Aug 2022, 04:31 inoas, @.***> wrote:

@.**** commented on this pull request.

In src/gleam/map.gleam https://github.com/gleam-lang/stdlib/pull/328#discussion_r945313743:

@@ -5,6 +5,12 @@ if javascript { import gleam/pair }

+if javascript {

• // hack to include another js file..
• pub external fn include_persistent_hash_map() -> Nil =
• "../persistent-hash-map.mjs" "__include_me"

Is this planned to be moved into gleam_stdlib.mjs?

— Reply to this email directly, view it on GitHub https://github.com/gleam-lang/stdlib/pull/328#pullrequestreview-1072104080, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPXYU3R7R66UQLKZO2WRP3VZENP3ANCNFSM54M2QYNQ . You are receiving this because you authored the thread.Message ID: @.***>

[lpil]

Hey @schurhammer ! I finally got to the bottom of my github inbox now that I'm full time on Gleam 😁

Is this something you would like to continue with?

[lpil]

I've rebase this into main