i_1280 Fix max_run_time by johnbrvc · Pull Request #1293 · pc2ccs/pc2v9

johnbrvc · 2026-08-27T02:16:54Z

Description of what the PR does

Keep track of the longest execution time in ExecutionData using a new member (maxExecuteTimeMS) along with the appropriate accessor and mutator. Previously, the executeTimeMS (getExecuteTimeMS()) member was used and this represents the execution time of the last test case. This caused the wrong value to appear on the CLICS event feed, as well as on the PC2 GUI that shows the execution data for a run.

Issue which the PR addresses

Fixes #1280

Environment in which the PR was developed (OS,IDE, Java version, etc.)

java version "1.8.0_321"
Java(TM) SE Runtime Environment (build 1.8.0_321-b07)
Java HotSpot(TM) 64-Bit Server VM (build 25.321-b07, mixed mode)

Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)

Load a clean copy of the clics_sumithello contest.
Bring up the administrator1 client.
Bring up the feeder1 client and start it (press Start button).
In command prompt window, issue the following curl command to read the event feed: curl -k https://administrator1:administrator1@localhost:50443/contests/SumH/event-feed
A bunch of events will scroll out and then it will sit there.
Back on the administrator1 client, start the contest (Times->Start All)
Bring up a team client (pc2team) and log in as team1/team1.
Make a submission for the Sumit problem (I used GNU C++ and isumit.cpp).
Open the pc2judge client using judge3 (it's set up to autojudge Sumit).
In the curl window, a bunch of events should have scrolled by. A bunch of "runs" followed by a "judgements". Look at the "max_run_time" property (it's the last one).
Scroll back through the runs (there are 7 of them) and look for the one with the biggest "run_time" property. The data for my run is shown below. The "max_run_time" property of the "judgements" notification should match the largest "run_time".
Back on the administrator1 client, go to the Run Contest->Runs tab.
Select the only run then press View Judgements at the bottom.
Look at the X time column. It should match the "max_run_time" property seen on the event feed.
You can make additional submissions if you like because the values will change slightly so you can see it may choose a different one as the one with the "max_run_time" next time.
You can ^C the curl to kill it.
Then close the judge, administrator and team clients.
Stop the contest (from the server, the Times tab, Stop All).
Close the server.
Done.

{"type":"runs","token":"pc2-49","id":"TestCase--5468749453065596807","data": {"id":"TestCase--5468749453065596807","judgement_id":"Run-7165368412488011938","ordinal":1,"judgement_type_id":"AC","time":"2026-08-26T22:06:03.981-04","contest_time":"00:05:11.792","run_time":0.263}}
{"type":"runs","token":"pc2-50","id":"TestCase--2183730955800265169","data": {"id":"TestCase--2183730955800265169","judgement_id":"Run-7165368412488011938","ordinal":2,"judgement_type_id":"AC","time":"2026-08-26T22:06:04.183-04","contest_time":"00:05:11.994","run_time":0.043}}
{"type":"runs","token":"pc2-51","id":"TestCase--926307959591635682","data": {"id":"TestCase--926307959591635682","judgement_id":"Run-7165368412488011938","ordinal":3,"judgement_type_id":"AC","time":"2026-08-26T22:06:04.375-04","contest_time":"00:05:12.186","run_time":0.036}}
{"type":"runs","token":"pc2-52","id":"TestCase--6239247762799142711","data": {"id":"TestCase--6239247762799142711","judgement_id":"Run-7165368412488011938","ordinal":4,"judgement_type_id":"AC","time":"2026-08-26T22:06:04.561-04","contest_time":"00:05:12.372","run_time":0.044}}
{"type":"runs","token":"pc2-53","id":"TestCase--425797505456334511","data": {"id":"TestCase--425797505456334511","judgement_id":"Run-7165368412488011938","ordinal":5,"judgement_type_id":"AC","time":"2026-08-26T22:06:04.736-04","contest_time":"00:05:12.547","run_time":0.03}}
{"type":"runs","token":"pc2-54","id":"TestCase--604826351888607225","data": {"id":"TestCase--604826351888607225","judgement_id":"Run-7165368412488011938","ordinal":6,"judgement_type_id":"AC","time":"2026-08-26T22:06:04.899-04","contest_time":"00:05:12.710","run_time":0.027}}
{"type":"runs","token":"pc2-55","id":"TestCase-1607340676828528961","data": {"id":"TestCase-1607340676828528961","judgement_id":"Run-7165368412488011938","ordinal":7,"judgement_type_id":"AC","time":"2026-08-26T22:06:05.070-04","contest_time":"00:05:12.881","run_time":0.027}}
{"type":"judgements","token":"pc2-56","id":"Run-7165368412488011938","data": {"id":"Run-7165368412488011938","submission_id":"1","judgement_type_id":"AC","start_time":"2026-08-26T22:05:59.750-04","start_contest_time":"00:05:07.721","end_time":"2026-08-26T22:06:05.189-04","end_contest_time":"00:05:13.160","max_run_time":0.263}}

Keep track of the longest execution time, as opposed to just the most recent one.

clevengr

Reviewed src/edu/csus/ecs/pc2/clics/API202306/JSON202306Utilities.java; left several comments.

clevengr · 2026-09-20T00:07:42Z

+     * event notification prefix.
+     * Adds event, (event) id, customProperty (an customValue) (if non-null) and data keyword to string.
+     *
+     * @param stringBuilder


This statement lists "stringBuilder" as a method parameter, but there's no such parameter actually declared in the method signature.

Copied from legacy comment in method above, which was not kept up-to-date. Fixed both the legacy comment above as well as this one. Next push.

clevengr · 2026-09-20T00:09:08Z

+     * Adds event, (event) id, customProperty (an customValue) (if non-null) and data keyword to string.
+     *
+     * @param stringBuilder
+     * @param eventType


There is no such parameter "eventType" in the method signature.

Same response. Legacy comment was copied, and legacy comment was wrong.

clevengr · 2026-09-20T00:10:32Z

+     *
+     * @param stringBuilder
+     * @param eventType
+     * @param data - json data for object


I would expect to see @param declarations for parameters eventSequence and id -- but there's no such statements here.

clevengr

Reviewed src/edu/csus/ecs/pc2/core/execute/ExecutionData.java; left one comment that I think is rather important.

clevengr · 2026-09-20T00:24:07Z


    public void setExecuteTimeMS(long inExecuteTime){
        executeTimeMS = inExecuteTime;
+        // Keep track of longest execution time


If I recall correctly how the code works, there is a separate ExecutionData object associated with the execution of each test case. If that's true, it doesn't seem logical to me to be storing the "longest execution time" in the ExecutionData object since each such object relates to (only) one test case execution. Rather, it seems like "longest execution time" should be stored in some higher-level object that associated with the entire sequence of test case executions. Am I mis-remembering how the code is structured?

Recall I wrote up a document explaining how ExecutionData is used. Here is a link: PC2 Execution Data Class
That should explain it. In a nutshell, the "per-test-case" execution information is only part of what is stored in ExecutionData. Compile results, and the final judgment for the Run is stored in there as well and examined by the caller to determine the disposition of the Run.

clevengr

Reviewed src/edu/csus/ecs/pc2/ui/AutoJudgingMonitor.java. See comments.

clevengr · 2026-09-20T00:28:04Z

            long totalSeconds = milliDiff / 1000;
            judgementRecord.setHowLongToJudgeInSeconds(totalSeconds);
-            judgementRecord.setExecuteMS(executeTimeMS);
+            judgementRecord.setExecuteMS(executionData.getMaxExecuteTimeMS());


The name of the setter now seems inappropriate; it's no longer setting an "execution time", it's setting a "maximum execution time over all the test cases".

The comment in JudgementRecord says, "Number of seconds it took to execute the run." Aside from the comment specifying the incorrect units (seconds, vs. MS), before this PR, that value was essentially meaningless (it was the execute time of the LAST test case). Now, it contains what I believe to be the original intent which is the time of the longest test case, since this value is used in the ViewJudgementsPane as the "X time", indicating (I would imagine) maximum execute time. What else could it possibly be? The "X time" column was added in 2011 by Doug:

bug 668: now sets the ms for the execute time into the system. Shows on reports, added column to View Judgement X time for execute time in ms. svn path=/trunk/; revision=2419

Keep in mind that JudgementRecord is different than ExecutionData. JudgementRecord is created from the information in ExecutionData. (ref. core.execute.JudgementUtilities.createJudgementRecord(...))

clevengr

Reviewed src/edu/csus/ecs/pc2/ui/SelectJudgementPaneNew.java; left minor comments.

clevengr · 2026-09-20T00:29:55Z

All the other changed files included updates to the copyright date; why not this one?

Fixed in next push.

clevengr

I reviewed all four changes files in the PR. I left individual review comments; a few of them are minor but a couple are things that I think either need to be fixed or I need to obtain a better understanding of how the code is working...

Proceeding to runtime testing of the PR...

clevengr · 2026-09-20T01:28:40Z

@johnbrvc : I loaded your branch for this PR, pulled to make sure it was up to date, built a new distribution from scratch in that branch, and ran the test steps listed in the PR. I got the following results on the Event Feed:


{"type":"submissions","token":"pc2-47","id":"1","data": {"id":"1","language_id":"cpp","problem_id":"sumit","team_id":"1","time":"2026-09-19T18:15:02.576-07","contest_time":"00:02:57.531","entry_point":null,"files":[{"href":"/contests/SumH/submissions/1/files","filename":"1.zip","hash":null,"mime":"application/zip"}]}}
{"type":"judgements","token":"pc2-48","id":"Run-7331534654699995379","data": {"id":"Run-7331534654699995379","submission_id":"1","start_time":"2026-09-19T18:15:31.582-07","start_contest_time":"00:03:26.509"}}
{"type":"runs","token":"pc2-49","id":"TestCase--6825957412722139346","data": {"id":"TestCase--6825957412722139346","judgement_id":"Run-7331534654699995379","ordinal":1,"judgement_type_id":"AC","time":"2026-09-19T18:15:33.116-07","contest_time":"00:03:27.945","run_time":0.113}}
{"type":"runs","token":"pc2-50","id":"TestCase--2184973950372930468","data": {"id":"TestCase--2184973950372930468","judgement_id":"Run-7331534654699995379","ordinal":2,"judgement_type_id":"AC","time":"2026-09-19T18:15:33.403-07","contest_time":"00:03:28.232","run_time":0.044}}
{"type":"runs","token":"pc2-51","id":"TestCase-4984028280940217660","data": {"id":"TestCase-4984028280940217660","judgement_id":"Run-7331534654699995379","ordinal":3,"judgement_type_id":"AC","time":"2026-09-19T18:15:33.720-07","contest_time":"00:03:28.549","run_time":0.053}}
{"type":"runs","token":"pc2-52","id":"TestCase--2673449443172590504","data": {"id":"TestCase--2673449443172590504","judgement_id":"Run-7331534654699995379","ordinal":4,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.019-07","contest_time":"00:03:28.848","run_time":0.059}}
{"type":"runs","token":"pc2-53","id":"TestCase-6159867825678113113","data": {"id":"TestCase-6159867825678113113","judgement_id":"Run-7331534654699995379","ordinal":5,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.314-07","contest_time":"00:03:29.143","run_time":0.043}}
{"type":"runs","token":"pc2-54","id":"TestCase-8218035131030148505","data": {"id":"TestCase-8218035131030148505","judgement_id":"Run-7331534654699995379","ordinal":6,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.618-07","contest_time":"00:03:29.447","run_time":0.052}}
{"type":"runs","token":"pc2-55","id":"TestCase-2274113654676303350","data": {"id":"TestCase-2274113654676303350","judgement_id":"Run-7331534654699995379","ordinal":7,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.902-07","contest_time":"00:03:29.731","run_time":0.046}}
{"type":"judgements","token":"pc2-56","id":"Run-7331534654699995379","data": {"id":"Run-7331534654699995379","submission_id":"1","judgement_type_id":"AC","start_time":"2026-09-19T18:15:31.582-07","start_contest_time":"00:03:26.509","end_time":"2026-09-19T18:15:35.177-07","end_contest_time":"00:03:30.104","max_run_time":0.046}}

As you can see, the "max_run_time" in the judgement is 0.046, but there are several "run_time" entries that are larger than that. I'm not sure what to make of this. (I also looked at the "Run Judgements" pane in the Admin's RunContest>Runs pane as suggested in the PR; it also says "X time = 46ms".)

Any suggestions as to what may be going wrong? Could this perhaps be related to the comments I made while reviewing the source -- specifically, about seeming to recall that there is a different ExecutionData object used for each test case? It seems like the results I'm seeing give the "max runtime" as the same as the "last executed test case"...

johnbrvc · 2026-09-20T13:42:21Z

@johnbrvc : I loaded your branch for this PR, pulled to make sure it was up to date, built a new distribution from scratch in that branch, and ran the test steps listed in the PR. I got the following results on the Event Feed:


{"type":"submissions","token":"pc2-47","id":"1","data": {"id":"1","language_id":"cpp","problem_id":"sumit","team_id":"1","time":"2026-09-19T18:15:02.576-07","contest_time":"00:02:57.531","entry_point":null,"files":[{"href":"/contests/SumH/submissions/1/files","filename":"1.zip","hash":null,"mime":"application/zip"}]}}
{"type":"judgements","token":"pc2-48","id":"Run-7331534654699995379","data": {"id":"Run-7331534654699995379","submission_id":"1","start_time":"2026-09-19T18:15:31.582-07","start_contest_time":"00:03:26.509"}}
{"type":"runs","token":"pc2-49","id":"TestCase--6825957412722139346","data": {"id":"TestCase--6825957412722139346","judgement_id":"Run-7331534654699995379","ordinal":1,"judgement_type_id":"AC","time":"2026-09-19T18:15:33.116-07","contest_time":"00:03:27.945","run_time":0.113}}
{"type":"runs","token":"pc2-50","id":"TestCase--2184973950372930468","data": {"id":"TestCase--2184973950372930468","judgement_id":"Run-7331534654699995379","ordinal":2,"judgement_type_id":"AC","time":"2026-09-19T18:15:33.403-07","contest_time":"00:03:28.232","run_time":0.044}}
{"type":"runs","token":"pc2-51","id":"TestCase-4984028280940217660","data": {"id":"TestCase-4984028280940217660","judgement_id":"Run-7331534654699995379","ordinal":3,"judgement_type_id":"AC","time":"2026-09-19T18:15:33.720-07","contest_time":"00:03:28.549","run_time":0.053}}
{"type":"runs","token":"pc2-52","id":"TestCase--2673449443172590504","data": {"id":"TestCase--2673449443172590504","judgement_id":"Run-7331534654699995379","ordinal":4,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.019-07","contest_time":"00:03:28.848","run_time":0.059}}
{"type":"runs","token":"pc2-53","id":"TestCase-6159867825678113113","data": {"id":"TestCase-6159867825678113113","judgement_id":"Run-7331534654699995379","ordinal":5,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.314-07","contest_time":"00:03:29.143","run_time":0.043}}
{"type":"runs","token":"pc2-54","id":"TestCase-8218035131030148505","data": {"id":"TestCase-8218035131030148505","judgement_id":"Run-7331534654699995379","ordinal":6,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.618-07","contest_time":"00:03:29.447","run_time":0.052}}
{"type":"runs","token":"pc2-55","id":"TestCase-2274113654676303350","data": {"id":"TestCase-2274113654676303350","judgement_id":"Run-7331534654699995379","ordinal":7,"judgement_type_id":"AC","time":"2026-09-19T18:15:34.902-07","contest_time":"00:03:29.731","run_time":0.046}}
{"type":"judgements","token":"pc2-56","id":"Run-7331534654699995379","data": {"id":"Run-7331534654699995379","submission_id":"1","judgement_type_id":"AC","start_time":"2026-09-19T18:15:31.582-07","start_contest_time":"00:03:26.509","end_time":"2026-09-19T18:15:35.177-07","end_contest_time":"00:03:30.104","max_run_time":0.046}}

As you can see, the "max_run_time" in the judgement is 0.046, but there are several "run_time" entries that are larger than that. I'm not sure what to make of this. (I also looked at the "Run Judgements" pane in the Admin's RunContest>Runs pane as suggested in the PR; it also says "X time = 46ms".)

Any suggestions as to what may be going wrong? Could this perhaps be related to the comments I made while reviewing the source -- specifically, about seeming to recall that there is a different ExecutionData object used for each test case? It seems like the results I'm seeing give the "max runtime" as the same as the "last executed test case"...

This really looks like you're not using the correct version of PC2 - it seems like it is doing what the non-PR version does. Are you running this from the debugger in Eclipse or from the command line? If the latter, check pc2ver to make sure it's running the right version. (Maybe try: where pc2judge.bat)

I just repeatedly made several submissions and they all worked as expected.

Fixed some legacy comments that specified incorrect parameters. Fixed wrong calling sequence for added method (left off the new parameters).

clevengr

I'm satisfied with the updates to the source. I still am having trouble getting the runtime tests to work correctly.

clevengr

I was finally able to resolve the issues that were not allowing me to produce the expected results when running the PR. For future reviewers, here's a potential issue to be aware of:

JohnB's GitHub fork (which I cloned and worked on locally on my Windows machine) has (or at least, had at the time I was testing) two separate but similarly-named branches: i1280_max_run_time_fix and i1280_fix_max_run_time. The former was apparently an old version which shouldn't be used (probably should be deleted) -- but it was the one I had selected for testing. It doesn't work; it doesn't contain the fixes implied by this PR. You should make sure that any testing you do uses the newer one: i1280_fix_max_run_time. That branch worked for me.

I approve the PR.

i_1280 Fix max_run_time

8030192

Keep track of the longest execution time, as opposed to just the most recent one.

johnbrvc added this to the 9.11.0 milestone Aug 27, 2026

johnbrvc requested review from JoeTerlizzi, SamanwaySadhu, clevengr and kkarakas August 27, 2026 02:16

johnbrvc self-assigned this Aug 27, 2026

clevengr requested changes Sep 20, 2026

View reviewed changes

i_1280 PR change requests. Fix comments and calling sequence

082d5f8

Fixed some legacy comments that specified incorrect parameters. Fixed wrong calling sequence for added method (left off the new parameters).

johnbrvc requested a review from clevengr September 20, 2026 13:44

clevengr reviewed Sep 20, 2026

View reviewed changes

clevengr approved these changes Sep 21, 2026

View reviewed changes

Conversation

johnbrvc commented Aug 27, 2026

Description of what the PR does

Issue which the PR addresses

Environment in which the PR was developed (OS,IDE, Java version, etc.)

Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)

Uh oh!

clevengr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clevengr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clevengr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clevengr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

clevengr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clevengr commented Sep 20, 2026

Uh oh!

johnbrvc commented Sep 20, 2026

Uh oh!

clevengr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clevengr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clevengr left a comment •

edited

Loading

clevengr left a comment •

edited

Loading