-
Notifications
You must be signed in to change notification settings - Fork 2
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] RefreshToken 시간복잡도 O(n) -> O(1) 으로 감소 #411
base: suyeon
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지훈님 덕분에 '지수 백오프 알고리즘'을 알게 되었습니다.
서버에 가해지는 부하를 줄이기 위한 이런 방법이 있다니 저도 다음에 필요하게 되면 적용해 보겠습니다.
고생하셨습니다!
DispatchQueue.main.async { | ||
callbacks.forEach { $0(.failure(error)) } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
콜백에 성공 신호를 메인 큐에서 전달하는 이유가 궁금합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드트리를 최대한 평평하게 하기 위함이였습니다만,, 글로벌 큐에서 작업을 했어도 되었을 작업이였습니다.
성능개선에만 집중하다보니 이상한 코드가 나와버렷네요
// 네트워크 요청 횟수를 추적하는 카운터 | ||
private var requestCount = 0 | ||
private let maxRequestRetries = 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxRequestRetries
가 3인 기준이 궁금합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
재시도 지연 시간이 0.2초, 0.4초, 0.8초로 증가하면서 총 1.4초까지만 대기하고자 하는 목적입니다 대부분 저 안에 해결이 될것이라 생각했고 대게 3-5회 정도 반복하는게 일시적인 문제를 해결할 수 있다고 생각했습니다
// 재시도 여부 결정 로직 | ||
private func shouldRetry() -> Bool { | ||
return requestCount < maxRequestRetries | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
계산 속성으로 구현해도 좋을 듯 하네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은생각이에요!
let callbacks = self.pendingCompletions | ||
self.pendingCompletions = [] | ||
self.isRefreshing = false | ||
self.requestCount = 0 | ||
|
||
DispatchQueue.main.async { | ||
callbacks.forEach { $0(.success(token)) } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어차피 배열의 아이템마다 어떠한 동작을 수행하고, 배열을 비워야 한다면 아래와 같은 코드도 가능할 것 같습니다.
while let callback = self.pendingCompletions.popLast() {
DispatchQueue.main.async {
callback(.success(token))
}
}
self.isRefreshing = false
self.requestCount = 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 의견입니다 하지만 해당 코드로 진행했을때 극단적으로 생각해서 중간에 값이 변경될 가능성이 있지 않을까요? (원자성X) 그리고 현재 코드는 단일 비동기 블록으로 처리하는 반면 제안주신 코드는 각 콜백마다 비동기 작업을 예약하는 형식이여서 스케쥴링 오버헤드가 좀더 발생할 여지가 있지 않을까요?
(물론 아주 극단적이고 아주아주 미미할것이라는건 알고있습니다,,)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 확실히 그러한 부분도 있을 수 있겠군요.
동시성 프로그래밍 속에서 race condition을 고려하면서 짜는 건 너무 어려운 것 같습니다.
지훈님의 TokenRefreshManager 또한 여러 DispatchQueue의 콜백이 존재하다 보니, 어떤 큐에서 어떻게 동작하는지 파악해야 하는 부분이 있네요.
이렇기 때문에 Actor 클래스가 등장한게 아닐까 싶습니다.
TokenRefreshManager를 Actor로 구현해 보시는 것도 쿨럭,,
guard let self = self, | ||
let currentRefreshToken = self.authService.getRefreshToken() else { return } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else
문은 개행 부탁드려요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
갱!
provider.request(.refreshToken(refreshToken: refreshToken)) { [weak self] result in | ||
guard let self = self else { return } | ||
|
||
self.queue.async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 작업은 글로벌 큐(Concurrent)에서 실행되고, 네트워크 작업이기 때문에 백그라운드에서 실행되는데 이 때 커스텀 직렬 큐로 하위 작업을 실행시켜야 하는지 잘 몰라서 여쭤보고 싶습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공유 상태(isRefreshing, pendingCompletions 등)를 업데이트하기위해 하위작업으로 동기화 시켜주는 작업입니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 코드 이해하기만 해도 많은 부분을 얻어갈 수 있었던 것 같네요.
리뷰는 진웅오빠가 매섭게 진행한 것 같으니... 저는 어프로브만 놓고 가겠습니다 LGTM!
🔗 연결된 이슈
📄 작업 내용
기존코드의 코드트리
변경코드 코드트리
💻 주요 코드 설명
주석은 머지시 모두 삭제할 예정이니 리뷰시에만 참고해주세요
기존 코드의 문제
기존 코드는 재귀적 호출 방식을 사용하여 토큰 갱신 요청을 처리하고 있었습니다.
이 코드는 토큰 갱신이 진행 중일 때 새로운 요청이 들어오면, 0.1초 지연 후 자기 자신(
refreshToken
)을 다시 호출하는 재귀적 패턴을 사용중이였습니다. 이렇게 했을때refreshToken
함수는 n번 호출됩니다.(직렬 큐 이기에 무한반복)와 같은 문제가 발생했습니다.
매서운 알고리즘+ 자료구조의 도입!!
1. 콜백 대기열 도입
기존 코드는 각 요청마다 재귀적으로 함수를 호출했지만, 지금은 대기열 패턴을 도입했습니다:
이렇게 변경해서
2. 디바운싱 메커니즘 추가
기존 코드는 항상 0.1초 지연 후 다시 시도했지만, 개선된 코드는 더 똑똑한 디바운싱 메커니즘을 도입했습니다:
이렇게 변경해서
3. 지수 백오프 알고리즘 도입
네트워크 실패 시 재시도 메커니즘으로 지수 백오프 알고리즘을 도입했습니다!
이렇게 변경해서
알고리즘 복잡도 개선
알고리즘 복잡도를 O(n)에서 O(1)로 감소시켰습니다.
기존 코드 (O(n)):
refreshToken
을 호출합니다.현재 코드 (O(1)):
pendingCompletions
배열에 추가됩니다.👀 기타 더 이야기해볼 점
알고리즘을 직접 프로젝트에 넣고 instruement로 직접 확인해보니 너무 재밋네요 여러분들도 해보세요!