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 caching to improve performance #148

Closed
wants to merge 2 commits into from

Conversation

konstin
Copy link

@konstin konstin commented Oct 5, 2019

dataclasses-json is very slow when parsing lists of objects because it introspects the dataclasses every time. This pull request caches the slowest of those calls using python's built-in lru_cache, Reducing the decoding time for my test json from 21s to 2.1s.

There are commits: With 56c3c9d the already failing test keeps failing, while the remainder of the tests passes. With cbe4d87 pytest fails on collecting tests cases due to some type being unhashable.

@lidatong
Copy link
Owner

lidatong commented Oct 5, 2019

Hey there, thanks for raising this and the contribution, I appreicate it a lot. I definitely agree that dataclasses-json is doing a rather inefficient thing by traversing the schema on every load.

Coincidentally, I previously considered addressing this with @lru_cache, but as you mention there is the hashable input limitation. This can be solved by using a more feature-rich cache implementation (e.g. cachetools). Specifically, cachetools supports a key function that allows you to specify how to derive a hashable key from your object.

Please feel free to implement, or I'm also happy to take this over the line. Thanks again!

@konstin
Copy link
Author

konstin commented Oct 20, 2019

Unfortunately I don't have the time to look into cachetools.

@lidatong lidatong force-pushed the master branch 6 times, most recently from 9f3749d to 6a4ff10 Compare November 3, 2019 13:25
@lidatong lidatong self-requested a review February 22, 2020 18:23
@lidatong
Copy link
Owner

Superseded by #175

@lidatong lidatong closed this Feb 22, 2020
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.

2 participants