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

Yield before flushing io buffers in fsync (sys_fs) #5506

Merged
merged 3 commits into from Mar 8, 2019

Conversation

Projects
None yet
5 participants
@elad335
Copy link
Contributor

commented Jan 5, 2019

https://blogs.msdn.microsoft.com/oldnewthing/20100909-00/?p=12913
File buffers flushing waits for the disk to push data to the disk, and on windows also waits for the disk to clear its cache.
This is an extremely performance expensive operation, atleast deschedule when doing it.
Thanks for jarves and yahfz for testing and the fix.

@kd-11
Copy link
Contributor

left a comment

Is it really necessary to even log this as warning? Its essentially supposed to be a NOP on a desktop OS and should have no side-effect anyway.

@elad335 elad335 force-pushed the elad335:fs-optimize branch from bcd88a8 to a602ffe Jan 5, 2019

@AniLeo

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2019

You should probably mention this on the comment itself, otherwise no one guarantees some years from now no one will accidentally do that mistake again

@AniLeo

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2019

GOW 3 Demo now loads to menu much faster 👍

@elad335 elad335 force-pushed the elad335:fs-optimize branch from a602ffe to 496c366 Jan 5, 2019

@elad335

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2019

@AniLeo Provided the reason for commenting out.

@elad335 elad335 force-pushed the elad335:fs-optimize branch from 496c366 to 681f8cf Jan 5, 2019

@AniLeo AniLeo added the Enhancement label Jan 6, 2019

@Nekotekina

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

It's not unnecessary, it's important to ensure consistent state of written files in the case of power loss events and similar disasters. It's not related at all to the write visibility.

The question would be, why it's ever used by a game and what files it's used for.

@elad335

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

In gow3 this is used after every 1mb write to a specific file, we can't fix game code logic.

@Nekotekina

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

I'd try to release the thread with lv2_obj::sleep before the sync. There are more things to consider.

@elad335

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

how much time do you think the thread should sleep, or only for yield?

@Nekotekina

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

It doesn't need timeout. Just sleep(ppu), it should reschedule after the syscall returns.

@elad335 elad335 force-pushed the elad335:fs-optimize branch from 681f8cf to 6bbf8e4 Jan 7, 2019

@elad335

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

Added the requested reschedule/yield.

@Nekotekina

This comment has been minimized.

Copy link
Member

commented Jan 7, 2019

I'd like to test it without removing the sync.

@elad335

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

are you for real?

@elad335 elad335 closed this Jan 7, 2019

@hch12907

This comment has been minimized.

Copy link

commented Jan 7, 2019

I wonder if making fsync asynchronous would help.

@elad335

This comment has been minimized.

Copy link
Contributor Author

commented Jan 7, 2019

@Nekotekina Give me one reason the file buffers flushing is still needed in the emulator.

@AniLeo AniLeo requested a review from Nekotekina Jan 8, 2019

@elad335 elad335 reopened this Mar 8, 2019

@elad335 elad335 changed the title Remove File buffers flushing from sys_fs Yield before flushing io buffers in fsync (sys_fs) Mar 8, 2019

@AniLeo

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

GoW3 Demo loading time improved by a lot, takes only a couple dozen seconds to reach title screen.
Title screen is now up to 18-19 fps from 1-2fps.

Tests on FX8350

@AniLeo AniLeo merged commit 3c9f039 into RPCS3:master Mar 8, 2019

2 checks passed

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

@elad335 elad335 deleted the elad335:fs-optimize branch Mar 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.