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

Considering using the C yaml loader in Python #1294

Closed
esmet opened this issue Mar 7, 2019 · 2 comments
Closed

Considering using the C yaml loader in Python #1294

esmet opened this issue Mar 7, 2019 · 2 comments

Comments

@esmet
Copy link
Contributor

esmet commented Mar 7, 2019

Please describe your use case / problem.

The default python yaml implementation is verty slow compared to native implementations. When Ambassador needs to manage many service (1000 or more) objects, the yaml data can get large enough to the point where updates take several seconds of CPU time or more.

Describe the solution you'd like

Use the CSafeLoader from pyyaml and specify Loader=CSafeLoader on calls to yaml.load and yaml.load_all. Note that yaml.load etc is typically unsafe with the default loader because it allows for arbitrary object instantiation in the python interpreter. The CSafeLoader has the word safe in it, but I tested this case manually anyway. Forunately, CSafeLoader does in fact prevent you from instantiating python objects, so I think there are no new security concerns by using this loader over the existing yaml.safe_load_all.

In practice, I observed that 20k service objects with the standard implementation takes 70s to load. With the C loader, it takes 6.5 seconds.

I can provide a patch for the simplest possible case if necessary.

Describe alternatives you've considered

The alternative would be to migrate Ambassador's internal python components to operate on JSON only. The JSON parser in Python is already fast.

Additional context

More context on the original performance investigation here: #1292

@richarddli
Copy link
Contributor

Good call, this is in #1312

@kflynn kflynn closed this as completed Mar 21, 2019
@kflynn
Copy link
Member

kflynn commented Mar 21, 2019

We use the C parser in 0.52.0.

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

No branches or pull requests

3 participants