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

fs_user: Add a delay for each file open #4396

Merged
merged 3 commits into from Mar 23, 2019

Conversation

FearlessTobi
Copy link
Contributor

@FearlessTobi FearlessTobi commented Nov 2, 2018

File IOs on a 3DS take much longer then our currently emulated 39000 ns for each SVC call. Without the additional delay added by that PR the thread opening a file is executed much faster then it should, leading e.g. to deadlocks.

The time it took for the opening of files were tested with homebrews (links to their source code can be found in the code). The delay values were taken from the averages of all FileOpen operations for the specified archive type.
For now RomFS, ExeFS, and reading savefiles were tested. For all other values for now the RomFS/ExeFS delay was assumed until they get measured to. Adding that delay instead of the 39000 ns seems to be at least a bit more precise.

The final goal is still to measure all delays for File IOs (Read, FileCreate, FileDelete, DirCreate, DirDelete, etc... ) and to add them to Citra.

For this PR, I made ArchiveBackend also able to hold the delay generator. I saw no other possibility than this if I wanted to add accurate delays for when file opening fails.

This fixes #2446.


This change is Reviewable

@FearlessTobi FearlessTobi requested review from B3n30 and wwylele and removed request for B3n30 Nov 2, 2018
Copy link
Member

@wwylele wwylele left a comment

FS_USER::OpenFileDirectly should also have the delay

src/core/file_sys/archive_backend.h Outdated Show resolved Hide resolved
src/core/hle/service/fs/fs_user.cpp Outdated Show resolved Hide resolved
src/core/hle/service/fs/fs_user.cpp Outdated Show resolved Hide resolved
src/core/hle/service/fs/fs_user.cpp Outdated Show resolved Hide resolved
src/core/file_sys/archive_backend.h Outdated Show resolved Hide resolved
@FearlessTobi
Copy link
Contributor Author

FearlessTobi commented Nov 3, 2018

Amended.

@ghost
Copy link

ghost commented Nov 3, 2018

SMM only works without save data created, doesn't work with save data created.

@FearlessTobi
Copy link
Contributor Author

FearlessTobi commented Nov 3, 2018

I can reproduce this. Need to investigate ._.

@ghost
Copy link

ghost commented Nov 3, 2018

@FearlessTobi with 758199 ns for ExtSaveData open, it works with and without save data created.

src/core/hle/service/fs/archive.h Outdated Show resolved Hide resolved
@@ -56,7 +57,7 @@ void FS_USER::OpenFile(Kernel::HLERequestContext& ctx) {

LOG_DEBUG(Service_FS, "path={}, mode={} attrs={}", file_path.DebugStr(), mode.hex, attributes);

ResultVal<std::shared_ptr<File>> file_res =
const auto [file_res, open_timeout_ns] =
archives.OpenFileFromArchive(archive_handle, file_path, mode);
IPC::RequestBuilder rb = rp.MakeBuilder(1, 2);
Copy link
Contributor

@B3n30 B3n30 Nov 4, 2018

Choose a reason for hiding this comment

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

Shouldn't the IPC result be built inside the lambda? Maybe that was also a mistake in my delay PR as well and thus you copy/pasted that mistake

Copy link
Member

@wwylele wwylele Nov 4, 2018

Choose a reason for hiding this comment

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

@B3n30 I don't think it is needed, as IPC result building is just writing data to a buffer. Also building result in the lambda would need more stuff be captured by the lambda, which is an unnecessary overhead. Need @Subv to confirm this.

@@ -104,7 +112,7 @@ void FS_USER::OpenFileDirectly(Kernel::HLERequestContext& ctx) {
}
SCOPE_EXIT({ archives.CloseArchive(*archive_handle); });

ResultVal<std::shared_ptr<File>> file_res =
const auto [file_res, open_timeout_ns] =
archives.OpenFileFromArchive(*archive_handle, file_path, mode);
rb.Push(file_res.Code());
Copy link
Contributor

@B3n30 B3n30 Nov 4, 2018

Choose a reason for hiding this comment

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

Dito

@wwylele
Copy link
Member

wwylele commented Jan 15, 2019

What's the status of this?

@wwylele
Copy link
Member

wwylele commented Feb 25, 2019

Removed from canary temporarily to let other PRs go into test first.

@wwylele
Copy link
Member

wwylele commented Mar 4, 2019

Please fix conflicts

@wwylele wwylele merged commit 21bda75 into citra-emu:master Mar 23, 2019
@FearlessTobi FearlessTobi deleted the open-delays branch Mar 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Super mario maker hang when creating extra data
4 participants