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

Windows: Use sequential file access #30139

Merged
merged 1 commit into from Jan 23, 2017
Merged

Conversation

lowenna
Copy link
Member

@lowenna lowenna commented Jan 13, 2017

Signed-off-by: John Howard jhoward@microsoft.com

This is a follow-on to #28272 and uses sequential file access in two additional locations. The perf team here have verified this change (and the positive impact). Quoting from an internal mail: "Excellent! The traces look great as well! All the changes and repurposing during export\import happen to low-pri standby, and high priority standby stays intact. In addition to AA improvement, looks like the change further improved commit as well."

Fixes Internal VSO Bug #9900466 @johnstep FYI

Copy link
Member

@johnstep johnstep left a comment

Choose a reason for hiding this comment

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

LGTM

// and writing, and returns the resulting *os.File.
// If dir is the empty string, TempFile uses the default directory
// for temporary files (see os.TempDir).
// Multiple programs calling TempFile simultaneously
Copy link
Member

Choose a reason for hiding this comment

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

TempFileSequential?

Copy link
Member Author

Choose a reason for hiding this comment

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

Was deliberate - that's the original comment from golang. I put "TempFileSequential" right at the top of the comment

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I knew it was copied. It seems slightly confusing here, but no big deal.

// TempFileSequential is a wrapper around golang ioutil.TempFile which
// uses file access sequential.
//
// TempFile creates a new temporary file in the directory dir
Copy link
Member

Choose a reason for hiding this comment

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

TempFileSequential?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above

@justincormack
Copy link
Contributor

cc @aaronlehmann PTAL

}

// TempFileSequential is a wrapper around golang ioutil.TempFile which
// uses file access sequential.
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case it's not a wrapper - it's a modified version. Can you clarify this in the comment - I think this will explain a few things that otherwise seem weird, like the use of a hand-rolled PRNG.

Copy link
Member Author

Choose a reason for hiding this comment

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

@aaronlehmann Comment updated.

@aaronlehmann
Copy link
Contributor

SGTM

Signed-off-by: John Howard <jhoward@microsoft.com>
@aaronlehmann
Copy link
Contributor

LGTM

@aaronlehmann aaronlehmann merged commit 062f2a3 into moby:master Jan 23, 2017
@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Jan 23, 2017
@lowenna lowenna deleted the jjh/sequential branch January 23, 2017 21:00
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
Windows: Use sequential file access
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.

None yet

6 participants