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

Remove I/O on thread setting #14999

Merged
merged 5 commits into from
Oct 19, 2021
Merged

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented Oct 9, 2021

We removed this a year ago, but it was put back because of something related to adhoc - see #13096. I think that's no longer necessary, and this setting being off can actually cause crashes. It's marked experimental, which might lead to some people disabling it.

This will remove the setting entirely.

-[Unknown]

@unknownbrackets unknownbrackets added this to the Future-Prio milestone Oct 9, 2021
@anr2me
Copy link
Collaborator

anr2me commented Oct 9, 2021

Hmm.. i don't remembered some adhoc issue need to have I/O on thread to be disabled, the last time i remembered AdhocMatching callback could be affected by I/O Timing Methods, usually changing the i/o timing method could solve this.
This is probably because we don't have an accurate timings on adhoc syscalls, thus sometimes a game's network thread overlap with another thread at a bad/critical time (ie. the game only waited for a short time) and when another thread took control for quite long, at the time the control switched back to the network thread the game may already consider it as timeout.

Btw, is there a reason why the default I/O Timings Method is not "Simulate UMD Delays"?
It seems to have a higher compatibility and able solves issue on some games
Does it cause issue on some other games or was it just for performance reason?

Edit: nevermind, based on this #7647 it seems to have issue on a few titles, not sure whether it's still happening by now or not.

@unknownbrackets
Copy link
Collaborator Author

Ideally, simulate UMD delays should be fixed to more correctly estimate UMD delays, and possibly should take into account UMD cache (which is faster than "Fast".) Then it should be enabled for UMD games only (i.e. not NP.)

Right now the setting is pretty inaccurate, and I'll be honest - I kinda have no idea how to correctly model it, which is why I haven't made any improvements to it. All it does right now is try to introduce a seek delay if two reads are far apart on the ISO.

It's unrelated to I/O on thread.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Oct 9, 2021

Should we change the ini name of the setting, so it gets reset to default for everybody? Otherwise, people might be stuck on the "wrong" value and unable to change it without modifying the ini.

@hrydgard hrydgard modified the milestones: Future-Prio, v1.13 Oct 9, 2021
@unknownbrackets
Copy link
Collaborator Author

Well, I was thinking of just removing it in a follow up change. I could rename it, but this is also a reported setting and I'm doing things to look at the results per that setting so I'd rather not have the name change. But if we're going to not remove it, I'll probably want to change the name indeed.

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Oct 9, 2021

Alright, then we'll just go ahead and remove it either in this PR or soon after. The intermediate state doesn't sound very useful, except maybe for reporting for a bit then as you say.

@hrydgard hrydgard added the I/O Affected by I/O timing settings, or other kind of I/O issue. label Oct 9, 2021
@unknownbrackets unknownbrackets changed the title Remove I/O on thread setting from UI Remove I/O on thread setting Oct 9, 2021
@hrydgard
Copy link
Owner

Tried using the web editor to resolve the tr_TR.ini conflict - not sure how I'm allowed to push to your branch, but hey.

@hrydgard hrydgard merged commit 3372f39 into hrydgard:master Oct 19, 2021
@unknownbrackets
Copy link
Collaborator Author

When you open a pull on another person's repo, there's a checkbox to allow editing of that branch by other collaborators.

-[Unknown]

@unknownbrackets unknownbrackets deleted the config-io-thread branch October 19, 2021 14:05
@ghost ghost mentioned this pull request Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I/O Affected by I/O timing settings, or other kind of I/O issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants