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

[refactor] 캐시 정책 시간복잡도 개선 #23

Merged
merged 12 commits into from
Jan 20, 2025

Conversation

pearhyunjin
Copy link
Collaborator

@pearhyunjin pearhyunjin commented Jan 16, 2025

🔖  Issue Number

close #21
close #13

📙 작업 내역

구현 내용 및 작업 했던 내역을 적어주세요.

  • 디스크 캐시에 캐시 정책 적용
  • 디스크 캐시 파일 입출력 비동기 처리하기
  • 캐시 정책에 배열이 아닌 OrderedSet 사용해 시간복잡도 개선하기

📝 PR 특이사항

PR을 볼 때 팀원에게 알려야 할 특이사항을 알려주세요.

  • 특이 사항 1
    메모리 캐시는 NSCache 이용해 자동 정책이 적용되기 때문에 디스크 캐시에만 정책을 추가하였습니다.
    정책의 기준은 데이터의 개수로 잡았습니다. 이미지를 다운샘플링해서 저장하기 때문에 용량으로 제한하지 않아도 용량 문제가 발생하지 않을 것이라고 판단했기 때문입니다.

  • 특이 사항 2
    Swift Collections를 SPM에서 다운받아 OrderedSet을 사용하는 과정에서 테스트 오류가 발생했습니다.
    �CI 테스트에서 Linker 오류가 발생하며 초기에 PersistenceTest를 실패하였습니다. Host Application을 변경하고 해당 테스트 코드 프레임워크로 추가하고 나니 CI 테스트 통과되었습니다.

👻 레퍼런스

@pearhyunjin pearhyunjin marked this pull request as ready for review January 20, 2025 02:21
@pearhyunjin pearhyunjin self-assigned this Jan 20, 2025
Copy link
Collaborator

@green-yoon87 green-yoon87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Host Application을 None으로 변경하고 어떤 에러가 났는지 발견하신거군요!
결국 import 문제였나 보네요! 갓 JK

return try? decoder.decode(CacheableImage.self, from: data)
} catch {
SNMLogger.error("CacheManager-imageFromDiskCache: \(error.localizedDescription) ")
func saveToDist(urlString: String, cacheableImage: CacheableImage) async throws {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3. 오타가 있네요! Disk를 잘못쓰신거 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커밋 내역으로 확인하셔서 그런 것 같습니다. fix 부분에 오탈자 수정 커밋 있어서 최종 변경 파일로 확인하시면 수정되어 있습니다~

if !usageOrder.contains(urlString) {
usageOrder.append(urlString)
}
func loadFromDist(urlString: String) async throws -> CacheableImage? {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3. 여기도요!

Copy link
Collaborator Author

@pearhyunjin pearhyunjin Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위와 같습니다 😁


private let cache: NSCache<NSString, CacheableImage>
private let fileManager: any FileManagable
actor DiskCacheManager {
private let encoder = JSONEncoder()
private let decoder = JSONDecoder()
private var usageOrder: [String] = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P5. 변수명을 조금 더 직관적으로 지으면 좋을 거 같아요.
순서를 가진다는 건 배열 자료구조에서도 들어나고 파일 이름을 저장하는 변수이니까 이 부분이 이름에 들어가면 좋을 거 같아요.

Comment on lines 26 to 29
func execute(fileName: String) async throws -> Data? {
if let cacheableImage = cacheManager.image(urlString: fileName) { // 캐시에 있을 때
if let cacheableImage = await cacheManager.image(urlString: fileName) { // 캐시에 있을 때
do {
let remoteImage = try await remoteImageManager.download(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정 사항 외이긴 한데, 이 로직대로라면 여기선 throws 키워드는 없어도 괜찮겠네요~

Copy link
Collaborator

@soletree soletree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굿입니다~bb 고생하셨습니다!

@pearhyunjin pearhyunjin changed the title [Refactor] 캐시 정책 시간복잡도 개선 [refactor] 캐시 정책 시간복잡도 개선 Jan 20, 2025
Copy link
Collaborator

@Kelly-Chui Kelly-Chui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로직이 깔끔해지고 직관적으로 변해서 좋습니다 😀😀

@pearhyunjin pearhyunjin merged commit 04e01ea into dev Jan 20, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] 캐시 정책 시간복잡도 개선 [refactor] 캐싱 정책 적용
4 participants