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

Static eval backend type #1166

Merged
merged 4 commits into from
Nov 13, 2021
Merged

Static eval backend type #1166

merged 4 commits into from
Nov 13, 2021

Conversation

hhugo
Copy link
Member

@hhugo hhugo commented Nov 8, 2021

No description provided.

@msprotz
Copy link

msprotz commented Nov 9, 2021

Thanks Hugo. I gave this a go and it looks like the partial evaluation isn't working quite as intended:

jonathan@absinthe:/tmp $ cat test.ml 

let main () = if Sys.backend_type = Other "js_of_ocaml" then "yes" else "no"
jonathan@absinthe:/tmp $ cat test.js
(function(g){"use strict";var
a=g.jsoo_runtime,b=a.caml_string_of_jsbytes,c=[0,b("js_of_ocaml")],e=b("yes"),f=b("no"),d=a.caml_get_global_data().Stdlib__sys;a.caml_register_global(4,[0,function(b){return a.caml_equal(d[5],c)?e:f}],"Test");return}(function(){return this}()));

This is with an OPAM pin of this pull request. I would expect the equality test to be skipped and the no case to disappear entirely from the generated JS.

Thanks!

@hhugo
Copy link
Member Author

hhugo commented Nov 9, 2021

It will not work with separate compilation. Try compiling with dune build --profile release. When using separate compilation, we don't leak any cross module information

@msprotz
Copy link

msprotz commented Nov 9, 2021

Here's what I have in my source:

  open Hacl_star

  external whacl_sha2_256_hash: bytes -> bytes = "whacl_sha2_256_hash"

  let sha2_256_hash b =
    if Sys.backend_type = Other "js_of_ocaml" then
      whacl_sha2_256_hash b
    else
      Hacl.SHA2_256.hash b

a git grep confirms that there are no other places that call into the Hacl_star package.

Here's what I'm seeing with --pretty:

193662               _cck_=
193663                caml_equal(backend_type,_bIk_)
193664                 ?whacl_sha2_256_hash(b)
193665                 :caml_call1(_bty_,b);

with:

 29127      _bIk_=[0,caml_string_of_jsbytes("js_of_ocaml")],

and

 30308      backend_type=caml_sys_const_backend_type(0),

am I doing something wrong? this is with dune build --profile=release

@msprotz
Copy link

msprotz commented Nov 9, 2021

just to confirm I have pinned the right branch:

jonathan@absinthe:~/Code/mls-proposals (protz_perf) $ opam pin list | grep js_of_ocaml
js_of_ocaml.3.11.0    git  git+ssh://git@github.com/ocsigen/js_of_ocaml#static-eval-backend-type

@msprotz
Copy link

msprotz commented Nov 9, 2021

and here is the relevant section from my dune file:

 (js_of_ocaml
   (flags "--disable" "genprim" "--opt" "3" "--set" "tc_depth=0" "--pretty")
   (javascript_files "BigInteger.js" "zarith.js" "overrides.js"))

@hhugo
Copy link
Member Author

hhugo commented Nov 9, 2021

I don't think we optimize polymorphic comparison function. Try pattern matching instead.

@msprotz
Copy link

msprotz commented Nov 9, 2021

Source:

  let sha2_256_hash b =
    match Sys.backend_type with
    | Other "js_of_ocaml" ->
        whacl_sha2_256_hash b
    | _ ->
        Hacl.SHA2_256.hash b

JS:

193661              if
193662               (typeof backend_type
193663                !==
193664                "number"
193665                &&
193666                !
193667                caml_string_notequal(backend_type[1],_bIk_))
193668               {var _cck_=whacl_sha2_256_hash(b);switch$0 = 1}
193669              if(! switch$0)var _cck_=caml_call1(_bty_,b);

Note that the caml_call1 is still here, meaning that the codepath that calls into ctypes was not eliminated...

Thanks,

Jonathan

@hhugo
Copy link
Member Author

hhugo commented Nov 10, 2021

I've change the tests to reflect more accurately reality.
I've just noticed you're not using the right version of the jsoo compiler. You've pinned js_of_ocaml but you should have pinned js_of_ocaml-compiler.

On a related note, it seems we have similar interest of using hacl_star in javascript. Do you know about https://gitlab.com/tezos/tezos/-/blob/master/src/lib_hacl_glue/js/src/hacl.ml ? I'd be interested to talk about your use case.

@msprotz
Copy link

msprotz commented Nov 10, 2021

Ah that would explain why I had trouble, then. I'll try again with the right pin.

My use-case is essentially "factor out" the crypto in my application and allow multiple implementations to be used in a JS context, such as:

So, I'm trying to "abstract out" the crypto at a high level.

