Skip to content

i_1262 Improve sandbox scripts for better reporting CPU time accuracy #1263

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.

Sign up for GitHub

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

Open
johnbrvc wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

i_1262 Improve sandbox scripts for better reporting CPU time accuracy #1263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fd7a88
i_1262 Improve sandbox scripts for better CPU time accuracy
johnbrvc Apr 20, 2026
f03b7c0
i_1262 PR Change requests in comment
johnbrvc Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 49 additions & 31 deletions scripts/pc2sandbox.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# $7... : command arguments
#
# Author: John Buck, based on earlier versions by John Clevenger and Doug Lane

DEFAULT_CPU_NUM=3
CPU_OVERRIDE_FILE=$HOME/pc2_cpu_override

Expand Down Expand Up @@ -43,7 +44,7 @@ MAXPROCS=32

# Compute taskset cpu mask for running submission on single processor

# Get system's maximum CPU number
# Get system's maximum CPU number - numbered starting at 0
MAX_CPU_NUM=`lscpu -p=cpu | tail -1`

# See if the admin wants to override the CPU by reading the override file
Expand Down Expand Up @@ -71,13 +72,11 @@ then
if [[ "$cpunum" =~ ^[1-9][0-9]*$ ]]

@clevengr clevengr Jun 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears to me that the regex in this line requires cpunum to start with a digit between 1 & 9. That excludes CPU 0. I don't understand why.

@johnbrvc johnbrvc Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that block of code, the script is trying to "guess" which CPU to use based on the logged in user's account ($USER). It will isolate the judge # from the logged in user name, eg. judge1 -> 1. Since we do not allow "judge0", we'd want that to use the default. This, code is used, for example, in non-WF type events, if you have multiple judges running on a multi-core system. Each judge can be pinned to it's own CPU based on its judge number, eg. judge1 -> cpu1, judge2 -> cpu2, etc.

then
# Restrict to number of CPU's.
cpunum=$(((cpunum-1)%(MAX_CPU_NUM+1)))
cpunum=$((cpunum%(MAX_CPU_NUM+1)))
else
cpunum=$(((DEFAULT_CPU_NUM+1)))
cpunum=$DEFAULT_CPU_NUM
fi
fi
cpunum=$((cpunum-1))
CPUMASK=$((1<<cpunum))

# Process ID of submission
submissionpid=""
Expand Down Expand Up @@ -110,6 +109,13 @@ function DEBUG()
[ "$_DEBUG" == "on" ] && "$@" >> $DEBUG_FILE
}

# control whether the script outputs debug info for timing details
_TDEBUG="on" # change this to anything other than "on" to disable debug/trace output
function TDEBUG()
{
[ "$_TDEBUG" == "on" ] && "$@" >> $DEBUG_FILE
Comment thread
clevengr marked this conversation as resolved.
}

