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

Implement RandomAccess.FlushToDisk() #89100

Merged
merged 9 commits into from
Aug 7, 2023
Merged

Commits on Jul 18, 2023

  1. Implement FlushToDisk().

    ericmutta committed Jul 18, 2023
    Configuration menu
    Copy the full SHA
    eca321a View commit details
    Browse the repository at this point in the history
  2. Fix typo.

    This "platform-depedent" should read "platform-dependent".
    
    Co-authored-by: Dan Moseley <danmose@microsoft.com>
    ericmutta and danmoseley authored Jul 18, 2023
    Configuration menu
    Copy the full SHA
    be1e81c View commit details
    Browse the repository at this point in the history
  3. Address PR feedback.

    Declarations of class members in System.Runtime.cs should be in
    alphabetic order (see dotnet#89100 (comment)).
    
    Simplify comments on the call to "FileStreamHelpers.FlushToDisk()"
    (see dotnet#89100 (comment)).
    ericmutta committed Jul 18, 2023
    Configuration menu
    Copy the full SHA
    32dcdb2 View commit details
    Browse the repository at this point in the history

Commits on Jul 20, 2023

  1. Trim non-essential comments.

    This includes comments that:
    
    1. Can become outdated if methods are renamed (see dotnet#89100 (comment)).
    
    2. Are not absolutely critical (see dotnet#89100 (comment)).
    
    I also removed the mention of using FileOptions.WriteThrough as an
    alternative to RandomAccess.FlushToDisk(). This is an advanced scenario
    that can actually degrade performance if done incorrectly (i.e. if the
    user doesn't manage their own buffers such that all writes keep hitting
    the disk) so it's probably not wise to mention it unless we go into more
    detail than is ideal for inclusion in the Remarks section of an XML doc
    comment.
    
    To remove the lengthy comment in the ValidateInput() function I had to
    rename the optional parameter from "flushingToDisk" to the more generic
    and self-describing "allowUnseekableHandles" (see dotnet#89100 (comment)).
    ericmutta committed Jul 20, 2023
    Configuration menu
    Copy the full SHA
    a7d6289 View commit details
    Browse the repository at this point in the history

Commits on Jul 21, 2023

  1. Restore important comment.

    The differences between Unix and Windows when it comes to flushing
    unseekable handles is worth noting explicitly and the comment that
    did this was previously deleted but has now been restored with some
    minor edits to fit its new context (see dotnet@a7d6289#r1270505888).
    ericmutta committed Jul 21, 2023
    Configuration menu
    Copy the full SHA
    4748b8a View commit details
    Browse the repository at this point in the history
  2. Convert some tests to theories.

    See the following discussions for the rationale:
      - dotnet#89100 (comment)
      - dotnet#89100 (comment)
    
    NOTE: the [MemberData] used by the theories returns one element in order
    to reduce the number of tests that need to run per build. Since the element
    is random, different builds will likely pick a different value allowing
    more variations to be tested over many build cycles.
    ericmutta committed Jul 21, 2023
    Configuration menu
    Copy the full SHA
    276f2c1 View commit details
    Browse the repository at this point in the history

Commits on Jul 24, 2023

  1. Configuration menu
    Copy the full SHA
    8ab5bee View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    54a3215 View commit details
    Browse the repository at this point in the history

Commits on Aug 4, 2023

  1. Harmonize flushing of read-only files.

    There is a difference in behaviour between FlushFileBuffers() on
    Windows and fsync() on Unix when it comes to read-only files. On
    Windows it fails with ERROR_ACCESS_DENIED while on Unix it just works.
    We made a decision to harmonize the platforms by making Windows behave
    like Unix (i.e. we catch and ignore the error if a user attempts to
    flush a file opened for reading, so it is effectively a no-op).
    
    See dotnet#89100 (comment)
    ericmutta committed Aug 4, 2023
    Configuration menu
    Copy the full SHA
    749d9b8 View commit details
    Browse the repository at this point in the history