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

Disable sync reports when we know we won't need them. #15593

Merged
merged 1 commit into from Sep 10, 2018

Conversation

Projects
None yet
3 participants
@pchote
Copy link
Member

pchote commented Sep 7, 2018

See commit message for details.

Desyncs in MP are now very rare, so I plan on disabling these by default in MP too after implementing #4084 and adding a server flag so that we can turn them back on without shipping a new release if any desyncs return in the future.

@pchote pchote added this to the Next + 1 milestone Sep 7, 2018

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Sep 7, 2018

For context, the regular large spikes in the raspberry pi perf are caused entirely by the report generation.

Turning the reports off gets us up to 30FPS.

Disable sync reports when we know we won't need them.
Generating the sync report takes ~twice as long as
a normal tick, and occurs once every 3 ticks.

These reports record of all of the synced state
(separate to the sync hash, which is still calculated)
in order to generate the syncreport.log of the game
desyncs. This perf overhead is completely unnecessary
when we know that we won't have other syncreports to
compare against (singleplayer, replays).

Disabling report generation in these cases gives
us an easy 40% average tick-time win.

@pchote pchote force-pushed the pchote:optional-sync-report branch from ea88a65 to dc6d4d6 Sep 7, 2018

@RoosterDragon

This comment has been minimized.

Copy link
Member

RoosterDragon commented Sep 7, 2018

Thought for if you do disable reports by default for MP: If DumpSyncReport doesn't have the a report for the frame in question. Is it worth generating a report for the current frame and dumping that (with a warning in the report to indicate as such) just in case the reports from several players happen to generate for the same frame? I'm wondering if that might prove useful on occasion even if per-frame reports are turned off.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Sep 7, 2018

Good idea

@RoosterDragon

This comment has been minimized.

Copy link
Member

RoosterDragon commented Sep 8, 2018

👍

@RoosterDragon

This comment has been minimized.

Copy link
Member

RoosterDragon commented Sep 8, 2018

Two tangential thoughts:

  • Why does the SyncReport class maintain the last 5 reports? If everything is in lockstep wouldn't the last report be sufficient?
  • If we do change reports to not run every frame, we should remove the if (hash != 0) checks in GenerateSyncReport. These make perf even worse (hence why I don't suggest this now) but fix a correctness issue.
@pchote

This comment has been minimized.

Copy link
Member

pchote commented Sep 8, 2018

  • Why does the SyncReport class maintain the last 5 reports? If everything is in lockstep wouldn't the last report be sufficient?

We are not quite lockstep, but instead transfer/simulate ticks in groups of three. In a 3p game you may therefore have two players who agree having simulated to N+1 before they realize that the other player desynced on tick N.

I don't know why we made this choice, hopefully @chrisforbes can chime in here. If we can eliminate the aggregate netticks then we would be able to eliminate this completely and generate the reports only on an actual desync.

@abcdefg30 abcdefg30 merged commit 8533aa8 into OpenRA:bleed Sep 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Sep 10, 2018

@pchote pchote deleted the pchote:optional-sync-report branch Nov 18, 2018

@pchote pchote referenced this pull request Jan 4, 2019

Open

Save Game Support #2743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment