-
Notifications
You must be signed in to change notification settings - Fork 88
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
optimize process-user-data startup time #2160
optimize process-user-data startup time #2160
Conversation
65149b6
to
46be024
Compare
8253fc8
to
d889a65
Compare
d889a65
to
4469e2a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the updates!
logger.Printf("provider: AWS, userDataUrl: %s\n", url) | ||
return aws.GetUserData(ctx, url) | ||
return imdsGet(ctx, url, false, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't the bool be true here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aws doesn't encode the userdata with base64, does it? at least it wasn't in the original code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.. I forgot about it.. Do you mind adding a comment above the code ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
the binary is currently importing a lot of libraries which have costly init() procedures (around 8-10s at startup) this is due to sharing some consts with the CAA modules. We can avoid that by inlining code in process-user-data and moving shared consts into dedicated modules. We can use a cpuid field to narrow down a hypervisor, so we don't have to probe an IMDS endpoint while circling through the cloud provider, delaying startup. Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
4469e2a
to
0b5f9f4
Compare
the binary is currently importing a lot of libraries which have costly init() procedures (around 8-10s at startup) this is due to sharing some consts with the CAA modules. We can avoid that by inlining code in process-user-data and moving shared consts into dedicated modules.
We can use a cpuid field to narrow down a hypervisor, so we don't have to probe an IMDS endpoint while circling through the cloud provider, delaying startup.
this is the result w/ debug enabled, the crypto libraries that are used to hash initdata are still consuming a few, but it's far better now.