Implement pre-compilation of exercises and graders #481

AltGr · 2022-04-01T12:46:55Z

Close #478, Close #414, Close #205, Close #312, Close #158

AltGr · 2022-04-01T14:36:41Z

This replaces the prepare.ml, solution.ml and test.ml files that were sent scrambled and evaluated textually in the grader's toplevel by compiled files (interfaces in cmi, implementation in cma and js depending on the backend).

The main benefit is two-fold:

it's now impossible to extract the solution (or grader) from a running server: only the compiled version — useless for cheating — is available

it's much more efficient:

                          MASTER  PRECOMPILE
  build -j1               1005s   33s
  build -j12              (broken)7.6s
  loading exercise        5.8s    5.2s
  grading (100%)          6.4s    4.0s

(these have been run on the learn-ocaml-corpus repository and the markov exercise, respectively. Check my (temporary) patches here)

That's a 132x speedup on learn-ocaml build in the best case!

Todo:

re-implement depend.txt handling (or replace it with something more powerful, it fits much better in the new backend and will only be needed at build-time); and check that the (temporarily disabled) mutations_test.ml works with it
add the ppx to introduce the register_sampler calls that are now required (@hernoufM)
samplers are not properly type-checked yet
one grader of the corpus segfaults (week5/seq1/ex2) (but see point just above!)
asak/solution partitioning is disabled: if someone knowing that part of the code could have a look, @nobrakal ?
restore feature to define custom printers
check how to handle the emacs-mode with the new server (@erikmd ?)
check that the small changes required in exercises are OK with users (in progress on the mailing-list)

AltGr · 2022-04-21T17:32:43Z

This should finally be all good feature-wise. Last bits:

Some documentation
Fix static builds / docker containers ; building an exercise repository now requires ocamlc to be available

AltGr · 2022-04-22T15:41:17Z

This is finalised and, I think, ready to review and go!
Report on the status of the tests:

✔️ building the static binaries works fine
✔️ building the Docker images and running the dockerised tests work fine
🟠 Build learn-ocaml and run tests uses master of learn-ocaml-corpus. There are some small changes required, detailed in this PR. These should be backwards-compatible¹ and I can confirm running the test locally with that branch of the corpus works.
✔️ Build learn-ocaml-client and run quick tests works on the current version image
🔴 Build learn-ocaml-client and run quick tests (ocamlsf/learn-ocaml:0.12/0.13.0): these tests check for HTTP API compatibility with learn-ocaml 0.12 and 0.13.0, which is a non-goal of this PR (we don't want to provide solution.ml files in any form anymore!). A major version bump is in order, and this will only affect users of the command-line client.

¹ at the loss however of the installed printers, on older versions of learn-ocaml, since they relied on the #install_printer directive which we can't keep, and they don't have the new mechanism. If this is a problem we can have a separate branch in learn-ocaml corpus and point the test to that.

AltGr · 2022-04-22T16:02:27Z

~~Ah, I forgot to add the copyright headers to all the new files.~~ [EDIT: done]

AltGr · 2022-06-04T10:49:32Z

I know some teachers who were expecting this to be at least merged for their classes in June; can we go forward, or should I advise them to use an unofficial branch ?

erikmd · 2022-06-07T14:19:11Z

@AltGr thanks for your ping 👍. Sorry, was swamped with teaching lately, my plan actually is to integrate it as soon as possible once:

asak-partitioning implementation is updated (I will email Alexandre to arrange some meeting slot soonish)
learn-ocaml.el mode is updated (will be easy AFAICT, will let you know ASAP)
and ideally after discussing with you, to figure out some way to keep compatibility with learn-ocaml-editor! 🙏
and finally after a slight-multi-squash to be fully release-please compliant

As a result, I'm afraid the feature won't be released this month, but definitely in July for September.

nobrakal · 2022-06-07T20:16:09Z

Woops, I am really sorry for my lack of reply, I did not see the previous mention.

I wonder if anyone actually uses the partitioning feature, I hope this is not a release-blocker.

Anyway, even if I lack the time to implement any modification by myself, I am available to review or discuss any change if needed.

erikmd · 2022-06-07T22:38:11Z

Hi!

Woops, I am really sorry for my lack of reply, I did not see the previous mention.

No worries, @nobrakal !

I wonder if anyone actually uses the partitioning feature,

I think so (and hope so :)

I hope this is not a release-blocker.

But even if we don't have usage statistics, I think it is a "release-blocker", at least given the asak-partitioning feature was part of the previous stable releases of learn-ocaml!

Anyway, even if I lack the time to implement any modification by myself, I am available to review or discuss any change if needed.

OK, thanks… anyway we'll be able to discuss this with @yurug at the end of this week

AltGr · 2022-09-15T11:42:56Z

Having rushed to have this ready back in April, I have to say I am pretty disappointed that this hasn't been merged yet, esp. since I told a few teachers that it would be in a release for the semester start in September.

I understand the issue with Asak getting temporarily disabled, but, like @nobrakal, I don't think that should be a release blocker: this is a massive overall improvement for everyone (the 100x speedup on learn-ocaml build is no joke! And the solution hiding was requested by many for a long time) while Asak remains a niche feature. Don't get me wrong, it's a nice one, but if advertised correctly in the new release people who rely on it will know to stay on the current release until ported, while not holding the others back. The fact that the feature is not yet visible in the interface should help here, too (you have to know to middle-click on a specific item).

AltGr · 2022-09-15T11:49:05Z

@yurug I see you made attempt to repair Asak, thanks! did it work ? The patches make prepare.ml visible to learn-ocaml build but not further on a built instance and I was thinking that the partitioning was taking place on a running instance.

I don't remember precisely how Asak works but I think it should in theory be fine with just the cmi for prepare.ml ?

erikmd · 2022-09-20T03:04:16Z

Hi @AltGr, thanks a lot for your messages!, and for this very nice work 🙏

Having rushed to have this ready back in April, I have to say I am pretty disappointed that this hasn't been merged yet, esp. since I told a few teachers that it would be in a release for the semester start in September.

I fully understand—AFAICT, my OCaml lab sessions precisely start this week, and I would have loved to benefit from #481.

No need to tell many details about the "why", but Yann and I got much less time than expected lately.

BTW:

If ever #475 had been implemented, PR #481 would immediately be "available" / easy to deploy by relying on static binaries, without any release. So I just labelled #475 as good first issue in passing 🙂
Also the CI was broken till a few days ago… (the fix was simple; but before that moment, doing any release was impossible).
Finally, you mentioned the asak issue (which is not an undocumented feature BTW 😉) but this is not the very last small point that remains to address.
Anyway, @yurug and I restarted last week our recurrent learn-ocaml maintainer telco; so, no ETA; but stay tuned 👍

gasche · 2022-11-14T09:26:11Z

@erikmd and @yurug : gentle ping, have you been able to make progress on integrating this PR?

erikmd · 2022-12-29T11:15:45Z

gentle ping, have you been able to make progress on integrating this PR?

@gasche sure! as said previously, this PR is a priority! in particular as soon as the bugfix PR #506 is merged (certainly at our next learn-ocaml maintainer telco), we'll make a point release, then work on this PR #481. We'd also like to invite @AltGr at that point so that we can talk together with @yurug for the last steps of the review.

hhugo · 2023-02-15T19:33:56Z

src/toploop/toploop_jsoo.ml

+
+let use_compiled_string code =
+  (* jsoo supports dynload, but relies on expectations on the parent object that
+     are no longer valid when running from a web-worker. Thus we compile with


Why is it no longer valid ? Can anything be done on the jsoo side ? Consider opening a jsoo issue.

Ha, I am sorry, that was too long ago for me to remember precisely 😬
Indeed this could probably be worked around in jsoo; from what I remember, just trying to dynload from within a webworker is enough to point to the issue, with errors about stuff missing from the parent object.

`learn-ocaml build` now requires write access to the repository since it writes compilation artefacts in-place.

Signed-off-by: Yann Regis-Gianas <yann@regis-gianas.org>

…haywire An uncaught exception could get caught upper on the stack, and lead the worker to start running pending lwt stuff that belong to the master.. Also attempt to fix "too many open files" error with many workers

since the libraries are already available

asak 0.4 is now released on opam

It's not expected that we remain compatible with versions that required exposing `solution.ml`.

these are required for compiling certain exercises

Closes ocaml-sf#529 which seemed to be a common complaint among teachers. * `learn-ocaml serve --replace` will kill an existing server (running on the same port) just before starting * `learn-ocaml build serve` with an existing server on the same port will fail fast (before actually doing the build) * `learn-ocaml build serve --replace` is more clever: - it will do the build *in a temporary directory* - then, only if everything is ok, kill the older server - swap the files and start the new server This is all done in order to minimise downtime and be convenient for server updates. Note that this PR sits on top of ocaml-sf#481 and should be rebased once it's merged.

erikmd · 2023-11-03T13:43:28Z

Hi @AltGr, thanks for the merge!, but please wait for the learn-ocaml.el and learn-ocaml-client fixes before we can release 1.0.0 🚀

gasche · 2023-11-03T13:46:58Z

Congratulations for the merge!

AltGr · 2023-11-03T14:51:33Z

@erikmd Of course! Porting them should be less tedious now that this is merged

AltGr · 2023-11-03T14:57:03Z

It'll probably be worth releasing a 1.0~beta asap, to have a little more testing before the final release

erikmd · 2023-11-03T15:08:15Z

Hi @AltGr,

More testing would be nice indeed!, but I think this does not require doing a beta release at all
(all the more as release-please does not support beta releases AFAIK).

Indeed, end users can already use the ocamlsf/learn-ocaml:master Docker image,
or the static binaries attached to the ✔️ last master commit.

Anyway, maybe you could prepare an announcement text mentioning ocamlsf/learn-ocaml:master and post it on https://discuss.ocaml.org? WDYT?

AltGr marked this pull request as draft April 1, 2022 12:47

erikmd added the kind: feature New user-facing feature. label Apr 1, 2022

erikmd added this to the learn-ocaml 1.0.0 milestone Apr 1, 2022

erikmd mentioned this pull request Apr 5, 2022

Upgrade to cmdliner.1.1.1 and improve clarity of manuals #480

Merged

4 tasks

AltGr marked this pull request as ready for review April 21, 2022 17:35

AltGr force-pushed the a7977b52 branch 4 times, most recently from 0699cef to d348acb Compare April 22, 2022 15:41

erikmd self-assigned this Apr 28, 2022

erikmd mentioned this pull request Sep 9, 2022

meta-issue: learn-ocaml 0.13.1+ release plan and milestones suggestions for upcoming releases #437

Closed

15 tasks

erikmd added the priority: high label Dec 17, 2022

erikmd mentioned this pull request Jan 7, 2023

meta-issue: learn-ocaml 0.15.0+ release plan and milestones suggestions for upcoming releases #508

Open

erikmd modified the milestones: learn-ocaml 1.1.0, learn-ocaml 1.0.0 Jan 7, 2023

yurug mentioned this pull request Jan 27, 2023

[build] Generate given_exo.{cmi,cmo,cmt} files #458

Closed

3 tasks

hhugo reviewed Feb 15, 2023

View reviewed changes

AltGr and others added 18 commits November 3, 2023 13:55

docs: Update French translation

f028b75

docs!: Remove doc tutorial on depend.txt (it will need rewriting)

9155145

docs: Update doc for pre-compiled exercises + test_libs.txt

2c89d9e

fix: Fix dune dependency glitch on recompilation of mutation_test

32ad13e

fix(docker): Include jsoo in Dockerfile, which is now needed

466e80c

fix(ci): Fix permission issues

fa2cd23

`learn-ocaml build` now requires write access to the repository since it writes compilation artefacts in-place.

docs: Add/Update copyright headers

5b4e0ab

fix: Expose prepare.ml file

365cbb7

Signed-off-by: Yann Regis-Gianas <yann@regis-gianas.org>

fix(partition-view): Reactivate the feature

57ca10b

Signed-off-by: Yann Regis-Gianas <yann@regis-gianas.org>

fix(CLI): Report JSON parse error origin and locations

ee57ac1

fix(grader): allow exercises to use vg, gg

ead187e

since the libraries are already available

fix(teacher_tab): use newer asak compatible with precompilation

942edc2

asak 0.4 is now released on opam

doc: update index.md

f572990

fix(build): update lockfiles

f1abb7d

fix(CI): attempt to fix running the docker image on the corpus

b94f053

fix(CI): disable compat tests with 0.12, 0.13

91a418e

It's not expected that we remain compatible with versions that required exposing `solution.ml`.

fix(docker): install more libs in server image

6ce797f

these are required for compiling certain exercises

AltGr force-pushed the a7977b52 branch from 0a507b6 to 6ce797f Compare November 3, 2023 12:56

AltGr self-assigned this Nov 3, 2023

AltGr merged commit 6356328 into ocaml-sf:master Nov 3, 2023
12 checks passed

lsylvestre mentioned this pull request Dec 12, 2023

remove residual code from PR #320, which is no longer useful #578

Open

erikmd removed their assignment Feb 12, 2024

dm0n3y mentioned this pull request Feb 19, 2024

Question: what is the 1.0 alternative to private grader helper modules a la depend.txt? #588

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement pre-compilation of exercises and graders #481

Implement pre-compilation of exercises and graders #481

AltGr commented Apr 1, 2022 •

edited by erikmd

AltGr commented Apr 1, 2022 •

edited

AltGr commented Apr 21, 2022 •

edited

AltGr commented Apr 22, 2022

AltGr commented Apr 22, 2022 •

edited

AltGr commented Jun 4, 2022

erikmd commented Jun 7, 2022 •

edited

nobrakal commented Jun 7, 2022

erikmd commented Jun 7, 2022

AltGr commented Sep 15, 2022

AltGr commented Sep 15, 2022

erikmd commented Sep 20, 2022

gasche commented Nov 14, 2022

erikmd commented Dec 29, 2022

hhugo Feb 15, 2023

AltGr Mar 1, 2023

erikmd commented Nov 3, 2023 •

edited

gasche commented Nov 3, 2023

AltGr commented Nov 3, 2023

AltGr commented Nov 3, 2023

erikmd commented Nov 3, 2023 •

edited

Implement pre-compilation of exercises and graders #481

Implement pre-compilation of exercises and graders #481

Conversation

AltGr commented Apr 1, 2022 • edited by erikmd

AltGr commented Apr 1, 2022 • edited

AltGr commented Apr 21, 2022 • edited

AltGr commented Apr 22, 2022

AltGr commented Apr 22, 2022 • edited

AltGr commented Jun 4, 2022

erikmd commented Jun 7, 2022 • edited

nobrakal commented Jun 7, 2022

erikmd commented Jun 7, 2022

AltGr commented Sep 15, 2022

AltGr commented Sep 15, 2022

erikmd commented Sep 20, 2022

gasche commented Nov 14, 2022

erikmd commented Dec 29, 2022

hhugo Feb 15, 2023

Choose a reason for hiding this comment

AltGr Mar 1, 2023

Choose a reason for hiding this comment

erikmd commented Nov 3, 2023 • edited

gasche commented Nov 3, 2023

AltGr commented Nov 3, 2023

AltGr commented Nov 3, 2023

erikmd commented Nov 3, 2023 • edited

AltGr commented Apr 1, 2022 •

edited by erikmd

AltGr commented Apr 1, 2022 •

edited

AltGr commented Apr 21, 2022 •

edited

AltGr commented Apr 22, 2022 •

edited

erikmd commented Jun 7, 2022 •

edited

erikmd commented Nov 3, 2023 •

edited

erikmd commented Nov 3, 2023 •

edited