Skip to content

Conversation

@carlossanlop
Copy link
Contributor

@carlossanlop carlossanlop commented Oct 13, 2021

As discussed in dotnet/runtime#59705 , we want to make sure the documentation is clear and detailed on how to use System.IO.FileStreamOptions.PreallocationSize.

@ghost
Copy link

ghost commented Oct 13, 2021

Tagging subscribers to this area: @carlossanlop
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: carlossanlop
Assignees: carlossanlop
Labels:

area-System.IO

Milestone: -

@opbld34

This comment has been minimized.

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
@opbld30

This comment has been minimized.

@opbld32

This comment has been minimized.

@carlossanlop carlossanlop requested a review from jozkee October 13, 2021 18:34
@carlossanlop carlossanlop marked this pull request as ready for review October 13, 2021 18:38
@carlossanlop carlossanlop requested review from a team as code owners October 13, 2021 18:38
@opbld31

This comment has been minimized.

Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
@opbld33

This comment has been minimized.

};
using var destination = new FileStream(DestinationPath, createForWriting);

source.CopyTo(destination);
Copy link
Member

Choose a reason for hiding this comment

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

This is good for showing PreallocationSize usage, but there are better ways to copy a file.

If you'd want to use a more 'realistic' use-case, you could consider something like:

  • write a large file with random data.
  • download a large file using HttpClient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to keep the main code example as simple as possible. Mixing HttpClient here would be distracting.

I'm not opposed to adding a separate code example showing what you described though. We can add as many code examples as we want. But the main one should be simple.

Co-authored-by: Günther Foidl <gue@korporal.at>
@opbld30

This comment has been minimized.

The file length is dependent on how many bytes were actually written to the file, not on how many were requested to be preallocated.
For example: if the user preallocates a 2 GB file, but writes only 1 GB to it, the final file length will be 1 GB.
Copy link
Contributor Author

@carlossanlop carlossanlop Oct 14, 2021

Choose a reason for hiding this comment

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

@adamsitnik here's your suggestion, but changed size for length:

Suggested change
For example: if the user preallocates a 2 GB file, but writes only 1 GB to it, the final file length will be 1 GB.
For example, if the user preallocates a 2 GB file, but writes only 1 GB to it, the allocated size will be 2 GB until the file gets closed, and after that, the length will be reported as 1 GB.

Copy link
Member

Choose a reason for hiding this comment

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

but changed size for length

I think that this is wrong. The size will be reported as 2 GB until the fd is closed.

I am not 100% sure so most probably you should write some small console app and verify it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean changing size to length is incorrect?

Copy link
Member

Choose a reason for hiding this comment

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

- The length will be 2 GB until the file gets closed, and after that, the length will be reported as 1 GB.
+ The size will be 2 GB until the file gets closed, and after that, the size will be reported as 1 GB.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer the word length as it describes the position of the EOF.
size is more ambiguous, it can either be length, or the amount of space allocated (cfr preallocationSize).

When you close the file on Linux, the length will be 1 GB, and the allocated size for it will still be 2 GB.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, so I think my suggestion above is the correct one. LTM if you feel like I should change it.

Copy link
Member

Choose a reason for hiding this comment

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

When you close the file on Linux, the length will be 1 GB, and the allocated size for it will still be 2 GB.

Isn't that a bug? Also, is there a way to revert the size to match the length? I wonder if using preallocation size you may end up with a full disk sooner than not using it.

Copy link
Member

Choose a reason for hiding this comment

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

Also, is there a way to revert the size to match the length?

Not using available .NET APIs.

I wonder if using preallocation size you may end up with a full disk sooner than not using it.

If you use this to specify the length of the file you're about to write, the difference should not be an issue.
If you specify a large value and only write a small file, you'll be wasting disk space if you don't intent to write to it further.

Copy link
Member

@jozkee jozkee Oct 15, 2021

Choose a reason for hiding this comment

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

Not using available .NET APIs.

What about with unix syscalls? Furthermore, I wonder if it may be a good idea to free the unused blocks when the FileStream gets Dispose'd.

Copy link
Member

Choose a reason for hiding this comment

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

On Linux, fallocate can also be used to deallocate.

@opbld34

This comment has been minimized.

@opbld31

This comment has been minimized.

@opbld31

This comment has been minimized.

@carlossanlop
Copy link
Contributor Author

@gewarren would you mind taking a look once again? I made several modifications after your sign-off.

@opbld34

This comment has been minimized.

@opbld33

This comment has been minimized.

<summary>The initial allocation size in bytes for the file. A positive value is effective only when a regular file is being created, overwritten, or replaced.
Negative values are not allowed.
In other cases (including the default 0 value), it's ignored.</summary>
<summary>The initial allocation size in bytes for the file. A positive value is effective only when a regular file is being created or overwritten (<see cref="F:System.IO.FileMode.Create"/> or <see cref="F:System.IO.FileMode.CreateNew"/>). Negative values are not allowed. In other cases (including the default 0 value), it's ignored. This value is a hint and is not a strong guarantee. It is not supported on Web Assembly (WASM) and FreeBSD (the value is ignored). For Windows, Linux and macOS we will try to preallocate the disk space to fill the requested allocation size. If that turns out to be impossible, the operation is going to throw an exception. The final file length (EOF) will be determined by the number of bytes written to the file.</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems too long for a summary, and some of it is repeated in the remarks. I'd suggest something like:

Suggested change
<summary>The initial allocation size in bytes for the file. A positive value is effective only when a regular file is being created or overwritten (<see cref="F:System.IO.FileMode.Create"/> or <see cref="F:System.IO.FileMode.CreateNew"/>). Negative values are not allowed. In other cases (including the default 0 value), it's ignored. This value is a hint and is not a strong guarantee. It is not supported on Web Assembly (WASM) and FreeBSD (the value is ignored). For Windows, Linux and macOS we will try to preallocate the disk space to fill the requested allocation size. If that turns out to be impossible, the operation is going to throw an exception. The final file length (EOF) will be determined by the number of bytes written to the file.</summary>
<summary>The initial allocation size in bytes for the file. For important details, see the Remarks section of the docs.</summary>

Copy link
Contributor Author

@carlossanlop carlossanlop Oct 15, 2021

Choose a reason for hiding this comment

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

@gewarren It was done on purpose. We want the PreallocationSize summary to contain all the info in IntelliSense.

Copy link
Contributor

@gewarren gewarren Oct 15, 2021

Choose a reason for hiding this comment

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

Suggested change
<summary>The initial allocation size in bytes for the file. A positive value is effective only when a regular file is being created or overwritten (<see cref="F:System.IO.FileMode.Create"/> or <see cref="F:System.IO.FileMode.CreateNew"/>). Negative values are not allowed. In other cases (including the default 0 value), it's ignored. This value is a hint and is not a strong guarantee. It is not supported on Web Assembly (WASM) and FreeBSD (the value is ignored). For Windows, Linux and macOS we will try to preallocate the disk space to fill the requested allocation size. If that turns out to be impossible, the operation is going to throw an exception. The final file length (EOF) will be determined by the number of bytes written to the file.</summary>
<summary>The initial allocation size in bytes for the file. A positive value is effective only when a regular file is being created or overwritten (<see cref="F:System.IO.FileMode.Create"/> or <see cref="F:System.IO.FileMode.CreateNew"/>). Negative values are not allowed. In other cases (including the default 0 value), it's ignored. This value is a hint and is not a strong guarantee. It is not supported on Web Assembly (WASM) and FreeBSD (the value is ignored). For Windows, Linux, and macOS, .NET attempts to preallocate the disk space to fill the requested allocation size. If that is unsuccessful, the operation throws an exception. The final file length (EOF) is determined by the number of bytes written to the file.</summary>

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM, big thanks for writing it down @carlossanlop !

BTW I love all these OS differences. They make our job so easy...

carlossanlop and others added 2 commits October 15, 2021 11:51
Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
@opbld32

This comment has been minimized.

@opbld33
Copy link

opbld33 commented Oct 15, 2021

Docs Build status updates of commit 0154397:

✅ Validation status: passed

File Status Preview URL Details
samples/snippets/csharp/System.IO/FileStreamOptions/FileStreamOptionsExamples.csproj ✅Succeeded
samples/snippets/csharp/System.IO/FileStreamOptions/PreallocationSizeExample.cs ✅Succeeded View
samples/snippets/visualbasic/System.IO/FileStreamOptions/FileStreamOptionsExamples.vbproj ✅Succeeded
samples/snippets/visualbasic/System.IO/FileStreamOptions/PreallocationSizeExample.vb ✅Succeeded View
xml/System.IO/FileStream.xml ✅Succeeded View
xml/System.IO/FileStreamOptions.xml ✅Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

@carlossanlop carlossanlop merged commit 8dc75ac into dotnet:main Oct 15, 2021
@carlossanlop carlossanlop deleted the PreallocationSize branch October 15, 2021 18:45
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.