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
Update dependency packages to use dune.2.6.0 and Multicore OCaml 4.10.0 #132
Update dependency packages to use dune.2.6.0 and Multicore OCaml 4.10.0 #132
Conversation
|
The following dependency packages have been upgraded to build with dune.2.6.0:
|
The build is still failing. Should we wait until it turns green for reviewing? |
The CI is using ocaml/opam2 Docker container from DockerHub:
|
I have updated .drone.yml to use ocurrent/opam:debian-10-ocaml-4.10 Docker image that can install dune.2.6.0. Since this PR also requires #131, I have temporarily applied that in the YAML file for the CI to pass.
When both #131 and this PR are merged, I shall create a PR to remove the patch application from the .drone.yml. file. |
.drone.yml
Outdated
- make ocaml-versions/4.07.1.bench | ||
- wget https://github.com/ocaml-bench/sandmark/pull/131.patch | ||
- patch -p1 < 131.patch | ||
- make ocaml-versions/4.10.0+multicore.bench |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we able to keep a CI build against stock OCaml 4.10 in here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can create multiple exec pipelines
in the same .drone.yml. Any specific reason to have it as part of this code base, rather than in stock OCaml 4.10 or our clone of the repository? The status of the build is determined by the success of both the pipeline runs though.
Source: https://docs.drone.io/pipeline/configuration/#multiple-pipelines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy for it to be in the same run or in another run (I think in the past we had two runs, one for stock and one for multicore). I'm just keen to have CI coverage in case we accidentally make a change to packages which works with multicore but not with stock.
As discussed, we need 3 pipelines,
|
The 4.10.0+stock build got killed because it reached the 60 minute default timeout setting. I tested the build on another machine and the benchmarks ran fine though. Since we are using the cloud hosted drone version, the default timeout is limited to 60 minutes. Only if we self-host will we have administrator access and the Drone configuration timeout can be increased. |
@ctk21 Do you have admin access to Drone? |
We don't have admin access. It is provided as a cloud service by Drone. I think we need to find a way for the run to be under 60mins (which is likely going to be a headache anyhow). Do we know what is taking the time? Maybe we need a cut-down config that can give us coverage of the places most likely to go wrong; this feels like the build stages of the benchmarks rather than the execution of all the benchmarks. |
a316377
to
42f85b2
Compare
I have now added a DRY_RUN parameter to the Makefile:
|
@shakthimaan could you summarize what is pending to get this PR merged? |
The PR can be merged to master:
|
The attached patch updates Sandmark to use dune 2.6.0 for Multicore OCaml 4.10.0.
In order to test, you need to first update dune locally on your system using:
You will need to apply this patch and the PR from #131 as well in order to build and test. You can obtain the raw patch from a PR by suffixing with .patch.
For example: https://patch-diff.githubusercontent.com/raw/ocaml-bench/sandmark/pull/131.patch.
You can then proceed to build the dependencies and run the serial benchmarks using the following command:
In order to run the parallel benchmarks, you can use the following in run_all_parallel.sh, and execute the script: