Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Won't Fix][P2][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety ioQueue.async(flags: .barrier) #38

Open
geekaurora opened this issue Jun 29, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@geekaurora
Copy link
Owner

geekaurora commented Jun 29, 2021

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

@geekaurora geekaurora changed the title [Improvement] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety [Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety Jan 23, 2022
@geekaurora
Copy link
Owner Author

geekaurora commented Jan 23, 2022

Solution2. [Not Work] writingFilePaths Set to de-dup.

setCacheFile(withUrl:): Add writingFilePaths set. If filePath in process, skip duplicate writing.

NO:
Later task will be skipped - UI won’t refresh (earlier task: is possible out of screen)

@geekaurora geekaurora changed the title [Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety [Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety. ioQueue.async(flags: .barrier) Jan 23, 2022
@geekaurora geekaurora changed the title [Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety. ioQueue.async(flags: .barrier) [Performance][P1] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety. ioQueue.async(flags: .barrier) Jan 23, 2022
@geekaurora geekaurora changed the title [Performance][P1] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety. ioQueue.async(flags: .barrier) [P1][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety. ioQueue.async(flags: .barrier) Jan 23, 2022
@geekaurora geekaurora changed the title [P1][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety. ioQueue.async(flags: .barrier) [P1][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety ioQueue.async(flags: .barrier) Jan 23, 2022
@geekaurora geekaurora changed the title [P1][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety ioQueue.async(flags: .barrier) [P2][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety ioQueue.async(flags: .barrier) Jan 23, 2022
@geekaurora geekaurora added the enhancement New feature or request label Jan 25, 2022
@geekaurora geekaurora changed the title [P2][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety ioQueue.async(flags: .barrier) [Won't Fix][P2][Performance] CZDiskCacheManager.setCacheFile() - WriteFile threadsafety ioQueue.async(flags: .barrier) Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant