Cap-go / capacitor-updater

Instant updates for Capacitor Ship updates, fixes, changes, and features within minutes
https://capgo.app
Mozilla Public License 2.0
504 stars 105 forks source link

bug: (iOS) SSZipArchive.unzipFile is blocking the main UI thread #221

Closed firen777 closed 6 months ago

firen777 commented 1 year ago

Bug Report

Capacitor Version

Latest Dependencies:       

  @capacitor/cli: 5.1.0    
  @capacitor/core: 5.1.0   
  @capacitor/android: 5.1.0
  @capacitor/ios: 5.1.0    

Installed Dependencies:    

  @capacitor/cli: 5.0.1
  @capacitor/core: 5.0.1
  @capacitor/ios: 5.0.1
  @capacitor/android: 5.0.1

[error] Missing <manifest package=""> attribute in app/src/main

Plugin Version

     💊   Capgo Doctor  💊

 OS: Windows 10 Pro

 Installed Dependencies:

   @capgo/cli: 3.11.7
   @capgo/capacitor-updater: ^5.0.1

✓ Latest Dependencies:

   @capgo/cli: 3.11.7
   @capgo/capacitor-updater: 5.0.1

🚨 Some dependencies are not up to date

context(s)

ManualModel: true
AutoMode: false
CapgoCloud: false
OnPremise: true

Platform(s)

iOS

Current Behavior

The UI hang when performing CapacitorUpdater.download(). Further digging suggest SSZipArchive.unzipFile() inside CapacitorUpdater.swift's saveDownloaded() is what blocks the main UI thread.

The issue is particularly serious on old iOS devices (e.g. iPad mini 4) and can hang the UI for about 10 seconds. For new-ish device (e.g. iPhone XR), it will hiccup for about 3 seconds.

Android does not exhibit this issue, and I have only tested manual mode. Trying to test auto mode is currently not allowed (or at least very painful) due to project requirement.

Expected Behavior

CapacitorUpdater.download() should not hang the UI.

Code Reproduction

The issue occurs whenever CapacitorUpdater.download() is called so not much code reproduction to speak of.

Below is the "digging" I do in order to identify the issue:

// in CapacitorUpdater.swift

private func saveDownloaded(sourceZip: URL, id: String, base: URL) throws {
    //...
    let date1 = Date();
    print("\(self.TAG) DEBUG: unzipping");
    if !SSZipArchive.unzipFile(atPath: sourceZip.path, toDestination: destUnZip.path) {
        throw CustomError.cannotUnzip
    }
    let date2 = Date();
    let elapsedTime = date2.timeIntervalSinceReferenceDate - date1.timeIntervalSinceReferenceDate;
    print("\(self.TAG) DEBUG: unzip DONE in \(elapsedTime) seconds");
    //...
}

console output show:

Capacitor-updater: Getting info for bundle [oldTIjqAIE]
Capacitor-updater: Cannot found privateKey or sessionKey
Capacitor-updater: DEBUG: unzipping
Capacitor-updater: DEBUG: unzip DONE in 14.255928993225098 seconds
Capacitor-updater: Getting info for bundle [oldTIjqAIE]
Capacitor-updater: DEBUG: unzipping
Capacitor-updater: DEBUG: unzip DONE in 14.152702927589417 seconds

The UI become unresponsive during the unzipping. Outright removing the unzipping code and the UI is no longer unresponsive.

(Also, did the unzip happened twice?)

riderx commented 1 year ago

Oh thanks for the report, i never saw this since my update are super slim. Yes the unzip happen twice since in IOS the update have to be saved in 2 place, one for hot reload (when the app is running) one for cold, when the app reboot ... I did compare with copying 2 times the folder and it was worst for what i remember. i will check why the UI hang

firen777 commented 1 year ago

Yes the unzip happen twice since in IOS the update have to be saved in 2 place, one for hot reload (when the app is running) one for cold, when the app reboot ...

Thanks for the clarification. That's what I suspected when I inspected the code.

I did compare with copying 2 times the folder and it was worst for what i remember.

I modified the code on my end and made it unzip once then copy. The result is ~17s unzipping + ~2s copying vs ~17s unzipping + ~17s unzipping. In my experience of interacting old hardware (not just i-devices), unzipping many files always causes slow down.

I feel like this is a case of YMMV tho so I'm not sure.

i will check why the UI hang

I opened a pull request ( #223 ) that seems to fixed that issue by changing request.responseURL callback to perform it's operation in background thread, as it seems like by default it would do stuffs in the main thread. (At least I think that's what I did? I have very little experience in multithreading & swift so I'm not sure).

Also, regarding the pull request, kinda regret the way I did it since I copied the entire download() function instead of modifying it as I wanted to only change the behavior of manual download() call. It might be a bad idea to merge it and it's better to serve as a reference instead.

Anyway, thanks for the awesome project!

riderx commented 1 year ago

i took your change and added it to the main function since it was working for both. i let you try and tell me