Stop serialize data through NSKeyedArchiver

[evnik]

Our app gets a lot of high energy consumption reports with backtraces similar to this:

#0 in __CFBasicHashAddValue ()
#1 in CFBasicHashAddValue ()
#2 in CFDictionaryAddValue ()
#3 in _flattenPlist ()
#4 in _flattenPlist ()
#5 in _flattenPlist ()
#6 in _flattenPlist ()
#7 in __CFBinaryPlistWriteOrPresize ()
#8 in -[NSKeyedArchiver finishEncoding] ()
#9 in -[NSKeyedArchiver encodedData] ()
#10 in +[NSKeyedArchiver archivedDataWithRootObject:requiringSecureCoding:error:] ()
#11 0x4e606e4 in specialized static Persistence.archiveToFile(_:object:token:) at ~/Projects/mixpanel-swift/Sources/Persistence.swift:164
#12 in closure #1 in static Persistence.archiveEvents(_:token:) ()
#13 in partial apply for thunk for @callee_guaranteed () -> () ()
#14 in thunk for @escaping @callee_guaranteed () -> () ()
#15 in _dispatch_client_callout ()
#16 in _dispatch_lane_barrier_sync_invoke_and_complete ()
#17 0x4e31c24 in closure #3 in closure #1 in MixpanelInstance.track(event:properties:) at ~/Projects/mixpanel-swift/Sources/MixpanelInstance.swift:1245
#18 0x4e66858 in closure #1 in ReadWriteLock.read(closure:) at ~/Projects/mixpanel-swift/Sources/ReadWriteLock.swift:19
#19 in partial apply for thunk for @callee_guaranteed () -> () ()
#20 in thunk for @escaping @callee_guaranteed () -> () ()
#21 in _dispatch_client_callout ()
#22 in _dispatch_sync_invoke_and_complete ()
#23 0x4e31954 in closure #1 in MixpanelInstance.track(event:properties:) at ~/Projects/mixpanel-swift/Sources/MixpanelInstance.swift:1244
#24 in thunk for @escaping @callee_guaranteed () -> () ()
#25 in _dispatch_call_block_and_release ()
#26 in _dispatch_client_callout ()
#27 in _dispatch_lane_serial_drain$VARIANT$armv81 ()
#28 in _dispatch_lane_invoke$VARIANT$armv81 ()
#29 in _dispatch_workloop_worker_thread ()
#30 in _pthread_wqthread ()

Screenshot from Xcode Organizer to make it more clear (this is a recent beta version, actually there are much more than 5 you can see here):

From what I see, NSKeyedArchiver provides poor performance itself as well as comparing to other solutions, which means it also takes more energy to handle the data. In addition, Apple documentation says

Avoid using “$” as a prefix for your keys. The keyed archiver and unarchiver use keys prefixed with “$” for internal values. Although they test for and mangle user-defined keys that have a “$” prefix, this overhead makes archiving slower.

From what I understand, Mixpanel actively uses $ prefixed keys, which makes performance even worse and energy consumption higher.

Finally there are bunch of fixed and even active (like #431) crashes, seems to be caused by the NSKeyedArchiver

I believe these arguments should be enough to reconsider implementation of Mixpanel Persistence class.

[zihejia]

hi @evnik , thanks for bringing this up. We will consider it.

[tealshift]

It seems Mixpanel is too much to handle for Series 3 watches and older. My team is getting lots of crash reports from Series 3 and 2 with this signature
Snapshot watchdog transgression. Exhausted CPU time allowance of 2.00 seconds
and Mixpanel archiving almost always listed in the stack trace.

Thread 3:
0   CoreFoundation                	0x500147ca CFRetain + 18 (CFInternal.h:0)
1   CoreFoundation                	0x50048afc _CFArrayReplaceValues + 254 (CFArray.c:989)
2   CoreFoundation                	0x50048452 CFArrayAppendValue + 106 (CFArray.c:738)
3   CoreFoundation                	0x5005063e _flattenPlist + 188 (CFBinaryPList.c:494)
4   CoreFoundation                	0x50050758 _flattenPlist + 470 (CFBinaryPList.c:511)
5   CoreFoundation                	0x5005071c _flattenPlist + 410 (CFBinaryPList.c:502)
6   CoreFoundation                	0x50050758 _flattenPlist + 470 (CFBinaryPList.c:511)
7   CoreFoundation                	0x5005071c _flattenPlist + 410 (CFBinaryPList.c:502)
8   CoreFoundation                	0x500502ee __CFBinaryPlistWriteOrPresize + 192 (CFBinaryPList.c:571)
9   CoreFoundation                	0x50050cdc __CFBinaryPlistWriteToStreamWithOptions + 16 (CFBinaryPList.c:658)
10  Foundation                    	0x50982e58 -[NSKeyedArchiver finishEncoding] + 422 (NSKeyedArchiver.m:797)
11  Foundation                    	0x50983e54 -[NSKeyedArchiver encodedData] + 58 (NSKeyedArchiver.m:813)
12  Foundation                    	0x509833ba +[NSKeyedArchiver archivedDataWithRootObject:requiringSecureCoding:error:] + 110 (NSKeyedArchiver.m:525)
13  [appname]                   	0x006aca40 specialized static Persistence.archiveToFile(_:object:token:) + 600 (Persistence.swift:142)
14  [appname]                   	0x006a9bee closure #1 in static Persistence.archiveEvents(_:token:) + 58
15  [appname]                   	0x006a9b96 closure #1 in static Persistence.archivePeople(_:token:) + 22
16  [appname]                   	0x006a93c2 partial apply for closure #1 in static Persistence.archivePeople(_:token:) + 22 (<compiler-generated>:0)
17  [appname]                   	0x0069b0bc thunk for @callee_guaranteed () -> () + 12 (<compiler-generated>:0)
18  [appname]                   	0x00695e5e thunk for @escaping @callee_guaranteed () -> () + 14 (<compiler-generated>:0)
19  libdispatch.dylib             	0x4fc12e1e _dispatch_client_callout + 6 (object.m:495)
20  libdispatch.dylib             	0x4fc1d49c _dispatch_lane_barrier_sync_invoke_and_complete + 38 (queue.c:996)
21  [appname]                   	0x006a811c closure #4 in closure #1 in People.addPeopleRecordToQueueWithAction(_:properties:) + 330 (Persistence.swift:89)
22  [appname]                   	0x006ad2d6 closure #1 in ReadWriteLock.read(closure:) + 12 (ReadWriteLock.swift:19)
23  [appname]                   	0x0069b0bc thunk for @callee_guaranteed () -> () + 12 (<compiler-generated>:0)
24  [appname]                   	0x00695e5e thunk for @escaping @callee_guaranteed () -> () + 14 (<compiler-generated>:0)
25  libdispatch.dylib             	0x4fc12e1e _dispatch_client_callout + 6 (object.m:495)
26  libdispatch.dylib             	0x4fc1d8f8 _dispatch_sync_invoke_and_complete + 38 (queue.c:996)
27  [appname]                   	0x006a7e70 closure #1 in People.addPeopleRecordToQueueWithAction(_:properties:) + 5416 (ReadWriteLock.swift:18)
28  [appname]                   	0x006a923e partial apply for closure #1 in People.addPeopleRecordToQueueWithAction(_:properties:) + 34 (<compiler-generated>:0)
29  [appname]                   	0x00695e5e thunk for @escaping @callee_guaranteed () -> () + 14 (<compiler-generated>:0)
30  libdispatch.dylib             	0x4fc11dc0 _dispatch_call_block_and_release + 10 (init.c:1408)
31  libdispatch.dylib             	0x4fc12e1e _dispatch_client_callout + 6 (object.m:495)
32  libdispatch.dylib             	0x4fc17d96 _dispatch_lane_serial_drain + 552 (inline_internal.h:2484)
33  libdispatch.dylib             	0x4fc18630 _dispatch_lane_invoke + 320 (queue.c:3863)
34  libdispatch.dylib             	0x4fc20334 _dispatch_workloop_worker_thread + 508 (queue.c:6445)
35  libsystem_pthread.dylib       	0x4fde6cfc _pthread_wqthread + 204 (pthread.c:2351)

I've tried everything I could find to address the normal causes of the snapshot background task failing with this watchdog signature, but to no avail. I am currently disabling Mixpanel for older hardware to see if we get better results.

My team would really appreciate seeing this issue addressed. Thanks!

[zihejia]

hi @tealshift , we've been working on this, will switch to sqlite.

[jaredmixpanel]

@evnik @tealshift We have a new beta release that addresses this issue if you'd like to try it out here: https://github.com/mixpanel/mixpanel-swift/releases/tag/v3.0.0.beta.4 (Please do note the beta release does not include Messages & Experiments functionality)

[jaredmixpanel]

Instructions for installing the beta branch are here: https://github.com/mixpanel/mixpanel-swift/tree/3.0.0.beta#installation

[foxware00]

@jaredmixpanel do you have a timeline you can share for 3.0.0 release? I'd love to move over the SQLite for event serialization but don't want to get bitten by shipping a beta.

[jaredmixpanel]

@foxware00 The intention is to keep the version without Messages & Experiments as "beta" until M&E is fully deprecated. We will move it out of beta after we get some feedback, but that won't be before we have deprecated M&E in January 2022.

[foxware00]

@foxware00 The intention is to keep the version without Messages & Experiments as "beta" until M&E is fully deprecated. We will move it out of beta after we get some feedback, but that won't be before we have deprecated M&E in January 2022.

Thanks for the reply @jaredmixpanel. This SQLite change seems really cool and it'll be a shame to wait until January for all that goodness but I understand the timeline with M&E deprecations. Just don't want to be stuck in a SQL migration issue down the road.

Have you got a roadmap for the 3.0.0 release we can feedback on directly? I don't know if this issue is the best place.

[jaredmixpanel]

@foxware00 The 3.0 "beta" is just the current release, minus M&E code, plus SQLite. We're fully supporting it now. You can open any new issues about it here on Github.

[foxware00]

@jaredmixpanel thanks for the response. I will have a play and see how it fits in!

[RamblinWreck77]

@foxware00 We're using 3.0.0.beta in prod with minimal issues. (it resolves this issue).

[zihejia]

Marking it closed now. Since the 3.0.0 release is just around the corner(Jan 1, 2022) and we are encouraging everyone to try it out. We are fully supporting it!

Reminder: On Jan 1, 2022, Messages & Experiments will be removed from Mixapnel.