Skip to content

Discussion: Restructuring OCaml stdlib (original source) to make string compilation better. #924

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

Closed
jordwalke opened this issue Nov 18, 2019 · 9 comments
Labels

Comments

@jordwalke
Copy link

jordwalke commented Nov 18, 2019

I'm not sure what would be required of JavaScript compilation but as part of rehp it had occurred to me that we could output much better string representations if the standard library of OCaml moved more of its string operations to externs:
For example the definition of ^:

external string_length : string -> int = "%string_length"
external bytes_length : bytes -> int = "%bytes_length"
external bytes_create : int -> bytes = "caml_create_bytes"
external string_blit : string -> int -> bytes -> int -> int -> unit
                     = "caml_blit_string" [@@noalloc]
external bytes_blit : bytes -> int -> bytes -> int -> int -> unit
                        = "caml_blit_bytes" [@@noalloc]
external bytes_unsafe_to_string : bytes -> string = "%bytes_to_string"

let ( ^ ) s1 s2 =
  let l1 = string_length s1 and l2 = string_length s2 in
  let s = bytes_create (l1 + l2) in
  string_blit s1 0 s 0 l1;
  string_blit s2 0 s l1 l2;
  bytes_unsafe_to_string s

If compiling using the jsoo bytecode decompiler, and when targeting PHP/Hack, we could actually just emit plain string concatenation in the output. I also believe this is possible in JavaScript as well (by implementing a toString() on MlBytes). But this would require intercepting the ^ which is currently not defined as an extern. It seems it could be implemented as an external without much downside.
If possible, this would improve readability of the output and improve performance of ^.

@TyOverby
Copy link
Collaborator

I also believe this is possible in JavaScript as well (by implementing a toString() on MlBytes)

Wouldn't this require that JavaScript strings support all the contents that MlBytes support?

@hhugo
Copy link
Member

hhugo commented Nov 22, 2019

You could, with some effort, already recognize the sequence of instructions generated for String.concat and emit special code for it.

I also believe this is possible in JavaScript as well (by implementing a toString() on MlBytes)

How does toString matter here ? also MlBytes.toString() already exists.

To make this work in JavaScript, I'd like to change the representation of string to be represented as JavaScript string directly (see #923)

@jordwalke
Copy link
Author

My question about the string ^ function not being an extern, was motivated by my planned effort to allow strings to be encoded in plain strings (php strings in my case). I wanted string ^ operations to be represented as +. Does your attempt at using plain JS strings allow for ^ operations to be performed via simple +? Wouldn't it also need the standard lib to represent this ^ as an external? It seems kind of fragile to try to determine after the fact which operation from the standard library was ^ (unless of course we can somehow retain the names of exported bindings from modules after they are compiled). If we had that ability then this optimization in compile output, and many others would benefit.

@jordwalke
Copy link
Author

@hhugo How close is that diff to being ready? That's pretty exciting stuff.

@hhugo
Copy link
Member

hhugo commented Dec 6, 2019

#923 works already but I'd like to control the new behavior with a flag.

@hhugo
Copy link
Member

hhugo commented Dec 6, 2019

I cannot give you any ETA

@jordwalke
Copy link
Author

I'm mostly curious if you think that you have solved all the major challenges with that approach. Having a flag does sound reasonable.

@hhugo
Copy link
Member

hhugo commented Apr 4, 2020

see #977

@hhugo
Copy link
Member

hhugo commented Jan 7, 2022

Optimization for String.append implemented in #977. Not merged yet. I'll close the issue. Continue discutions in #977

@hhugo hhugo closed this as completed Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants