Make ZippyJSONDecoder thread-safe and conform to Sendable

[JaviSoto]

[michaeleisel]

thank you for this pr. i should mention that JSONDecoder is superior to ZippyJSON for iOS 17+, as discussed here: https://github.com/michaeleisel/ZippyJSON/issues/69 . things may get archived in, say, a year. so if you're doing this change in part for your own code base, i'm curious to know if it supports ios 16 or prior

although this pr makes sense with what it's trying to do, i have a high level concern. if someone, say, changes the data decoding strategy of a decoder on thread A while thread B is doing json decoding with it, then although the changeover will be safe, the results are probably undesired. data decoding would behave differently for the first part of the json document than the second. i don't know what the best solution is here, but i'm not sure if people should actually be using it from multiple threads here (it should be noted i haven't done a ton with swift since Sendable came out)

[JaviSoto]

thank you for this pr. i should mention that JSONDecoder is superior to ZippyJSON for iOS 17+, as discussed here: #69 . things may get archived in, say, a year. so if you're doing this change in part for your own code base, i'm curious to know if it supports ios 16 or prior

Hey! Thanks for that :) I was actually just in the process of doing benchmarks in my app. But that is good to hear! May be worth updating the README :)

changes the data decoding strategy of a decoder on thread A while thread B is doing json decoding with it, then although the changeover will be safe, the results are probably undesired. data decoding would behave differently for the first part of the json document than the second. i don't know what the best solution is here, but i'm not sure if people should actually be using it from multiple threads here (it should be noted i haven't done a ton with swift since Sendable came out)

yeah totally. I wonder what JSONDecoder does. It's marked @unchecked Sendable. To me it would make more sense as a struct which would avoid that issue altogether.

I'm happy to just close this PR though!

[michaeleisel]

Hey! Thanks for that :) I was actually just in the process of doing benchmarks in my app. But that is good to hear! May be worth updating the README :)

Sure, that's a good idea, I'll update it.

yeah totally. I wonder what JSONDecoder does. It's marked @unchecked Sendable. To me it would make more sense as a struct which would avoid that issue altogether.

It looks like this:

    open var nonConformingFloatDecodingStrategy: NonConformingFloatDecodingStrategy {
        get {
            optionsLock.lock()
            defer { optionsLock.unlock() }
            return options.nonConformingFloatDecodingStrategy
        }
        _modify {
            optionsLock.lock()
            var value = options.nonConformingFloatDecodingStrategy
            defer {
                options.nonConformingFloatDecodingStrategy = value
                optionsLock.unlock()
            }
            yield &value
        }
        set {
            optionsLock.lock()
            defer { optionsLock.unlock() }
            options.nonConformingFloatDecodingStrategy = newValue
        }
    }

So I guess they use a shared lock. It has that fancy _modify which I guess is after my time. They're modifying a struct options, and based on JSONDecoderImpl(userInfo: self.userInfo, from: map, codingPathNode: .root, options: self.options), they keep a separate struct for each parsing. So it looks like they've solved the higher level issue.

If you want to solve the high level problem and then mark it Sendable, or if you want to merge your current changes minus marking it Sendable, or of course abandon the PR, all are things I'm on board with