geekaurora / czhttpfile Goto Github PK

[DownloadState]

Should wait for completeSetCachedItemsDict before completes downloading to ensure downloaded state correct, which repies on cachedItemsDict.

Should only update visitedDate if itemKey already exists, otherwise it won't download as URL itemKey already exists in cachedItemDict.

• Fix Bug: Should only update visitedDate if itemKey already exists, otherwise it won't download as URL itemKey already exists in cachedItemDict.
• FixTest: Should call clearCache() to clear cached files, otherwise it returns the cached file directly without checking cachedItemDict.

Issue

diskCacheManager.getCachedFile(withUrl:)

• Image from disk: not used for completion // It will fetch each time

Description

Call Stack

debounceTaskScheduler?.schedule()
**CZDiskCacheManager.flushCachedItemsDictToDisk(_:)**
CZDiskCacheManager.setCachedItemsDictWithoutLock(cachedItemsDict:key:subkey:value:skipIfKeyNotExists:)
 SimpleThreadLock.execute<τ_0_0>(_:)
 CZDiskCacheManager.cachedItemsDictLockWrite<τ_0_0>(isAsync:closure:)
 CZDiskCacheManager.setCachedItemsDictForNewURL(_:fileSize:)
CZDiskCacheManager.setCacheFile(withUrl:data:completeSetCachedItemsDict:completeSaveCachedFile:) 

Reason

• @TheadSafe property wrapper: when write the underlying variable.

Solution

• Replace @TheadSafe property wrapper with SimpleThreadLock.

Other solutions

• threadLock: _task
  • Rename inner variable

[FlakyTest] Assertion at CZDiskCacheManager.setCacheFile(withUrl:).

Error

Failed to write file - file doesn’t exist."

Reason

• CZDiskCacheManager.clearCache() isn't thread safe.
• The file already exists?

Solution

ioQueue.async(flags: .barrier) :

    ioQueue.async(flags: .barrier) { [weak self] in
      guard let `self` = self else { return }
      CZFileHelper.removeDirectory(path: self.cacheFolderHelper.cacheFolder, createDirectoryAfterDeletion: true)
    }

Description

CZGithubAppKit - CZWebImage Crash (Force quit 5 times: Data = empty)(First page)

Reason

• data is empty (non-nil): so can't be transformed to image = nil: triggers assertion
  • Same image downloads: Same URL not cancelled

Details

Write to file wasn't atomic, and no ioQueue.async(flags: .barrier):

In CZDiskCacheManager.setCacheFile(),
data.write(to: fileURL) should be data.write(to: fileURL, options: [.atomic]).

Solution

In CZDiskCacheManager.setCacheFile(),

• Data.write: atomic
• ioQueue.async(flags: .barrier): will cause performance issue as many image files are being written to disk when scrolling. (it was ioQueue.async(flags: .barrier), removed for performance reason) - #38

Others

• cleanDiskCacheIfNeeded() - only if shouldEnableCachedItemsDict is true.

Issue

CZHttpFileDownloader.downloadHttpFile() - completion closure is called twice.

Solution

• Only call once in self.cache.setCacheFile(withUrl:) completeSetCachedItemsDict closure.

Parent Ticket: #35

Description

In CZDiskCacheManager.setCacheFile(),

• ioQueue.async(flags: .barrier): will cause performance issue as many image files are being written to disk when scrolling. (it was ioQueue.async(flags: .barrier), removed for performance reason)

p.s. Files writing is discrete - mostly won't conflict.

Issue (OK Now)

• Same urls written to disk at same time: possible to fail to write - but won't crash.
  • OK: Cached in memory - Next time it will be downloaded again.
• data.write(to: fileURL, options: [.atomic]): atomic - won't Crash.

Solutions - [Won't Fix]

[Risky - OK]: if two threads are writing data to the same temporary file - won't damage but data isn't corrent?

The reason to abandon .barrier flag: it blocks other operations if any writing is on going. (Files writing is discrete: mostly won't conflict)
[Won't Fix] It won't cause issues:

1. As data.write(to:) is called with .atomic option, which won't write the target file with temp file if writing fails.
2. If writing fails, the downloader will download the file again as it doesn't find file in mem / disk cache.

atomic: An option to write data to an auxiliary file first and then replace the original file with the auxiliary file when the write completes.
https://developer.apple.com/documentation/foundation/nsdata/writingoptions/1411764-atomic

1. Add ioQueue.async(flags: .barrier) back: FPS test [Cleaner: same - .atomic write]

• Don't override the same file

2. Or add writingFiles Set: skip if file is being written

• No blocking: Set updating is O(1) - fast

Related ticket

Crash caused by thread safety - #37

My Doc

Description

• CZGithubReactAppKit - Slow when scrolling - CZWebImage.
• Even scroll at same screen - scroll up/down. (As it updates visitedDate for each image)

Sub ticket

• #38

Reasons: CZDiskCacheManager in CZHttpFile package

• CZDiskCacheManager.flushCachedItemsDictToDisk(): Without flushCachedItemsDictToDisk() = 60 FPS

CZWebImage - Call Stack

1. Write fileInfo - setCachedItemsDict

• CZHttpFileManager.downloadFile() // OK
• CZDiskCacheManager.getCachedFile(withUrl:) // OK
• update visiteData // OK: removed
• CZDiskCacheManager.setCachedItemsDict(key:subkey:value:skipIfKeyNotExists:) // OK: DebounceTaskScheduler
• CZDiskCacheManager.cachedItemsDictLockWrite(:) // OK: DebounceTaskScheduler
• CZMutexLock.writeLock(.barrier) (Block main thread) // OK: DebounceTaskScheduler
• flushCachedItemsDictToDisk() (DiskIO: shouldn't write for each cache file getting - visitDate: batchUpdate) // OK: DebounceTaskScheduler

2. Write downloaded file - with .barrier async: [Risky][Won't Fix] write file with .atomic option.

Sub ticket: #38

Investigation

• Remove CZWebImage: 60FPS (Always: even load more)

Solutions

Without flushCachedItemsDictToDisk(): 60 FPS

1. DebounceTaskScheduler - cachedItemsDict: merges the same tasks with gap and only executes the last task.
2. Write file: remove .barrier - CZDiskCacheManager.ioQueue.async()

• Avoid blocking: writing files is very frequent

3. Flag shouldEnableCachedItemsDict: indicates whether to save cached file information. e.g. url, size. Defaults to false.

• CZWebImage = false, CZHttpFile = true - large files.

Added back

• [CZHttpFile] httpManager.cancelTask(with: url) // 2022.11.25

[Risky?] Not Added Back!!

• Write downloaded file with .barrier async - [Risky][Won't Fix] write file with .atomic option.
  Sub ticket: #38

Other improvements

• CZHttpFileManager.downloadFile
  • getCache()
  • ioQueue.async(): it was sync() - block main thread when get cache.
• ioQueue: QoS - from .userInitiated to .default (Faster: image slower)

Sub improvements

• Remove visitedDate - clean cache with createdDate

No need

• cachedItemsDict file.write(): Async - ioQueue (separate serialQueue?)