-
Notifications
You must be signed in to change notification settings - Fork 194
Use javascript string by default #976
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
Conversation
0eeebae
to
fdc7198
Compare
d177ba0
to
4b149ee
Compare
I know this is WIP, but I'm curious. What would be the consequence for users, notably in term of FFI (and performances) ? |
This is not really a WIP anymore. It's being tested now. There should be no implication for FFI. One still need to use existing primitive/stubs to convert between js (utf16) and ocaml (utf8) strings. I would expect some performances improvement but I didn't run any bench yet. |
Hmm, maybe I misunderstood then ! If it's neither for FFI nor perf, what's the goal ? :) |
It would be for perf ! We still need to run some bench. |
(#655 contains all the logic and I believe does some nice cleanup) |
0325ad6
to
64864fd
Compare
@ejgallego, would you be able to test jscoq against this GPR ? |
Hi @hhugo , sorry I missed this. Yes, we can test soon. |
@hhugo I get "caml_jsstring_of_string not implemented", I was able to fix that by adding stuff in |
I don't understand why you get "caml_jsstring_of_string not implemented". Have you pin both js_of_ocaml and js_of_ocaml-compiler to this PR ? |
I did pin a bunch of libs, let me re-pin everything again, indeed maybe I forgot to pin |
Ok, now I got:
in Coq itself. Will switch to debug mode and research more. |
I can help you if you need. Note that marshal on bytes will not work properly. |
https://github.com/jscoq/jscoq/blob/v8.11/coq-js/js_stub/str.js#L63 ( |
Very well spotted indeed (cc: @corwin-of-amber )
that should be easy to patch by myself. |
With that patched, jsCoq demo file seems to work fine! |
Rebased on top of the new Str support. |
I share @Drup's curiosity with thrill. Do you have a design doc somewhere that describes the move and its implications ? The JavaScript vs OCaml strings has been an annoying technical point of jsoo when designing OCaml APIs that interact with the browser APIs. Does it mean that if I have a value of type |
In that mode, an ocaml string will be represented as a javascript string containing a sequence of bytes. It would only be safe to pass it to the dom as is for ascii strings. In the general case, you'll still need to convert the string to utf16. I will update the feature description to explain the design and the plan moving forward. In short:
|
Okay so the dichotomy remains. But I guess given the nature of OCaml strings -- namely a sequence of bytes -- there's not much choice. |
9d23925
to
9006a93
Compare
one more step, num will be compatible with that change ocaml/num#21 |
9006a93
to
02b5ea1
Compare
02b5ea1
to
6228c24
Compare
At least #984 have been merged. |
6228c24
to
fcef82a
Compare
e4ce045
to
30beca9
Compare
30beca9
to
d962c56
Compare
Is there any bench somewhere for this? |
https://ocsigen.org/js_of_ocaml/latest/manual/performances was updated with some numbers. |
Great! Thanks for the pointer! |
In that mode, an OCaml string will be represented as a JavaScript string containing a sequence of bytes. Converting the string to utf16 is still required for interaction with the DOM, or other javascript API.
This current alternative representation of strings (implemented in #923) is not set in stone and could be changed based on benchmarks and usability.
Here are a bunch of things to consider:
MlString
) to ease debugging.string.ml
to make more optimization possible (String.sub
,(^)
, ..)The PR is useful to check that existing program don't make any assumption about the internal representation of strings.