-
Notifications
You must be signed in to change notification settings - Fork 30
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
Migrate to cgroup2, debian:bookworm, Python 3.8+, aiohttp 3+, and ruamel.yaml 0.18.0+ #87
Conversation
jd4/compile.py
Outdated
@@ -155,7 +155,7 @@ async def build(lang, code): | |||
def _init(): | |||
try: | |||
with open(_LANGS_FILE) as file: | |||
langs_config = yaml.load(file, Loader=yaml.RoundTripLoader) | |||
langs_config = yaml.YAML().load(file) |
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.
Constructing the YAML object looks expensive. Maybe create file-level globals.
Test with:
[yaml.YAML() for i in range(100000)][:0]
[y.YAML().load('a:1') for i in range(100000)][:0]
y = yaml.YAML()
[y.load('a:1') for i in range(100000)][:0]
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.
fixed
if not (path.isdir(PIDS_CGROUP_ROOT) and access(PIDS_CGROUP_ROOT, W_OK)): | ||
cgroups_to_init.append(PIDS_CGROUP_ROOT) | ||
if cgroups_to_init: | ||
if not (path.isdir(CGROUP2_ROOT) and access(CGROUP2_ROOT, W_OK)): |
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.
if ... return to simplify control flow
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 cannot return here because we should do the below things (Put myself into the cgroup that I can write.
) each time the process gets started.
else: | ||
logger.error('Cgroup not initialized') | ||
|
||
# Put myself into the cgroup that I can write. |
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.
Why is this not needed previously but needed now?
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.
On systems with cgroup2 enabled, at the beginning, it is very likely that the process is in a cgroup that it does not have permission to write so it cannot move its children from this initial cgroup to the cgroup used for judging (because leaving a cgroup requires write permissions).
if cgroup_file: | ||
enter_cgroup(cgroup_file) | ||
execve(compiler_file, compiler_args, SPAWN_ENV) | ||
try: |
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.
@iceboy233 Is my added try
necessary? What if this code throws an exception? Is this a bug?
@@ -80,7 +80,7 @@ def test_php(self): | |||
""") | |||
|
|||
def test_py(self): | |||
self.do_lang('py', b'print sum(map(int, raw_input().split()))') | |||
self.do_lang('py', b'print(sum(map(int, input().split())))') |
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.
why not just drop py
?
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.
To maintain the compatibility with the front end...
No description provided.