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

Add implementation for foreign-object #13

Merged
merged 11 commits into from
Jul 3, 2024
Merged

Add implementation for foreign-object #13

merged 11 commits into from
Jul 3, 2024

Conversation

f-f
Copy link
Contributor

@f-f f-f commented May 20, 2024

This PR adds FFI for foreign-object using a symbol-hashtable together with a list to keep track of insertion order, which is an invariant of the JS implementation.

It's not strictly needed for Scheme, but it's good to have for compatibility with existing JS-backend code.

There is one slow test that I need to look more into, but the patch passes all tests otherwise.

@f-f f-f requested a review from anttih May 20, 2024 10:51
@f-f f-f mentioned this pull request May 20, 2024
@f-f
Copy link
Contributor Author

f-f commented May 20, 2024

CI fails in a strange way:

[1/2 ModuleNotFound] foreign-object/src/Foreign/Object.purs:63:1

  63  import Type.Row.Homogeneous (class Homogeneous)
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  Module Type.Row.Homogeneous was not found.
  Make sure the source file exists, and that it has been provided as an input to the compiler.

[2/2 ModuleNotFound] foreign-object/test/Test/Foreign/Object.purs:13:1

  13  import Data.Map as M
      ^^^^^^^^^^^^^^^^^^^^

  Module Data.Map was not found.
  Make sure the source file exists, and that it has been provided as an input to the compiler.

These modules are supposed to be there.

Copy link
Contributor

@anttih anttih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using an alist in a box for the Object? That way you can approximate a mutable object but still be faster than hashtable.

@f-f
Copy link
Contributor Author

f-f commented May 21, 2024

How about using an alist in a box for the Object? That way you can approximate a mutable object but still be faster than hashtable.

I tried the alist but without the box and had issues with the mutability, didn't try wrapping in a box. Are hashtables really that slow? I'll try this out

@anttih
Copy link
Contributor

anttih commented May 22, 2024

You need to have a lot of entries (like ~hundreds) until hashtables are faster than alists, so in that sense they are slow.

@f-f
Copy link
Contributor Author

f-f commented Jul 3, 2024

@anttih moved to alist in a box, it's much faster now.
CI fails because of purescm/purescm#244

@f-f
Copy link
Contributor Author

f-f commented Jul 3, 2024

CI is stuck because the latest Spago is broken, see purescript/spago#1242

@f-f f-f merged commit 98f12cc into master Jul 3, 2024
1 check passed
@f-f f-f mentioned this pull request Jul 4, 2024
56 tasks
@f-f f-f deleted the add-foreign-object branch July 4, 2024 21:09
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.

2 participants