# For per-testcase reporting/logging
function REPORT()
{
Expand Down Expand Up @@ -369,19 +375,17 @@ else
echo "max" > $PC2_SANDBOX_CGROUP_PATH/memory.swap.max
fi

# Create variables for various ulimit values. These will be set once inside the cgroup.
# We use ulimit to limit CPU time, not cgroups. Time is supplied in seconds. This may have to
# be reworked if ms accuracy is needed. The problem is, cgroups do not kill off a process that
# exceeds the time limit, ulimit does.
TIMELIMIT_US=$((TIMELIMIT * 1000000))
REPORT_DEBUG Setting cpu limit to $TIMELIMIT_US microseconds "("ulimit -t $TIMELIMIT ")"
ulimit -t $TIMELIMIT

MAXPROCS=$((MAXPROCS+`ps -T -u $USER | wc -l`))
REPORT_DEBUG Setting maximum user processes to $MAXPROCS
ulimit -u $MAXPROCS

REPORT_DEBUG Setting stack size to unlimited
ulimit -s unlimited

# Keep track of details for reports
REPORT_BRIEF ${JUDGEIN}
Expand All @@ -390,49 +394,63 @@ REPORT_BRIEF $cpunum
REPORT_BRIEF $$
REPORT_BRIEF $(date "+%F %T.%6N")

# Remember wall time when we started
starttime=`GetTimeInMicros`

#put the current process (and implicitly its children) into the pc2sandbox cgroup.
REPORT Putting $$ into $PC2_SANDBOX_CGROUP_PATH cgroup
if ! echo $$ > $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
then
echo $0: Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs - not executing submission.
SysFailure Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
exit $FAIL_SANDBOX_ERROR
fi
TDEBUG echo -n 'CGroup2 cpu usec to start: '
TDEBUG grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat

# run the command
# execute it directly (it's a child so it should still fall under the cgroup limits).
REPORT_DEBUG Executing "setsid taskset $CPUMASK $COMMAND $*"
REPORT_DEBUG Executing "setsid taskset -c ${cpunum} $COMMAND $*"

# Set up trap handler to catch wall-clock time exceeded and getting killed by PC2's execute timer
trap HandleTerminateFromPC2 15

# This will create a new process group
#bash -imc "taskset ${CPUMASK} $COMMAND $*" <&0 &
setsid taskset ${CPUMASK} $COMMAND $* <&0 &
TDEBUG echo -n 'CGroup2 cpu usec right before starting submission: '
TDEBUG grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat

# Remember wall time when we started
starttime=`GetTimeInMicros`

# Get CPU starting time (first line)
read cpustart < $PC2_SANDBOX_CGROUP_PATH/cpu.stat

# This will create a new process group and put it into the created cgroup
( if ! echo $BASHPID > $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
then
echo $0: Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs - not executing submission.
SysFailure Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
exit $FAIL_SANDBOX_ERROR
fi
ulimit -t ${TIMELIMIT} -u ${MAXPROCS} -s unlimited
exec setsid taskset -c ${cpunum} $COMMAND $*
) <&0 >&1 2>&2 &

# Remember child's PID/PGRP for possible killing off later
submissionpid=$!

DEBUG echo Waiting for submission pid $submissionpid
# Wait for child
wait $submissionpid

COMMAND_EXIT_CODE=$?

# Get cpu time used by submission
cputime=`grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat | cut -d ' ' -f 2`

# Get wall time - we want it close to when we fetch the sub-process (submission) completes
endtime=`GetTimeInMicros`

@clevengr clevengr Jun 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears to me that computing endtime here (after the calculation of cputime) means that the time taken by the grep and the cut in the preceeding statement will be included in the process' execution time. Am I wrong? Is that intentional? (It seems like computing endtime should be done before the computation of cputime -- and possibly even before the saving of COMMAND_EXIT_CODE; i.e. immediately after the wait is finished...)

@johnbrvc johnbrvc Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No the times are NOT included in the process' execution time. That was the point of the moving the code around. Line 416-425 put a SUBSHELL ($BASHPID) only in the cgroup:

# This will create a new process group and put it into the created cgroup
( if ! echo $BASHPID > $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
  then
    echo $0: Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs - not executing submission.
    SysFailure Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
    exit $FAIL_SANDBOX_ERROR
  fi
  ulimit -t ${TIMELIMIT} -u ${MAXPROCS} -s unlimited
  exec setsid taskset -c ${cpunum} $COMMAND $*
) <&0 >&1 2>&2 &

Hence the ( ...) &. Parens in bash create a subshell. So, process $! is the PID of the created subshell in the parens, which we assign to submissionpid. ($submissionpid is the process id that is in the cgroup, NOT the current shell.) The only thing charged to the process' cpu time is ulimit -t ... and the "exec setsid taskset", all of which are necessary and have to be done before firing off the submission's executable in the context of the new submissions PID to set the time limit and session ID and pin to a specific CPU.

We DO charge the wall-time used by grep usage_usec from $PC2_SANDBOX_GROUP_PATH/cpu.data to the submission, but since we don't use wall time for anything other than display, it's not really important to worry about that. I guess technically, we could move the:

endtime=`GetTimeInMicros`

to right before the:

cputime=`grep ...`

but again, it's not important.

@clevengr clevengr Jul 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was exactly my point: it seems like endtime=GetTimeInMicros should come before cputime=grep ... Otherwise, as you seem to agree, the code is charging the time to execute the grep (and also the cut, I think?) to the execution time of the submission. It's not clear to me why you think it's "not important"; doesn't that add inaccuracy to the computed execution time?

@johnbrvc johnbrvc Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endtime is the WALL time; which is NOT that important. cputime is the important one. If wall time is off by, say, 5ms, it wont make any difference to anyone. Wall is affected by many things; system load, disk I/O, etc., all of which are not charged to the CPU time of the run since we only look at the real CPU time from the cgroup. We only use wall time as a failsafe, eg. if some contestant does "sleep(10);" That's what those thresholds are for that we added to the Settings (so-called "grace periods"). Since sleep(10) uses no cpu time, the pc2 walltime timers will kick in and kill it.

Now, this IS different from the way that PC^2 USED to work when we only used wall time to measure CPU time (and, the way it does it if sandboxes aren't used). Now, wall time is just for information purposes and really isn't used (unless the failsafe trips as described above).

walltime=$((endtime-starttime))

read cpuend < $PC2_SANDBOX_CGROUP_PATH/cpu.stat

TDEBUG echo CGroup2 Start: $cpustart End: $cpuend

# See if we were killed due to memory - this is a kill 9 if it happened

kills=`grep oom_kill $PC2_SANDBOX_CGROUP_PATH/memory.events | cut -d ' ' -f 2`

KillChildProcs

# Get cpu time used.
cputime=`grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat | cut -d ' ' -f 2`

# Get wall time - we want it as close as possible to when we fetch the cpu time so they stay close
# since the cpu.stat includes the time this script takes AFTER the submission finishes.
endtime=`GetTimeInMicros`
walltime=$((endtime-starttime))
TDEBUG echo -n 'CGroup2 cpu usec after kill all children: '
TDEBUG grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat

# Newer kernels support memory.peak, so we have to check if it's there.
if test -e $PC2_SANDBOX_CGROUP_PATH/memory.peak
Expand Down
68 changes: 46 additions & 22 deletions scripts/pc2sandbox_interactive.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,11 @@ then
if [[ "$cpunum" =~ ^[1-9][0-9]*$ ]]
then
# Restrict to number of CPU's.
cpunum=$(((cpunum-1)%(MAX_CPU_NUM+1)))
cpunum=$((cpunum%(MAX_CPU_NUM+1)))
else
cpunum=$(((DEFAULT_CPU_NUM+1)))
cpunum=$DEFAULT_CPU_NUM
fi
fi
CPUMASK=$((1<<cpunum-1))

# Process ID of submission
submissionpid=""
Expand Down Expand Up @@ -137,6 +136,13 @@ function DEBUG()
[ "$_DEBUG" == "on" ] && "$@" >> $DEBUG_FILE
}

# control whether the script outputs debug info for timing details
_TDEBUG="on" # change this to anything other than "on" to disable debug/trace output
function TDEBUG()
{
[ "$_TDEBUG" == "on" ] && "$@" >> $DEBUG_FILE
}

# For per-testcase reporting/logging
function REPORT()
{
Expand Down Expand Up @@ -223,6 +229,8 @@ HandleTerminateFromPC2()
REPORT_DEBUG "Killing off submission process group $submissionpid and all children"
KillValidator
KillChildProcs
GetSandboxStats
ShowStats ${cputime} ${TIMELIMIT_US} ${walltime} ${peakmem} $((MEMLIMIT*1024*1024))
REPORT_DEBUG Wall time exceeded - exiting with code $FAIL_WALL_TIME_LIMIT_EXCEEDED
exit $FAIL_WALL_TIME_LIMIT_EXCEEDED
}
Expand All @@ -242,6 +250,8 @@ GetSandboxStats()
{
# Get cpu time immediately to minimize any usage by this shell
cputime=`grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat | cut -d ' ' -f 2`
read cpuend < $PC2_SANDBOX_CGROUP_PATH/cpu.stat
TDEBUG echo CGroup2 Start: $cpustart End: $cpuend
# Get wall time - we want it as close as possible to when we fetch the cpu time so they stay close
# since the cpu.stat includes the time this script takes AFTER the submission finishes.
endtime=`GetTimeInMicros`
Expand Down Expand Up @@ -532,20 +542,18 @@ $VALIDATOR $JUDGEIN $JUDGEANS $INT_FEEDBACKDIR > $INFIFO < $OUTFIFO &
intv_pid=$!
REPORT_DEBUG Started interactive validator PID $intv_pid

# Create variables for various ulimit values. These will be set once inside the cgroup.

# We use ulimit to limit CPU time, not cgroups. Time is supplied in seconds. This may have to
# be reworked if ms accuracy is needed. The problem is, cgroups do not kill off a process that
# exceeds the time limit, ulimit does.
TIMELIMIT_US=$((TIMELIMIT * 1000000))
REPORT_DEBUG Setting cpu limit to $TIMELIMIT_US microseconds "("ulimit -t $TIMELIMIT ")"
ulimit -t $TIMELIMIT

MAXPROCS=$((MAXPROCS+`ps -T -u $USER | wc -l`))
REPORT_DEBUG Setting maximum user processes to $MAXPROCS
ulimit -u $MAXPROCS


REPORT_DEBUG Setting stack size to unlimited
ulimit -s unlimited

# Keep track of details for reports
REPORT_BRIEF ${JUDGEIN}
Expand All @@ -554,23 +562,32 @@ REPORT_BRIEF $cpunum
REPORT_BRIEF $$
REPORT_BRIEF $(date "+%F %T.%6N")

TDEBUG echo -n 'CGroup2 cpu usec to start: '
TDEBUG grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat

# run the command
REPORT_DEBUG Executing "setsid taskset -c ${cpunum} $COMMAND $* < $INFIFO > $OUTFIFO"

TDEBUG echo -n 'CGroup2 cpu usec right before starting submission: '
TDEBUG grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat

# Remember wall time when we started
starttime=`GetTimeInMicros`

#put the current process (and implicitly its children) into the pc2sandbox cgroup.
REPORT Putting $$ into $PC2_SANDBOX_CGROUP_PATH cgroup
if ! echo $$ > $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
then
echo $0: Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs - not executing submission.
SysFailure Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
rm -f "$INFIFO" "$OUTFIFO"
exit $FAIL_SANDBOX_ERROR
fi
# Get CPU starting time (first line)
read cpustart < $PC2_SANDBOX_CGROUP_PATH/cpu.stat

# run the command
REPORT_DEBUG Executing "setsid taskset $CPUMASK $COMMAND $* < $INFIFO > $OUTFIFO"
# This will create a new process group and put it into the created cgroup
( if ! echo $BASHPID > $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
then
echo $0: Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs - not executing submission.
SysFailure Could not add current process to $PC2_SANDBOX_CGROUP_PATH/cgroup.procs
exit $FAIL_SANDBOX_ERROR
fi
ulimit -t $TIMELIMIT -u $MAXPROCS -s unlimited
exec setsid taskset -c ${cpunum} $COMMAND $*
) <$INFIFO >$OUTFIFO 2>&2 &

setsid taskset $CPUMASK $COMMAND $* < $INFIFO > $OUTFIFO &
# Remember child's PID/PGRP for possible killing off later
submissionpid=$!

Expand Down Expand Up @@ -605,9 +622,11 @@ do
# If interactive validator finishes
if test "$child_pid" -eq "$intv_pid"
then
need_child_stats=0
REPORT_DEBUG Validator finishes with exit code $wstat
if test "$contestant_done" -eq 0
then
need_child_stats=1
# Added this test 04/30/2025 - apparently it was missing. We will only kill the submission if this
# flag is 1, as per the comment at the top of this script.
if test ${KILL_WA_VALIDATOR} -eq 0
Expand All @@ -617,8 +636,6 @@ do
wait -n "$submissionpid"
COMMAND_EXIT_CODE=$?

GetSandboxStats

if test $COMMAND_EXIT_CODE -eq 127
then
REPORT_DEBUG No more children found. Setting submission exit code to 0
Expand All @@ -627,13 +644,17 @@ do
FormatExitCode $COMMAND_EXIT_CODE
REPORT_DEBUG Contestant PID $submissionpid finished with exit code $result but after the validator
fi
ShowStats ${cputime} ${TIMELIMIT_US} ${walltime} ${peakmem} $((MEMLIMIT*1024*1024))
else
REPORT_DEBUG Killing child submission pid $submissionpid since it is still running "(KILL_WA_VALIDATOR=1)"
fi
fi

KillChildProcs
if test $need_child_stats -eq 1
then
GetSandboxStats
ShowStats ${cputime} ${TIMELIMIT_US} ${walltime} ${peakmem} $((MEMLIMIT*1024*1024))
fi

if test "$wstat" -eq $EXITCODE_AC
then
Expand Down Expand Up @@ -758,6 +779,9 @@ do
fi
done

TDEBUG echo -n 'CGroup2 cpu usec after all done: '
TDEBUG grep usage_usec $PC2_SANDBOX_CGROUP_PATH/cpu.stat

DEBUG echo
REPORT "________________________________________"

Expand Down
Loading