Skip to content
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

DEV9: Improve speed of TCP connections on the sockets backend #11046

Merged
merged 5 commits into from
Apr 8, 2024

Conversation

TheLastRar
Copy link
Contributor

Description of Changes

Increases the number of sent packets that are yet to be acknowledged (from 1 to 64).
Consider the amount of outstanding, yet to be acknowledged, data when determining the max packet size
Fixes incorrect code for handling sequence overflow
Corrects some logs

Rationale behind Changes

The code was over cautious, and was bottlenecking TCP connections, we need more speed
Fixes a (probably very rare?) bug

Suggested Testing Steps

Test networking games that use TCP (mostly server based games)

@github-actions github-actions bot added the DEV9 label Apr 6, 2024
@TheLastRar TheLastRar force-pushed the DEV9-SocketsNoAckCheck branch from 36958d3 to 058b511 Compare April 6, 2024 21:42
@TheLastRar
Copy link
Contributor Author

I'm not sure why the Clang build failed, It built correctly on my branch

@AmyRoxwell
Copy link

Doesn't seem to do anything for Monster Hunter 2 Dos, spits same information after coming from a quest with 3+ players:

Master:
Screenshot_04_06_2024-22
Emulog: (not full due to me forgetting to get it oops)
Screenshot_04_06_2024-21

Pr:
Screenshot_04_06_2024-25
Emulog:
emulog.zip

@TheLastRar TheLastRar force-pushed the DEV9-SocketsNoAckCheck branch from 058b511 to e16949d Compare April 7, 2024 14:42
@TheLastRar TheLastRar force-pushed the DEV9-SocketsNoAckCheck branch from e16949d to 6fe2cd3 Compare April 7, 2024 15:10
@TheLastRar
Copy link
Contributor Author

Doesn't seem to do anything for Monster Hunter 2 Dos, spits same information after coming from a quest with 3+ players:

I think I've figured out this issue, it's caused by the following;

  1. unsigned long is 32bits on Windows and (typically) 64bits on linux
  2. The log message used %d instead of %lu, which expects int, and ignores the upper 32bits of 64bit types
  3. FIONREAD, the ioctl, used to get the amount of available data, is unsigned long on windows, but int on linux
  4. The available variable wasn't initialised.

On windows, everything is the same size (32bits) and works as expected

On Linux;
available is created with 64bits of uninitialised data
FIONREAD writes 32bits
The condition to print the log reads the full 64bits, and because of the unwritten upper 32bits of available, ends up (sometimes) passing
The log message only reads 32bits, and prints Got a lot of data: 0 (or however much actually is available).

These small differences between platforms are annoying to deal with.

I've opted to zero initialise available, which should fix the issue, the variable is only used for logging, and you've confirmed the game works fine, so the only issue was the incorrect log spam.

Can you test the latest build on this pr to confirm it works

@AmyRoxwell
Copy link

Doesn't seem to do anything for Monster Hunter 2 Dos, spits same information after coming from a quest with 3+ players:

I think I've figured out this issue, it's caused by the following;

1. `unsigned long` is 32bits on Windows and (typically) 64bits on linux

2. The log message used `%d` instead of `%lu`, which expects int, and ignores the upper 32bits of 64bit types

3. `FIONREAD`, the `ioctl`, used to get the amount of available data, is `unsigned long` on windows, but `int` on linux

4. The `available` variable wasn't initialised.

On windows, everything is the same size (32bits) and works as expected

On Linux; available is created with 64bits of uninitialised data FIONREAD writes 32bits The condition to print the log reads the full 64bits, and because of the unwritten upper 32bits of available, ends up (sometimes) passing The log message only reads 32bits, and prints Got a lot of data: 0 (or however much actually is available).

These small differences between platforms are annoying to deal with.

I've opted to zero initialise available, which should fix the issue, the variable is only used for logging, and you've confirmed the game works fine, so the only issue was the incorrect log spam.

Can you test the latest build on this pr to confirm it works

Works like a charm, would need to test more but no longer spamming :D

new pr:
Screenshot_04_07_2024-10
Emulog:
emulog.zip

Copy link
Member

@F0bes F0bes left a comment

Choose a reason for hiding this comment

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

Works fine from my testing. It is a lot faster now :^)

@lightningterror lightningterror merged commit 52ddb0e into PCSX2:master Apr 8, 2024
12 checks passed
@TheLastRar TheLastRar deleted the DEV9-SocketsNoAckCheck branch April 8, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants