Skip to content

Use C-accelerated safe_load for YAML#4011

Merged
nemacysts merged 14 commits intomasterfrom
luisp/use-csafeloader
Feb 27, 2025
Merged

Use C-accelerated safe_load for YAML#4011
nemacysts merged 14 commits intomasterfrom
luisp/use-csafeloader

Conversation

@nemacysts
Copy link
Member

PaaSTA is really just YAML all the way down, so using the pure Python
functions here leads to a considerable slowdown - this change alone
shaves ~20s off of setup_tron_namespace in one of our non-prod clusters

e.g., the below are from a "noop" (i.e., no tron config changes to
deploy) run of setup_tron_namespace:
status quo: https://fluffy.yelpcorp.com/i/2tSjKC6btSV1RR8t3Z9FdBJrQrhRBPTR.svg
this change: https://fluffy.yelpcorp.com/i/lMfLdcX1ZldCG8X1SNGQTB2Sg51fvldP.svg

(h/t to @Rob-Johnson for getting me to go down this rabbit-hole with his recent meshd change)

PaaSTA is really just YAML all the way down, so using the pure Python
functions here leads to a considerable slowdown - this change alone
shaves ~20s off of setup_tron_namespace in one of our non-prod clusters

e.g., the below are from a "noop" (i.e., no tron config changes to
deploy) run of setup_tron_namespace:
status quo: https://fluffy.yelpcorp.com/i/2tSjKC6btSV1RR8t3Z9FdBJrQrhRBPTR.svg
this change: https://fluffy.yelpcorp.com/i/lMfLdcX1ZldCG8X1SNGQTB2Sg51fvldP.svg
jfongatyelp
jfongatyelp previously approved these changes Feb 26, 2025
cuza
cuza previously approved these changes Feb 26, 2025
@nemacysts nemacysts dismissed stale reviews from cuza and jfongatyelp via a9de492 February 27, 2025 17:34
Turns out that the way we do this in Tron is not exactly safe - so let's
go back to a differently named file.

That said, we can "shadow" `yaml` at import time with a well-placed
`from ... import ... as yaml` :p
@nemacysts nemacysts merged commit aea829b into master Feb 27, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants