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

Clean up CreateFile loop in ReadUTF8File #11753

Open
DHowett opened this issue Nov 12, 2021 · 1 comment
Open

Clean up CreateFile loop in ReadUTF8File #11753

DHowett opened this issue Nov 12, 2021 · 1 comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone

Comments

@DHowett
Copy link
Member

DHowett commented Nov 12, 2021

ReadUTF8File in the Settings library tries three times to read the settings file. If the size changes while it is being read, it'll try again.

Right now, the code looks like this (roughly):

while(tries) {
    open
    check permissions
    read in full
    check read size
}

It's expensive to open the file and check the permissions every time. Instead, we could:

open
check permissions
while (tries) {
    reset file pointer
    read in full
    check read size
}

That'll save us closing/reopening the file up to three times.

@DHowett DHowett added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. labels Nov 12, 2021
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 12, 2021
@DHowett
Copy link
Member Author

DHowett commented Nov 12, 2021

Though -- what if we fail to open the file because somebody has it locked? Wouldn't we want to retry some time later so as to not frag the user's settings as "invalid"? That suggests that we may want to retry opening it as well...

@zadjii-msft zadjii-msft added this to the Backlog milestone Feb 17, 2022
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

2 participants