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

Use a socket to communicate with the jit runtime process #4887

Merged
merged 12 commits into from
Apr 17, 2024

Conversation

dolio
Copy link
Contributor

@dolio dolio commented Apr 16, 2024

This reworks the run.native command and the racket backing to use a TCP socket for communication of code and results. This makes it possible for run.native execution to make use of standard in (previously was occupied by communication of the code to the runtime) and for actual execution results to be reported by ucm.

A little error message tweaking is included. I noticed that a lot of blank lines were being generated in some situations, so I switched to using linesNonEmpty.

Parameterizes various things by the ports they interact with. Without
any arguments in `run` mode, it just uses stdin to read, and a dummy
output to write. But when passed a port argument it will open a socket
for those.
Avoids potential lock ups if the runtime process exits before connecting
back.

Also includes some error formatting to avoid empty lines and empty stack
traces.
@aryairani

This comment was marked as outdated.

@aryairani
Copy link
Contributor

aryairani commented Apr 16, 2024

image

Could you add one blank line back between the indented error and the outdented stack trace

@aryairani
Copy link
Contributor

Something (my fault) is causing the jit tests not to actually run in CI — I don't see them actually running on any of these builds, it's just getting a false pass. I will take a look into it tonight or tomorrow. Sorry :-\

@dolio
Copy link
Contributor Author

dolio commented Apr 16, 2024

No worries.

…runtime test results (#4889)

Plus:
* cache jit binaries even if tests fail
* move checkout early enough to hash racket source
@aryairani aryairani self-requested a review April 16, 2024 20:40
@aryairani
Copy link
Contributor

@dolio Ok I think I fixed the test harness 🤞

It's currently failing on a "contract violation" in a "cli args" test:
https://github.com/unisonweb/unison/actions/runs/8712142414/job/23897816805#step:18:118

image image

The evaluated program can ask for command line arguments, but without
intervention, it will receive all the flags from the call to the parent
program. Previously none were expected, but now we have the port.

So, as part of command line parsing, grab all the remaining args and
install them as the whole list of arguments before starting evaluation.
Whitespace differences
@dolio
Copy link
Contributor Author

dolio commented Apr 17, 2024

Okay, took me a bit to figure that one out. The problem was that the evaluated program was getting the -p <port> flags for the runtime communication. So I needed to remove the unison-runtime flags from the list after parsing them.

@aryairani
Copy link
Contributor

Okay, took me a bit to figure that one out. The problem was that the evaluated program was getting the -p <port> flags for the runtime communication. So I needed to remove the unison-runtime flags from the list after parsing them.

Ah — do you like the convention stack and some programs use, where any args before -- go to the runtime and anything after goes to the evaluated program?

@aryairani
Copy link
Contributor

Oh I see what you did there. 👍

@dolio
Copy link
Contributor Author

dolio commented Apr 17, 2024

I think the racket command-line automatically handles --.

@aryairani aryairani merged commit ac4d817 into trunk Apr 17, 2024
16 checks passed
@aryairani aryairani deleted the topic/jit-eval-side-channel branch April 17, 2024 20:06
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