I took at look at your bindings and it looks like the purpose is to mimic the Hacl_star namespace, but redirecting the calls to JS implementations. Is this using the "virtual library" feature that François Bobot mentioned on the other github issue? If so, this could be a pretty good way to "solve" my problem, and I would actually be quite supportive of integrating this into the official hacl-star repository alongside the regular OCaml bindings, rather than keeping it in the Tezos repository. That way, both could be kept in sync and the hacl-star package could even declare two "virtual libraries" with the "normal" implementation as default and your hacl-js stubs as an alternative.

What do you think?

@msprotz
Copy link

msprotz commented Nov 10, 2021

Thanks, I can confirm that with the new pin on js_of_ocaml-compiler, the calls to ctypes bindings are successfully eliminated from the generated JS in release mode. Unfortunately,

  • I still get a lot of warnings about missing primitives, it looks like merely linking in ctypes generates a lot of static initializers that keep these definitions reachable
  • perhaps as a consequence, I get an exception thrown at JS load-time:

Screen Shot 2021-11-10 at 11 41 46 AM

So, maybe virtual libraries is my only option here. Any thoughts?

@hhugo
Copy link
Member Author

hhugo commented Nov 13, 2021

I'd like to try compile ctypes to jsoo and see where that lead. see hacl-star/hacl-star#494

@hhugo hhugo merged commit c7f9b26 into master Nov 13, 2021
@hhugo hhugo deleted the static-eval-backend-type branch November 13, 2021 10:42
hhugo added a commit to hhugo/opam-repository that referenced this pull request Jan 24, 2022
…s_of_ocaml-ppx_deriving_json, js_of_ocaml-ppx, js_of_ocaml-lwt and js_of_ocaml-compiler (4.0.0)

CHANGES:

## Features/Changes
* Compiler: add --target-env flag, for JS runtime specific compilation targets (ocsigen/js_of_ocaml#1160).
* Compiler: static evaluation of backend_type (ocsigen/js_of_ocaml#1166)
* Compiler: speedup emitting js files (ocsigen/js_of_ocaml#1174)
* Compiler: simplify (a | 0) >>> 0 into (a >>> 0) (ocsigen/js_of_ocaml#1177)
* Compiler: improve static evaluation of cond (ocsigen/js_of_ocaml#1178)
* Compiler: be more consistent dealing with js vs ocaml strings (ocsigen/js_of_ocaml#984)
* Compiler: Compiler: add BigInt to provided symbols (fix ocsigen/js_of_ocaml#1168) (ocsigen/js_of_ocaml#1191)
* Compiler: use globalThis, drop joo_global_object ocsigen/js_of_ocaml#1193
* Compiler: new -Werror flag to turn wanrings into errors (ocsigen/js_of_ocaml#1222)
* Compiler: make the inlining less agressive, reduce size, improve pref (ocsigen/js_of_ocaml#1220)
* Compiler: rename internal library js_of_ocaml-compiler.runtime to js_of_ocaml-compiler.runtime-files
* Lib: new runtime library to improve compatibility with Brr and gen_js_api
* Lib: add messageEvent to Dom_html (ocsigen/js_of_ocaml#1164)
* Lib: add PerformanceObserver API (ocsigen/js_of_ocaml#1164)
* Lib: add CSSStyleDeclaration.{setProperty, getPropertyValue, getPropertyPriority, removeProperty} (ocsigen/js_of_ocaml#1170)
* Lib: make window.{inner,outer}{Width,Height} non-optional
* Lib: introduce Js_of_ocaml.Js_error module, deprecate Js_of_ocaml.Js.Error exception.
* Lib: add deprecation warning for deprecated code
* PPX: json can now be derived for mutable records (ocsigen/js_of_ocaml#1184)
* Runtime: use crypto.getRandomValues when available (ocsigen/js_of_ocaml#1209)
* Misc: move js_of_ocaml-ocamlbuild out to its own repo
* Misc: add support for OCaml 4.14 (ocsigen/js_of_ocaml#1173)

## Bug fixes
* Compiler: fix sourcemap warning for empty cma (ocsigen/js_of_ocaml#1169)
* Compiler: Strengthen bound checks. (ocsigen/js_of_ocaml#1172)
* Compiler: fix `--wrap-with-fun` under node (ocsigen/js_of_ocaml#653, ocsigen/js_of_ocaml#1171)
* Compiler: fix parsing of annotaions in js stubs (ocsigen/js_of_ocaml#1212, fix ocsigen/js_of_ocaml#1213)
* Ppx: allow apostrophe in lident (fix ocsigen/js_of_ocaml#1183) (ocsigen/js_of_ocaml#1192)
* Runtime: fix float parsing in hexadecimal form
* Runtime: fix implementation of caml_js_instanceof
* Graphics: fix mouse_{x,y} (ocsigen/js_of_ocaml#1206)
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.

None yet

2 participants