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

[engine] 성능 최적화 - 처음 조회한 브랜치 데이터 캐싱 #659

Closed
wants to merge 2 commits into from
Closed

Conversation

BeA-Pro
Copy link
Contributor

@BeA-Pro BeA-Pro commented Aug 22, 2024

Related issue

Githru의 성능 개선을 위해 캐싱을 사용했습니다.
아직 논의해야 할 부분이 많지만 우선 개발을 해보는 것이 중요하다고 생각하여 한 번 조회한 브랜치의 데이터를 캐싱하여 다시 조회하는 경우 캐싱된 데이터를 보여주는 식으로 구현하였습니다.

Result

ezgif-7-96631f0ab4

처음 origin/HEAD 브랜치의 데이터를 조회할 때보다 이후 데이터를 조회하는 경우 속도가 더 빠른 것을 확인 할 수 있습니다.

Work list

  • webview-loader 생성시 이전에 캐시된 내용 삭제
  • 최초 브랜치 조회시 테이터 캐싱

Discussion

Todo

  • CSM 생성 방식 이해
  • 특정 기간 혹은 개수에 대한 요청이 올 때 어떻게 CSM을 생성할지 고민해보기

@BeA-Pro BeA-Pro requested review from a team as code owners August 22, 2024 07:22
@ytaek
Copy link
Contributor

ytaek commented Aug 22, 2024

CI 에러 있네요~.
https://github.com/githru/githru-vscode-ext/actions/runs/10503661123/job/29097458434?pr=659

패키지 설치를 global 말고 i --save-dev 로 해주시면 될 것 같습니다.

Comment on lines +20 to +25
//캐시 초기화
console.log("Initialize cache data");
context.workspaceState.keys().forEach(key => {
context.workspaceState.update(key, undefined);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

loading할 때는 무조건 새로 부르는거라고 보면 될까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

맞습니다! 만약 기존 데이터가 수정되는 경우, 캐싱된 데이터도 수정해야하는데 적절한 방법을 찾지 못하여 우선 로딩시 무조건 새로 불러오는 방식으로 구현하였습니다.

console.log("No cache Data");
console.log("baseBranchName : ",baseBranchName);
analyzedData = await fetchClusterNodes(baseBranchName);
context.workspaceState.update(`${ANALYZE_DATA_KEY}_${baseBranchName}`, analyzedData);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ytaek
Copy link
Contributor

ytaek commented Aug 22, 2024

ci 에러 해결이 안되어 있네요.

혹시 새로 package를 설치하신게 아니라면, package-lock.json 은 바뀔게 없어야 되긴 합니다.

npm error Missing: [email protected] from lock file
npm error Missing: @types/[email protected] from lock file
npm error Missing: @types/[email protected] from lock file
npm error Missing: [email protected] from lock file
npm error Missing: @types/[email protected] from lock file
npm error Missing: [email protected] from lock file
npm error Missing: [email protected] from lock file
npm error Missing: [email protected] from lock file
npm error Missing: @yeoman/[email protected] from lock file
npm error Missing: @types/inquirer@[9](https://github.com/githru/githru-vscode-ext/actions/runs/10503874556/job/29098406280#step:4:10).0.7 from lock file
npm error Missing: @types/[email protected] from lock file
npm error Missing: [email protected] from lock file
npm error Missing: @types/[email protected].[10](https://github.com/githru/githru-vscode-ext/actions/runs/10503874556/job/29098406280#step:4:11)5 from lock file

@BeA-Pro
Copy link
Contributor Author

BeA-Pro commented Aug 22, 2024

우선 해당 PR을 닫고 문제 해결 후 다시 올리도록 하겠습니다

@BeA-Pro BeA-Pro closed this Aug 22, 2024
@ytaek
Copy link
Contributor

ytaek commented Aug 22, 2024

close 안하셔도 될 것 같아요!
ci 가 완전 clean한 환경에서 새로 설치하는 거니, 혹시 local에서 재현이 안되다면
해당 PR에다가 계속 push 해서 CI로 테스트 하셔도 괜찮습니다!!
(어차피 테스트는 github 서버 비용으로 하는거니까요 :imp)

@BeA-Pro
Copy link
Contributor Author

BeA-Pro commented Aug 22, 2024

CI 에러 관련해서 질문 있습니다.

저는 초기에 npm install 이후 npm run build:all를 실행할 경우 [webpack-cli] Error: Cannot find module 'react-refresh'라는 에러 메시지를 띄우게 되었습니다. 이를 해결하기 위해 npm react-refresh를 통해 react-refresh 모듈을 설치하였는데 이 때문에 CI 에러가 발생하는 것 같습니다.

제 생각에는 애초에 [webpack-cli] Error: Cannot find module 'react-refresh'에러가 발생하면 안될 것 같은데 제 로컬 환경 문제일까요?
만약 이 문제를 해결하기 위해 react-refresh 모듈을 설치해야 한다면, npm i --save-dev react-refresh 명령어를 사용하면 괜찮을까요?

@ytaek
Copy link
Contributor

ytaek commented Aug 22, 2024

CI 에러 관련해서 질문 있습니다.

저는 초기에 npm install 이후 npm run build:all를 실행할 경우 [webpack-cli] Error: Cannot find module 'react-refresh'라는 에러 메시지를 띄우게 되었습니다. 이를 해결하기 위해 npm react-refresh를 통해 react-refresh 모듈을 설치하였는데 이 때문에 CI 에러가 발생하는 것 같습니다.

제 생각에는 애초에 [webpack-cli] Error: Cannot find module 'react-refresh'에러가 발생하면 안될 것 같은데 제 로컬 환경 문제일까요? 만약 이 문제를 해결하기 위해 react-refresh 모듈을 설치해야 한다면, npm i --save-dev react-refresh 명령어를 사용하면 괜찮을까요?

아 혹시 node version 문제는 아닌걸까요?
저도 이번에 publish하면서 node 버전 관련 이슈들이 좀 있었거든요.

일단 저는 최신 upstream main은 문제없이 빌드 됩니다.
(version올린지 꽤 됐는데 다른 분들도 위와 유사한 리포트는 한 적이 없으시거든요)
모두에게 해당 문제가 생긴다면 위 pkg를 설치하는게 맞긴 합니다만..

새로 repo를 땡겨와서 테스트해보던지 (clean 환경) 하는게 필요할 수도 있겠습니다.
(node version 아니면 npm package version, 혹은 global 설치 등의 문제일듯)

그리고, 가장 상위 폴더는 workspace(/packages/폴더)들의 연계빌드만 관장하는 곳이라
가급적 다른 package들은 설치하지 않는게 좋습니다.

@ytaek
Copy link
Contributor

ytaek commented Aug 22, 2024

테스트를 위해 임시로 reopen 합니다.
(추후 수정도 그냥 여기에 해주셔도 될것 같네요. 금방 해결하실 듯 😈 )

@ytaek
Copy link
Contributor

ytaek commented Aug 22, 2024

테스트를 위해 임시로 reopen 합니다. (추후 수정도 그냥 여기에 해주셔도 될것 같네요. 금방 해결하실 듯 😈 )

아.. 이미 삭제하셨군요 ㅜ.ㅜ

@choisohyun choisohyun added this to the v0.7.1 milestone Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants