Skip to content

[Fix] #507 - 솝탬프 메모리 최적화 #510

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

Merged
merged 6 commits into from
Mar 7, 2025

Conversation

dlwogus0128
Copy link
Contributor

@dlwogus0128 dlwogus0128 commented Mar 6, 2025

🌴 PR 요약

솝탬프 기능의 메모리 최적화를 진행했습니다.

🌱 작업한 브랜치

🌱 PR Point

1. 문제 상황 인지

스크린샷 2025-03-07 오전 1 39 37

  • 솝탬프 디테일뷰에 진입했을 때의 모습인데요. 디테일뷰에 들어갈 때마다 메모리가 증가함을 알 수 있었고, 해당 뷰를 pop 시켰음에도 메모리가 줄어들지 않아 이상함을 느꼈습니다. (ListDetailVC에서 deinit이 호출되지 않는 것을 보고, 메모리에 해당 VC가 계속 남아있음을 알았습니다.)

2. 객체 개수 확인

image

  • Leaks 검사를 했을 때 잡히지 않는 것 같아서 memgraph를 살펴보았습니다.

image

  • ListDetailVC, ListDetailViewModel, MissionView 등 디테일뷰로 진입했을 때, 진입 횟수만큼 관련 Presentation 객체가 생겨나고 있음을 알았습니다.

스크린샷 2025-03-07 오전 1 48 26

  • ListDetailVC의 그래프를 좀 더 자세히 찾아보니, NotificationCenterSTNavigationBar가 VC를 강하게 참조하고 있음을 알 수 있었습니다.

2.1. NotificationCenter

AS-IS

NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardWillShow),
      name: UIResponder.keyboardWillShowNotification,
      object: nil
)

TO-BE

keyboardWillShowObserver = NotificationCenter.default.addObserver(
          forName: UIResponder.keyboardWillShowNotification,
          object: nil,
          queue: .main
      ) { [weak self] notification in
          self?.keyboardWillShow(notification as NSNotification)
      }
  • NotificationCenterSelf를 약하게 참조하도록 변경했습니다. 또한 deinit할 때 NotificationCenter.removeObserver()도 호출해주고 있습니다.

2.2. STNavigationBar

public init(_ vc: UIViewController, type: NaviType, ignorePopAction: Bool = false)
  • 기존 STNavigationBar를 보면, 네비바를 들고 있는 VC를 주입받아 강하게 참조하고 있었는데요. 이 때문에 VC가 계속해서 메모리에서 해제되지 못하고 있었습니다.
@objc
private func popToPreviousVC() {
    self.vc?.navigationController?.popViewController(animated: true)
}
  • 그래서 주입받은 vc를 어떻게 활용하고 있는지 살펴보았는데, 해당 vc를 pop하기 위해서만 사용되고 있었습니다. 이 부분의 경우, 기존 방식처럼 Coor에서 화면 전환을 수행해주는 것이 아닌, View단에서 pop시키는 방식으로 구현되고 있었습니다. (아마 초반에 만들어진 코드라서 잘못 구현된 것 같습니다.)
  • 그 이외에는 네비바가 vc를 참조해야 할 이유는 없어보여서 제거했습니다.

2.3. 그 외

missionDetail.onNaviBackTap = { [weak self] in
    self?.router.popModule()
    self?.finishFlow?()
}
  • ListDetailVC가 pop 되었을 때 finishFlow를 부르는 코드가 누락되어 있어, 이 또한 메모리에 잔존해있는 원인이 될 것 같아 추가했습니다.

3. memgraph - command line에서

  • 위와 같이 강한 참조 문제가 있었던 부분들을 해결했는데도, 여전히 메모리 문제가 해결되지 않아 memgraph를 command line에서 더 자세히 살펴보았습니다.
    image
  • vmmap summary를 살펴보았을 때 ImageIO의 swapped size가 큰 메모리를 차지하고 있었고, 해당 화면의 이미지 관련 문제임을 유추할 수 있었습니다.
    image
  • (여기까진 안봐도 알 것 같지만) 해당 객체를 참조하고 있는 트리도 확인해보았습니다.

3.1. Kingfisher 이미지 캐싱 시 다운샘플링

AS-IS

options: [
    .scaleFactor(UIScreen.main.scale/4),
    .transition(.fade(0.5)),
    .cacheMemoryOnly
]

TO-BE

