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

Remove js_of_ocaml #310

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

moazzammoriani
Copy link
Contributor

This PR Fixes #18.

@Sudha247
Copy link
Contributor

Good work on enabling js_of_ocaml again!

Now, if you see the CI run, the 5.0.0+trunk run has failed as expected: https://cloud.drone.io/ocaml-bench/sandmark/1053/3/2.

This is since the version on js_of_ocaml-compiler we have in sandmark is known to not work with the 5.0.0+trunk compiler. In order to fix this, you will have to update the version of js_of_ocaml present in dependencies/.

For that, you will have to add a new version of js_of_ocaml-compiler to dependencies/packages: https://github.com/ocaml-bench/sandmark/tree/main/dependencies/packages/js_of_ocaml-compiler. You can create a new directory there called js_of_ocaml-compiler.4.0.0 and copy the opam file from here (https://github.com/ocaml/opam-repository/blob/master/packages/js_of_ocaml-compiler/js_of_ocaml-compiler.4.0.0/opam) to the newly created directory.

A couple of items need to be updated in the opam file:

depends: [
  "dune" {>= "2.9"}
  "ocaml" {>= "4.04" & < "5.01.0"}
  "num" {with-test}
  "ppx_expect" {>= "v0.14.2" & with-test}
  "ppxlib" {>= "0.15.0"}
  "re" {with-test}
  "cmdliner" {>= "1.1.0"}
  "menhir"
  "menhirLib"
  "menhirSdk"
  "yojson"
  "odoc" {with-doc}
]

The CI should be green after this.

@moazzammoriani
Copy link
Contributor Author

moazzammoriani commented Mar 29, 2022

the checksum that you're talking about, is it the one in the field in this screenshot or the one in the url of the repo you've linked to? scrot29march

@Sudha247
Copy link
Contributor

the checksum that you're talking about, is it the one in the field in this screenshot or the one in the url of the repo you've linked to?

Yep, it's the one you pointed to.

@moazzammoriani
Copy link
Contributor Author

moazzammoriani commented Mar 29, 2022

I committed and pushed this new change but I think it still failed. Did I do something wrong?

@moazzammoriani
Copy link
Contributor Author

moazzammoriani commented Mar 29, 2022

Line 495 of the log at https://cloud.drone.io/ocaml-bench/sandmark/1053/3/2 seems to be giving the same error message as before.

@Sudha247
Copy link
Contributor

I think run completed successfully now: https://cloud.drone.io/ocaml-bench/sandmark/1054/3/2. Great work @moazzammoriani! :)

Before merge this PR, could you do a run on your local machine and report the results for the js_of_ocaml benchmark? The result would look something like this with the name js_of_ocaml. No worries if not, I'll do it later today and merge this PR. Thanks for your contribution!

@moazzammoriani
Copy link
Contributor Author

I tried running it on my local machine but I seem to be getting this error for some reason even though I do recall installing all the dependencies for sandmark and adding sandmark to PATH.
sanderr

@Sudha247
Copy link
Contributor

I suspect the error is due to CPU number here: https://github.com/ocaml-bench/sandmark/blob/main/run_config.json#L5. I'd change the --cpu-list to 1 and run again.

We usually run it on >20 core server machines, hence 5 is a valid CPU number. Laptop/PCs usually have 4 cores and 5 isn't a valid number for it. Hope this helps.

@moazzammoriani
Copy link
Contributor Author

Unfortunately that did not fix it :(

@Sudha247
Copy link
Contributor

All right! Afraid not, I'll give this a spin tomorrow and let you know what the error is. Meanwhile, feel free to pick up some other issue if you'd like.

@Sudha247
Copy link
Contributor

I did a run on this branch locally and I think I've narrowed down what the error is; so the benchmark uses frama-c.byte as input for the js_of_ocaml executable. It happens that frama-c is not compatible with 5.0.0+trunk, so it isn't installed, in effect the file is not found. I'll try to get a new input file for this.

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.

js_of_ocaml fails to run on multicore
2 participants