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

OutOfMemoryError when loading a ludicrously huge JSON animation on a Galaxy S3 #39

Closed
subtleGradient opened this issue Feb 2, 2017 · 3 comments · Fixed by #572
Closed

Comments

@subtleGradient
Copy link

JSON is 1.5 MB uncompressed, 94 KB zipped.
Lottie should probably switch from JSON to Flatbuffers. I'll open a separate issue for that.

@gpeal
Copy link
Collaborator

gpeal commented Feb 2, 2017

@subtleGradient Can you attach the AE file? There may be some things we can do in the meantime to reduce overhead.

@gpeal
Copy link
Collaborator

gpeal commented Feb 9, 2017

@subtleGradient so I spent way too many hours this week migrating over the JsonReader. However, the existing json structure of bodymovin makes it incredibly inconvenient unfortunately. I'm going work with Hernan from bodymovin to figure out a good solution here.

@gpeal gpeal mentioned this issue Jun 29, 2017
@gpeal gpeal closed this as completed in #572 Jan 9, 2018
gpeal added a commit that referenced this issue Jan 9, 2018
Previously, Lottie used JsonObject for deserialization. That meant that:
1) Deserialization is not guaranteed to be O(n) where n is the size of the json file.
2) The entire json string must be loaded into memory.

Switching to JsonReader has the following advantages:
1) Reading is guaranteed to be O(n).
2) Large files can be read in buffers.

However, deserialization code is much more cumbersome because you can't query for things like the existence of a key, the length of an array, and you can't re-walk the same part of the json multiple times.

@felipecsl Did some work (#137, #139, #145, #152) a year ago to prepare to decouple parsing logic so that people could use jackson or some other method of deserialization. However, JsonReader is now the most optimal solution so some of the factory code can be simplified in a future PR.

## Performance
Most animations deserialize ~50% faster than before.
I was also able to deserialize a 50mb json file in 10s that couldn't come close to completing without OOMing before.

|Animation|Old time (ms)|New time (ms)|% improvement|
|----------|--------------|--------------|----------------|
|Tadah|107|55|48%|
|Nudge|100|51|49%|
|Notifications|85|48|43%|
|Star Wars|74|41|45%|
|City|65|24|64%|
|9squares|17|7|59%|
|Empty State|9|4|56%|
|Hello World|6|2|66%|
|Hamburger Arrow|2|1|50%|

Fixes #39
@gpeal
Copy link
Collaborator

gpeal commented Jan 9, 2018

@subtleGradient Only took a year but I finally got this working :)

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 a pull request may close this issue.

3 participants
@subtleGradient @gpeal and others