options: [
    .processor(DownsamplingImageProcessor(size: self.bounds.size)),
    .scaleFactor(UIScreen.main.scale),
    .transition(.fade(0.5)),
    .cacheMemoryOnly
]
  • 따라서 이미지를 캐싱할 때 다운샘플링하도록 기존 코드를 수정했습니다.
  • 기존에는 scaleFactor로 이미지 크기 자체를 조정해 저장하고 있었는데, DownsamplingImageProcessor로 ImageView의 사이즈로 다운샘플링하고 이미지 크기 자체는 해상도를 유지하도록 수정했습니다.

4. 결과

스크린샷 2025-03-07 오전 2 29 26

  • 우리의 메모리가 정상으로 돌아왔어요!

스크린샷 2025-03-07 오전 2 27 42

  • summary에도 ImageIO가 사라졌네요. 야호~

📌 참고 사항

  • 솝탬프 코드는 건드리기 항상 두려운 것 같아요... 이번에는 어떻게 될 지 모르겠지만, 만약 활동 기수에서 솝탬프 기능을 사용한다면 사용률이 가장 높은 기능이기도 하고, 또 이전에 작성된 코드라 프로젝트의 전반적인 코드 스타일이랑 좀 다른 것 같아요... 글구 제가 솝탬프의 플로우를 100% 이해하고 있는 게 아닐수도 있어서, 수정이 더 두렵네요...
  • 그렇지만 솝탬프 디테일뷰는 저번 기수동안 이유 모를 크래시가 더러 발생하기도 했었고, 그 때 원인을 킹피셔의 버전 문제로 부정확하게 넘어갔던 게 마음에 걸려서 좀 더 살펴봤습니다.
  • 그래서 제가 잘못 생각했거나, 확인해보시고 우려되는 부분이 보인다면 코리 달아주세요!! 🙇🏼
  • 메모리 딥 다이브 보면서 따라갔습니다. 옛날 영상이지만.. 정말 멋지네요...

📸 스크린샷

생략

📮 관련 이슈

@dlwogus0128 dlwogus0128 added Fix 문제 해결, 코드 수정 재현✦ labels Mar 6, 2025
@dlwogus0128 dlwogus0128 self-assigned this Mar 6, 2025
Copy link

height bot commented Mar 6, 2025

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Contributor

@meltsplit meltsplit left a comment

Choose a reason for hiding this comment

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

wwdc 잘 적용해주셨네요!!
수고하셨습니다 :)

Comment on lines +48 to +49
.processor(DownsamplingImageProcessor(size: self.bounds.size)),
.scaleFactor(UIScreen.main.scale),
Copy link
Contributor

Choose a reason for hiding this comment

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

이미지 실제 크기가 아닌 UIImageView의 사이즈에 맞게 다운샘플링 하는거군요!

Comment on lines +69 to 72
public init(type: NaviType) {
super.init(frame: .zero)
self.vc = vc
self.setUI(type)
self.setLayout(type)
Copy link
Contributor

Choose a reason for hiding this comment

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

이 구조가 맞는 것 같아요 :) 👍

@dlwogus0128 dlwogus0128 merged commit 81ab4c9 into develop Mar 7, 2025
@dlwogus0128 dlwogus0128 deleted the fix/#507-soptamp-memory-opt branch March 7, 2025 05:20
Copy link
Contributor

@yungu0010 yungu0010 left a comment

Choose a reason for hiding this comment

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

덕분에 많이 배워갑니다!!

image

첨부해주신 이미지에서 NotificationCenter가 강한참조임을 어떻게 찾으신건가요?
저는 굵은 선 == 강한참조라는 것만 알고있어서 어떻게 찾으신건지 궁금합니다,, 🫶

image

ListDetailVC에서 naviBar의 함수 매개변수로 self를 전달하고 있는데, STNavigationBar에서 클로저를 속성에 저장하고 있어 강한 순환 참조가 일어나는 것 같아요. 이 부분도 열려있는 PR에서 같이 작업해주시면 감사하겠습니다!! 최고 👍

Comment on lines 95 to +97
private func setAddTarget(_ ignorePopAction: Bool = false) {
guard !ignorePopAction else { return }
self.leftButton.addTarget(self, action: #selector(popToPreviousVC), for: .touchUpInside)
// self.leftButton.addTarget(self, action: #selector(popToPreviousVC), for: .touchUpInside)
Copy link
Contributor

Choose a reason for hiding this comment

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

setAddTarget도 이제 사용되지 않는 것 같네요! (setAddTarget의 역할이 없어보여 setAddTarget 부분도 함께 제거해주세요 ! )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix 문제 해결, 코드 수정 size/L 재현✦
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fix] 솝탬프 메모리 최적화
3 participants