Cannot get next value -- unkeyed container is at end about zippyjson

[zabolotiny]

@michaeleisel Can you please give me a hint how to fix this?

[zabolotiny]

@michaeleisel The issue happens here:
func decode<T>(_ type: T.Type) throws -> T where T : Decodable { try ensureArrayIsNotAtEnd() let decoded = try decoder.unbox(currentValue, as: T.self) advanceArray() return decoded }

it is called from extension for decoding a heterogeneous list of objects for a given family in Lottie library

func decode<T : Decodable, U : ClassFamily>(_ heterogeneousType: [T].Type, ofFamily family: U.Type, forKey key: K) throws -> [T] { var container = try self.nestedUnkeyedContainer(forKey: key) var list = [T]() var tmpContainer = container while !container.isAtEnd { let typeContainer = try container.nestedContainer(keyedBy: Discriminator.self) let family: U = try typeContainer.decode(U.self, forKey: U.discriminator) if let type = family.getType() as? T.Type { list.append(try tmpContainer.decode(type)) } } return list }

Maybe you can give some hint here?

[michaeleisel]

I'll take a look soon

[michaeleisel]

Could you send me a main.swift that reproduces the issue?

[zabolotiny]

@michaeleisel Hi, can you please check the fork of lottie https://github.com/sheff1422/lottie-ios.git . Please use branch ZippyJSON . Just run example on the simulator and you will see the issue.
I will be checking this thread during today so let me know if i can help u somehow.

[michaeleisel]

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

[zabolotiny]

@michaeleisel I also came up to this yesterday. But not sure what solution i can apply safely without breaking anything.... Any ideas?

[michaeleisel]

Let me think about how to solve it, it may require some architectural changes

[zabolotiny]

@michaeleisel I am ready to participate. Maybe you can share your ideas?

[michaeleisel]

it may involve memcpy to replicate C++ objects, which is technically undefined behavior. it will need some experimentation, and i don't want to have you do changes only to reject them. i'll take care of this in the next couple weeks

[zabolotiny]

@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.

[michaeleisel]

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

[michaeleisel]

I haven't forgotten about this, a fix is incoming. Just a lot of yak shaving to do for it

[michaeleisel]

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

[michaeleisel]

(also, make sure you fix ZippyJSONCFamily, its dependency, at that one's latest master), e.g. using dev cocoapds

[zabolotiny]

@michaeleisel I will check that during nearest 2 days

[zabolotiny]

@michaeleisel Unfortunately the problem is still there :(
valueNotFound(Any, Swift.DecodingError.Context(codingPath: [JSONKey(stringValue: "layers", intValue: nil), JSONKey(stringValue: "Index 9", intValue: 9)], debugDescription: "Cannot get next value -- unkeyed container is at end.", underlyingError: nil))
To reproduce you can check this repo:
https://github.com/sheff1422/lottie-ios.git
branch: Zippy-Json-experiments

[zabolotiny]

@michaeleisel 👆

[michaeleisel]

yeah i see it

[michaeleisel]

new fix is in latest master, i've tested your case and it goes from broken to fixed

[zabolotiny]

@michaeleisel thank you, i will test tomorrow in the morning and get back to this ticket with the feedback

[zabolotiny]

@michaeleisel Another update is required to make it work

After this change all looks correct.
I will measure performance tonight to understand the difference

[zabolotiny]

@michaeleisel After that change it works but has a huge performance issues.
Decode times:

1. try ZippyJSONDecoder().decode
   Time to decode animation 153.78289997577667 sec

2. try JSONDecoder().decode
   Time to decode animation 1.1195780038833618

Tested on iPhone 7.
Maybe you can recommend something?

[zabolotiny]

@michaeleisel Latest ver is available here https://github.com/zabolotiny/lottie-ios branch: Zippy-Json-experiments

[michaeleisel]

what's your code to test for equality? JSONEncoder sadly produces non-deterministic output

[michaeleisel]

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

[michaeleisel]

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

[michaeleisel]

please try 1.2.1 and see if it works better for you - it looks good for me

[zabolotiny]

@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.
Just some feedback from 1.1 -> when i was trying to change the build config and use production one. Time comparison was:
JSONDecoder().decode -> 1sec
ZippyJSONDecoder().decode -> 5sec

[michaeleisel]

Yes, there was a clear bottleneck that has now been fixed

[zabolotiny]

@michaeleisel Sorry for late response, I was testing it and with my complicated animation pack again:
ZippyJSON 1.2.1
iPad Air 1
Lottie (JSONDecoder) -> 3.3701289892196655s
Lottie (ZippyJSON) -> 3.8821139335632324s

[zabolotiny]

@michaeleisel One more thing. When I say 1.2.1 -> I mean latest master.
If you think there is a chance to significantly improve performance I would love to help with some Lottie decoding tests.

[michaeleisel]

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?

[michaeleisel]

Also, I noticed in Zippy-Json-experiments that it's not on 1.2.1, it's on 1.2.0

[zabolotiny]

@michaeleisel Just noticed your message, I will try to find some time on weekends to setup easy project for you.
P.S. the version with 1.2.1 was not pushed. And I used debug mode, I will test in release also and let you know.
P.P.S.: It's pre Christmas rush here because of Appstore review team holidays. We want to ship a lot of stuff. So I am sorry for late replies.

[michaeleisel]

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)

[zabolotiny]

@michaeleisel Finally I am back again after releasing a big feature on my main project.
Fresh experiment is showing a great boost.
ZippyJSON 1.2.1
iPhone 7
Lottie (JSONDecoder) -> 0.326s
Lottie (ZippyJSON) -> 0.08 - 0.176s
Values significantly change when you test in release mode.
I want to add this to my project so I will make a PR to Lottie branch also in the nearest time.

[michaeleisel]

Nice!