Skip to content

feature(ActionTimeout): add garbage collection time tracking#418

Open
maxblan wants to merge 3 commits into
mainfrom
feature/action-timeout
Open

feature(ActionTimeout): add garbage collection time tracking#418
maxblan wants to merge 3 commits into
mainfrom
feature/action-timeout

Conversation

@maxblan

@maxblan maxblan commented Apr 26, 2023

Copy link
Copy Markdown

This commit adds a new variable gcTime to the ActionTimeout class and subtracts it from the total time when calculating the time difference between the start and stop timestamps. The gcTime variable is calculated by iterating over the list of Garbage Collector MXBeans and summing up the collection times. This change improves the accuracy of the time tracking by accounting for the time spent in garbage collection.

These changes has not been tested, but are more a idea to be presended.

…nTimeout class

This commit adds a new variable gcTime to the ActionTimeout class and subtracts it from the total time when calculating the time difference between the start and stop timestamps. The gcTime variable is calculated by iterating over the list of Garbage Collector MXBeans and summing up the collection times. This change improves the accuracy of the time tracking by accounting for the time spent in garbage collection.
@maxblan
maxblan requested a review from xeruf as a code owner April 26, 2023 21:44
@xeruf

xeruf commented Apr 30, 2023

Copy link
Copy Markdown
Member

Did not expect that this is even possible, interesting!
But I think we have to wait for next season so we have sufficient time for testing in practice.

@xeruf xeruf added this to the 24 milestone Apr 30, 2023
@xeruf xeruf modified the milestones: 24, long-term Mar 21, 2024
@soerendomroes

Copy link
Copy Markdown
Member

@xeruf This seems to be interesting did you find time to test this in practice? How do you plan to test this in practice?

@xeruf

xeruf commented Feb 21, 2026

Copy link
Copy Markdown
Member

testing it locally on cli and gui, on staging, in friendly encounters on the live system and on the probespieltage - basically we can merge this in the 2027 next branch, I just want to give it time to mature and find any issues

@xeruf

xeruf commented Jul 16, 2026

Copy link
Copy Markdown
Member

@soerendomroes I think it is the right time now to merge this

@soerendomroes

Copy link
Copy Markdown
Member

@xeruf how did you test this? Could you commit your test here?

Comment thread sdk/src/main/server-api/sc/framework/plugins/ActionTimeout.java Outdated
Comment thread sdk/src/main/server-api/sc/framework/plugins/ActionTimeout.java
@soerendomroes

Copy link
Copy Markdown
Member

Hi, I logged the values , and the gcTime is never reset.

this(canTimeout, DEFAULT_HARD_TIMEOUT, DEFAULT_SOFT_TIMEOUT);
}
private static final int DEFAULT_HARD_TIMEOUT = 10000;
private static final int DEFAULT_SOFT_TIMEOUT = 5000;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xeruf is there a reason that the default soft-timeout is 5 seconds instead of 2?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it such that the advanced player cannot run into a timeout? Since it may take more than 2 seconds in the current implementation? software-challenge/gui#121

@soerendomroes
soerendomroes requested a review from xeruf July 21, 2026 14:54
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.

3 participants