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

Use json instead of simplejson #146

Closed
ijl opened this issue Feb 20, 2020 · 3 comments · Fixed by #152
Closed

Use json instead of simplejson #146

ijl opened this issue Feb 20, 2020 · 3 comments · Fixed by #152
Milestone

Comments

@ijl
Copy link

ijl commented Feb 20, 2020

itsdangerous and by extension flask, starlette, and others use simplejson if it's installed due to:

try:
    import simplejson as json
except ImportError:
    import json

Is there still a reason to use simplejson? I don't know of a feature or compatibility reason.

The reason I have seen mentioned is better performance. That is not the case with at least python3.6+ and a modern distribution (that probably compiled python with PGO and LTO) with simplejson compiled. See these benchmarks.

If simplejson is installed in a container without a compiler, it falls back to running in pure Python. This is a footgun:

In [1]: import json, simplejson

In [2]: data = list(range(0, 100000))

In [3]: %timeit json.dumps(data)
17.4 ms ± 1.14 ms per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [4]: %timeit simplejson.dumps(data)
89.4 ms ± 7.21 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

It seems preferable to just use json.

@CoburnJoe
Copy link

json was added to Python 2.6, so the main reason to use simplejson is backwards compatibility with Python 2.4+ (although, as someone who occasionally has to maintain Python 2.4 code, the sooner it dies the better).

Considering the oldest test suite on this project runs on Python 2.7, it seems a good candidate to refactor this.

@davidism
Copy link
Member

Yeah, I'm fairly ok with this once we drop Python 2.

davidism added a commit to pallets/werkzeug that referenced this issue Mar 19, 2020
In modern Python it's unlikely to be significantly better than the
built-in json module. The module used by JSONMixin is overridable, so
users can plug it in if they want.

See pallets/itsdangerous#146
davidism added a commit to pallets/werkzeug that referenced this issue Mar 19, 2020
In modern Python it's unlikely to be significantly better than the
built-in json module. The module used by JSONMixin is overridable, so
users can plug it in if they want.

See pallets/itsdangerous#146
@davidism davidism added this to the 2.0.0 milestone Apr 14, 2020
@dkarp0
Copy link

dkarp0 commented Jun 1, 2020

When using the 2.0.0a1 build my tests failed and I've narrowed it down to this change.

One big difference between json and simplejson is that the latter supports serializing Decimal types. Flask-sqlalchemy uses Decimal, so this change might affect quite a few people when they upgrade.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants