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

Added file.sync_data() so the time reported is more accurate #1

Merged
merged 4 commits into from Nov 10, 2020
Merged

Added file.sync_data() so the time reported is more accurate #1

merged 4 commits into from Nov 10, 2020

Conversation

ghost
Copy link

@ghost ghost commented Nov 10, 2020

Previously the benchmark would report a disk speed of 1100 MB/s however now I get a speed of 170 MB/s which is much closer to what you would expect this is due to the kernel buffering writes and only committing the data to the disk after a call to file.sync_data()

@sassman
Copy link
Owner

sassman commented Nov 10, 2020

Thank you very much for this PR, very appreciated.
Allow me some comments in the code to stick to the original ideas.

Copy link
Owner

@sassman sassman left a comment

Choose a reason for hiding this comment

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

It would be great if you can have a look at those comments and adjust the code a bit. Thanks.

Cargo.toml Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
	modified:   src/main.rs
@ghost ghost requested a review from sassman November 10, 2020 21:33
@ghost ghost closed this Nov 10, 2020
@ghost ghost reopened this Nov 10, 2020
Copy link
Owner

@sassman sassman left a comment

Choose a reason for hiding this comment

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

looks good. Thanks for the contribution.

@sassman sassman merged commit 94f7f22 into sassman:master Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants