-
Notifications
You must be signed in to change notification settings - Fork 160
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
Allow HPC-GAP to run as a forkable server process. #3715
Conversation
Thanks @rbehrends - this seems to work, we will do some further testing now. |
The one thing that I'm not happy with (yet) is that this basically requires starting the signal handler thread manually if run with |
@rbehrends maybe new command line option ( |
Something like this is what I'm thinking, except that this is only going to be used programmatically, so we don't have to waste a single letter command line option on it. Something like |
@rbehrends let's spare a single-letter option indeed, but both |
The thing is that it's not really "Jupyter mode". What it does is it making sure that GAP starts up with only a single thread running. This is more of a fork-compatible mode. Jupyter is just one application for that. |
I see. And who knows, maybe for SCSCP package it will be useful as well. Anyhow, it serves the purpose for now, and we can use it next week, and think about a good option name in the meantime. Thanks again! |
I've added a |
@rbehrends thanks - then we will update our setup for Jupyter notebooks HPC-GAP demo. |
This option was added in gap-system/gap#3715
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.
In the PR description, you write that you changed the behavior of -S
, but that doesn't really seem to be the case?
@fingolfin See my comment above. I reverted the changes to |
5d9137d
to
5d7dad6
Compare
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.
This actually looks good to me. Why exactly is this marked as WIP? Because the name of that command line option might be changed? Any other reason?
If @alex-konovalov and @stevelinton and @ChrisJefferson are happy with this PR, I am also happy to have it merged :-)
Yeah, it's WIP because I basically just made up a name for the command line option and wanted to first check that this is fine with everyone. |
Also, another thing with |
@rbehrends shall we merge this? |
If it's okay this way, I'll squash the commits, update the commit message, and then we can, yes. |
This commit includes the following changes: 1. The Boehm GC is configured to properly handle fork(). 2. The new --single-thread command line option now starts HPC-GAP up in single-threaded mode without any other threads running. This option implies -S (as the thread UI requires threads). 3. Normally, signals are handled by a separate thread; this is now being done in the function `InstallHPCGAPSignalHandling`. If started with --single-thread, this function needs to be called explicitly in order to enable proper handling of SIGINT etc.
5d7dad6
to
2d354dd
Compare
Update: I've squashed the commits as noted above, if the tests work, it can be merged. |
@rbehrends thanks! |
Shall this be backported to 4.11? |
@fingolfin I think so! |
Backported to stable-4.11 in commit ac4b898 |
This change is prompted by trying to make HPC-GAP work for Jupyter. Note that it changes the meaning of the
-S
command line option and therefore is labeled as WIP. I've put up the pull request so that people can test the functionality of Jupyter + HPC-GAP, even if details still needs to be settled. Note also that so far I've only tested this on macOS.This commit includes the following changes:
mode without any other threads running.
done in the function
InstallHPCGAPSignalHandling
. If started with-S, this function needs to be called explicitly in order to enable
proper handling of SIGINT etc.
Please use the following template to submit a pull request, filling
in at least the "Text for release notes" and/or "Further details".
Thank You!