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.

Sign up for GitHub

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

Jump to bottom

Environment service: sync read access to UUID file on startup #39034

Closed
bpasero opened this issue Nov 23, 2017 · 2 comments
Closed

Environment service: sync read access to UUID file on startup #39034

bpasero opened this issue Nov 23, 2017 · 2 comments
Assignees
@joaomoreno @bpasero
Labels
debt Code quality issues perf-startup
Milestone
November 2017

Comments

@bpasero
Copy link
Member

bpasero commented Nov 23, 2017 •
edited

I am seeing 2 sync read file access in environment service right from the constructor and we use this thing both in each window that opens and the main side. So everyone pays the price of resolving these properties over and over.

I think these properties are special and only used for certain corner cases but not important on the critical startup path. Can we

a) put them into the process environment from the main side to pay this cost only once

or

b) resolve them only when actually needed and/or make the resolution async
@bpasero bpasero changed the title Environment service: sync access to 2 files from ctor Environment service: sync read access to 2 files from ctor Nov 23, 2017
@joaomoreno
Copy link
Member

#17108 (comment)

I've changed the getCommonHttpHeaders thing to simply use a UUID which we store after generating it. Getmac won't be called anymore. Unfortunately this is still happening on startup in a sync fashion. The reason for this is that we don't have an atomic storage mechanism across all processes. If we don't generate it sync on startup, we might have 2 processes generating 2 different UUIDs. The mitigation for this is to use a single machineid file for this in user data. This will make reading and writing to this file extremely fast.

Option (a) doesn't work because then the waterfall happens #17108.

Option (b) doesn't work because there is no atomic storage across processes...

😢

@bpasero bpasero reopened this Nov 24, 2017
@bpasero bpasero self-assigned this Nov 24, 2017
@bpasero bpasero added the debt Code quality issues label Nov 24, 2017
bpasero added a commit that referenced this issue Nov 24, 2017
@bpasero
prevent sync access to install source on startup (for #39034)
e278c54
@bpasero
Copy link
Member Author

bpasero commented Nov 24, 2017 •
edited

Moved the installSource lookup into the resolveCommonProperties.

@bpasero bpasero changed the title Environment service: sync read access to 2 files from ctor Environment service: sync read access to UUID file on startup Dec 1, 2017
@bpasero bpasero added this to the November 2017 milestone Dec 1, 2017
@bpasero bpasero closed this as completed in bc6e4f5 Dec 1, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@joaomoreno joaomoreno

@bpasero bpasero

Labels
debt Code quality issues perf-startup
Projects
None yet
Milestone
November 2017
Development

No branches or pull requests
2 participants
@joaomoreno @bpasero