ops/main.py: Use CSafeLoader

[jameinel]

Testing shows that on a modest yaml file, it can be 2ms instead of 25ms
to load a config.yaml. (postgresql's config.yaml).

By default 'yaml.safe_load' doesn't use the C loader (neither does
yaml.load).

This also unifies the code path that loads YAML for both main.main() and our test suites.

[facundobatista]

What's the point of logging this to the user?

[jameinel]

When not using cyaml, loading of YAML files is about 10x slower (reading postgresql's config.yaml is ~25ms vs 2ms). So it is potentially a significant performance degradation. it is logged at DEBUG level so that if people are digging into things they have a chance of noticing and doing something about it, but isn't something that will spam the console in the common case. Since we read config.yaml and metadata.yaml on *every* hook invocation, it is a nontrivial amount of time. It isn't something that we do on every interaction with Juju (relation-get, relation-set, juju-log, etc) so it isn't bad enough to be a WARNING IMO, but it is something that I don't think we want to be silent about.

…

On Fri, Jun 12, 2020 at 3:52 PM Facundo Batista ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In ops/main.py <#325 (comment)>: > @@ -288,8 +277,16 @@ def main(charm_class): dispatcher = _Dispatcher(charm_dir) dispatcher.run_any_legacy_hook() - metadata, actions_metadata = _load_metadata(charm_dir) - meta = ops.charm.CharmMeta(metadata, actions_metadata) + metadata = (charm_dir / 'metadata.yaml').read_text() + actions_meta = charm_dir / 'actions.yaml' + if actions_meta.exists(): + actions_metadata = actions_meta.read_text() + else: + actions_metadata = None + + if not yaml.__with_libyaml__: + logger.debug('yaml does not have libyaml extensions, using slower pure Python yaml loader') What's the point of logging this to the user? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#325 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABRQ7NGLJ3GSYD3G5XDRV3RWIJJBANCNFSM4N2IJG4Q> .

[facundobatista]

Thanks!