-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
Cannot get next value -- unkeyed container is at end #24
Comments
@michaeleisel Can you please give me a hint how to fix this? |
@michaeleisel The issue happens here: it is called from extension for decoding a heterogeneous list of objects for a given family in Lottie library
Maybe you can give some hint here? |
I'll take a look soon |
Could you send me a |
@michaeleisel Hi, can you please check the fork of lottie https://github.com/sheff1422/lottie-ios.git . Please use branch |
The issue is with having multiple simultaneous references to an unkeyed container, and how mutations to one affect the other with ZippyJSON. Swift makes things difficult without a copy constructor |
@michaeleisel I also came up to this yesterday. But not sure what solution i can apply safely without breaking anything.... Any ideas? |
Let me think about how to solve it, it may require some architectural changes |
@michaeleisel I am ready to participate. Maybe you can share your ideas? |
it may involve |
@michaeleisel Sorry for bothering u. How it is going? I am looking for some alternative ways to fix performance at our project rn but your library looks very promising. |
Hi, it may take a bit for me to do this. You’re free to fork either lottie or my project to do what you want |
I haven't forgotten about this, a fix is incoming. Just a lot of yak shaving to do for it |
if you have time, please give latest master a shot and see if it fixes it, i.e. as part of release testing before i cut a release |
(also, make sure you fix ZippyJSONCFamily, its dependency, at that one's latest master), e.g. using dev cocoapds |
@michaeleisel I will check that during nearest 2 days |
@michaeleisel Unfortunately the problem is still there :( |
yeah i see it |
new fix is in latest master, i've tested your case and it goes from broken to fixed |
@michaeleisel thank you, i will test tomorrow in the morning and get back to this ticket with the feedback |
@michaeleisel Another update is required to make it work |
@michaeleisel After that change it works but has a huge performance issues.
Tested on iPhone 7. |
@michaeleisel Latest ver is available here https://github.com/zabolotiny/lottie-ios branch: Zippy-Json-experiments |
what's your code to test for equality? JSONEncoder sadly produces non-deterministic output |
by the way, if you'd like to add a some lottie decoding tests to the test suite, it'd help a lot. i can provide any support if you're interested |
investigating further, i see what you mean about the bug. i will fix that. as for the performance, it's tricky because the slowdown comes from all the exceptions being thrown during json parsing, where it computes the codingPath. zippy json is optimized to only compute this in exceptional circumstances, and so here it's slower than apple |
please try 1.2.1 and see if it works better for you - it looks good for me |
@michaeleisel Sorry for late reply. I will check that. We are launching on the new market so i had no chance to check notifications. I will try get back to you with some info during this week. |
Yes, there was a clear bottleneck that has now been fixed |
@michaeleisel Sorry for late response, I was testing it and with my complicated animation pack again: |
@michaeleisel One more thing. When I say 1.2.1 -> I mean latest master. |
Hi, can you explain exactly how to run this benchmark and reproduce these results? Also, have you ensured that you're testing with Release mode and not Debug? |
Also, I noticed in Zippy-Json-experiments that it's not on 1.2.1, it's on 1.2.0 |
@michaeleisel Just noticed your message, I will try to find some time on weekends to setup easy project for you. |
Sounds good. And what do you mean "not pushed"? I see it in https://github.com/michaeleisel/ZippyJSON/tags and https://cocoapods.org/pods/ZippyJSON (but sometimes Cocoapods is bad at updating these things) |
@michaeleisel Finally I am back again after releasing a big feature on my main project. |
Nice! |
I am trying to extend Lottie-Ios with Zippy Json.
But when i use standard init:
try ZippyJSONDecoder().decode(Animation.self, from: json)
It throws exception:
JSON Attached
SpinnerLoader.json.zip
The text was updated successfully, but these errors were encountered: