search

amazon-ion / ion-python

A Python implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
253 stars 50 forks source link

Refactor simpleion pure-python load methods #341

Closed rmarrowstone closed 2 months ago

rmarrowstone commented 3 months ago

This change removes some duplication in the simpleion implementation.

Before this change the _load and _load_iteratively methods were near duplicates and used the python call stack for maintaining the Ion container stack.

After this change _load is removed in favor of _load_iteratively which uses a deque to maintain the container stack.

It also factors out the handling of the single_value and parse_eagerly flags which was also nearly duplicated between the ionc and pure-python usages.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

rmarrowstone commented 2 months ago

This approach looks good to me. Any idea if there is performance impact?

There is a significant regression for pypy. I'm not actually concerned about that, but I want to gather more data about cpython before moving forward, so i'm postponing this for now.

rmarrowstone commented 2 months ago

See comment about pypy. Will re-open if/when I see enough benefit (or at least not harm) on cpython.