Compare timebase of netplay users to detect desyncs. #2178

RachelBryk · 2015-03-07T08:38:50Z

No description provided.

Source/Core/Core/NetPlayClient.cpp

+	if (isPlayer)
+	{
+		bool bWii = SConfig::GetInstance().m_LocalCoreStartupParameter.bWii;
+		u64 hash = XXH64(Memory::GetPointer(bWii ? 0x90000000 : 0x80000000), bWii ? 0x4000000 : 0x2000000, 0);


Source/Core/Core/NetPlayServer.cpp

+			int pid = -1;
+			if (s_hashes[frame].size() > 2)
+			{
+				for (int i = 0; i < s_hashes[frame].size(); ++i)


delroth · 2015-03-08T04:54:40Z

Aka. killing caches for sadness and negative profit.

Source/Core/Core/NetPlayServer.cpp

+		hash.first = x | ((u64)y >> 32);
+		hash.second = player.pid;
+
+		s_hashes[frame].push_back(hash);


ghost · 2015-03-09T11:36:43Z

This idea seems really excessive. Can't you just compare timebases of the machines when swapping input, since practically any divergence in the code being executed will change the number of instructions having been run?

RachelBryk · 2015-03-09T11:39:36Z

Nope, tried that before, it is useless.

ghost · 2015-03-09T12:01:03Z

Care to explain why? It seems a bit strange that two machines executing different code paths would have the exact same instruction count at consecutive intervals.

RachelBryk · 2015-03-09T12:03:40Z

idk, but i've tested it, it is not reliable.

ghost · 2015-03-09T12:33:11Z

Sounds like your testing was unreliable if you can't explain the results. The timebase I'm referring to is what SystemTimers::GetFakeTimeBase() returns, which is the same as the CPU's timebase registers. Not CoreTiming::GetTicks() and certainly not CEXIIPL::GetGCTime().

You're testing for differences in RAM (in a really slow and ugly way, see Delroth's comment): those differences can't just spontaneously happen (except for maybe differences in the ISO, but that definitely should not be solved in this way), there has to be instructions being executed on one machine and not the other that causes them.

RachelBryk · 2015-03-09T12:44:21Z

Oh, okay, that seems to work.

comex · 2015-03-10T17:34:06Z

@tueidj But GetFakeTimeBase is just a wrapper around GetTicks..?

Source/Core/Core/NetPlayServer.cpp

+			int pid = -1;
+			if (s_timebase[frame].size() > 2)
+			{
+				for (auto hash : s_timebase[frame])


ghost · 2015-03-10T18:20:45Z

@comex Not just a wrapper, it's GetTicks + an offset that can be modified by emulated code. Although it's unlikely for any game to modify the timebase registers, there's no reason to pretend it couldn't happen.

Source/Core/Core/NetPlayServer.cpp

@@ -584,6 +586,51 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player)
 	}
 	break;

+	case NP_MSG_TIMEBASE:
+	{
+		u32 x,y, frame;


phire · 2015-03-11T02:57:23Z

I'm not sure checking the timebase once a frame is the best way to catch desyncs.
Certainly faster than hashing RAM, but I worry that if the de-sync is exactly a multiple of the idle loop execution cycles then you could have a false negative, with one client executing a few extra idle loops.

With the standard 3 instruction idle loop (which I think dolphin executes in 3 cycles), that's a 33% chance of a false negative.

Perhaps you should stop the cpu thread 10 to 100 times a frame and check if the program counters match, maybe hashing the other registers too.

ghost · 2015-03-11T03:36:21Z

If the clients are only desynced by a couple of idle loops is it really going to be a problem if it's not detected? There would likely have to be a greater divergence in execution paths before problems started to manifest, which would cause more significant differences in the timebases.

phire · 2015-03-11T03:41:23Z

It's modulus. One client could be executing 3, 300, 3000 or 3 million extra cycles, as long as it's a modulus of the length of the idle loop.

It might be a non-issue, probability is that you will detect the desync within 3 frames.

RachelBryk · 2015-03-11T04:04:36Z

Yeah, i do not think it is an issue, it should still always detect desyncs long before the players notice. Testing it by changing settings always caught it instantly.

RachelBryk · 2015-03-13T06:57:19Z

If there's no more issues, i think this should be good to merge.

Source/Core/Core/NetPlayClient.cpp

@@ -514,7 +533,7 @@ void NetPlayClient::ThreadFunc()
 				break;
 			}
 		}
-
+		Common::SleepCurrentThread(1);


RachelBryk · 2015-03-18T12:31:39Z

Rebased due to conflicts.

RachelBryk · 2015-03-29T19:34:02Z

Anything stopping this from being merged?

JMC47 · 2015-03-29T19:51:17Z

The only thing I can think of is if it works for everything now. I know it doesn't work on Dualcore at all with this method (instant desync) but single core should work without a performance down afaict so far.

JMC47 · 2015-04-22T03:10:53Z

Is this working? I tried testing the latest build of it, but even when I have obviously wrong settings, it still doesn't trigger. Ping and whatnot.

RachelBryk · 2015-04-22T03:20:19Z

As far as I know?

On 2015/04/21, at 23:10, JMC47 notifications@github.com wrote:

Is this working? I tried testing the latest build of it, but even when I have obviously wrong settings, it still doesn't trigger. Ping and whatnot.

―
Reply to this email directly or view it on GitHub.

JMC47 · 2015-04-22T03:25:14Z

I can't get desyncs to trigger yet no matter what I do. I made sure the setting was enabled in my INI.

RachelBryk · 2015-04-22T03:49:44Z

What settings? Is net play overriding it?

On 2015/04/21, at 23:25, JMC47 notifications@github.com wrote:

I can't get desyncs to trigger yet no matter what I do. I made sure the setting was enabled in my INI.

―
Reply to this email directly or view it on GitHub.

JMC47 · 2015-04-22T03:51:09Z

I used different INIs with different cheats, I manually overrode the CPU clock, I used different save files and nothing produced a message.

RachelBryk · 2015-04-22T04:01:39Z

Okay I'll look at it when I get home tomorrow

On 2015/04/21, at 23:51, JMC47 notifications@github.com wrote:

I used different INIs with different cheats, I manually overrode the CPU clock, I used different save files and nothing produced a message.

―
Reply to this email directly or view it on GitHub.

RachelBryk force-pushed the netplay-hash branch from 71f3a2f to 2e58850 Compare March 7, 2015 08:42

magumagu reviewed Mar 7, 2015
View reviewed changes

RachelBryk force-pushed the netplay-hash branch from 2e58850 to fbbe5d6 Compare March 8, 2015 02:44

lioncash reviewed Mar 8, 2015
View reviewed changes

Source/Core/Core/NetPlayServer.cpp

int pid = -1;

if (s_hashes[frame].size() > 2)

{

for (int i = 0; i < s_hashes[frame].size(); ++i)

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

RachelBryk force-pushed the netplay-hash branch from fbbe5d6 to 9f92ec2 Compare March 8, 2015 03:46

RachelBryk force-pushed the netplay-hash branch from 9f92ec2 to 997625b Compare March 8, 2015 10:54

lioncash reviewed Mar 8, 2015
View reviewed changes

Source/Core/Core/NetPlayServer.cpp

hash.first = x | ((u64)y >> 32);

hash.second = player.pid;

s_hashes[frame].push_back(hash);

This comment was marked as off-topic.

Sign in to view

RachelBryk force-pushed the netplay-hash branch from 997625b to e4e7fa2 Compare March 8, 2015 13:51

RachelBryk force-pushed the netplay-hash branch from e4e7fa2 to 7f37e72 Compare March 10, 2015 08:32

RachelBryk changed the title ~~Hash ram and compare to check for desyncs in netplay.~~ Compare timebase of netplay users to detect desyncs. Mar 10, 2015

RachelBryk force-pushed the netplay-hash branch from 7f37e72 to 3ff5faa Compare March 10, 2015 08:34

comex reviewed Mar 10, 2015
View reviewed changes

Source/Core/Core/NetPlayServer.cpp

int pid = -1;

if (s_timebase[frame].size() > 2)

{

for (auto hash : s_timebase[frame])

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

lioncash reviewed Mar 10, 2015
View reviewed changes

Source/Core/Core/NetPlayServer.cpp

@@ -584,6 +586,51 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player)

}

break;

case NP_MSG_TIMEBASE:

{

u32 x,y, frame;

This comment was marked as off-topic.

Sign in to view

RachelBryk force-pushed the netplay-hash branch from 3ff5faa to 2a343ce Compare March 11, 2015 02:40

RachelBryk force-pushed the netplay-hash branch 2 times, most recently from 7887a5a to f91f333 Compare March 11, 2015 06:50

degasus reviewed Mar 13, 2015
View reviewed changes

Source/Core/Core/NetPlayClient.cpp

@@ -514,7 +533,7 @@ void NetPlayClient::ThreadFunc()

break;

}

}

Common::SleepCurrentThread(1);

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

Compare timebase of netplay users to detect desyncs.

48a5991

RachelBryk force-pushed the netplay-hash branch from f91f333 to 48a5991 Compare March 18, 2015 12:29

comex closed this Jun 4, 2015

comex mentioned this pull request Jun 4, 2015

[rebased] Compare timebase of netplay users to detect desyncs. #2517

Merged

Compare timebase of netplay users to detect desyncs. #2178

Compare timebase of netplay users to detect desyncs. #2178

Conversation

RachelBryk commented Mar 7, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth commented Mar 8, 2015

This comment was marked as off-topic.

ghost commented Mar 9, 2015

RachelBryk commented Mar 9, 2015

ghost commented Mar 9, 2015

RachelBryk commented Mar 9, 2015

ghost commented Mar 9, 2015

RachelBryk commented Mar 9, 2015

comex commented Mar 10, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

ghost commented Mar 10, 2015

This comment was marked as off-topic.

phire commented Mar 11, 2015

ghost commented Mar 11, 2015

phire commented Mar 11, 2015

RachelBryk commented Mar 11, 2015

RachelBryk commented Mar 13, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

RachelBryk commented Mar 18, 2015

RachelBryk commented Mar 29, 2015

JMC47 commented Mar 29, 2015

JMC47 commented Apr 22, 2015

RachelBryk commented Apr 22, 2015

JMC47 commented Apr 22, 2015

RachelBryk commented Apr 22, 2015

JMC47 commented Apr 22, 2015

RachelBryk commented Apr 22, 2015