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] 최대 쓰레드 수 3개 제한 및 git 로그 최초 한 번만 불러오기 #758

Merged
merged 3 commits into from
Oct 7, 2024

Conversation

BeA-Pro
Copy link
Contributor

@BeA-Pro BeA-Pro commented Oct 6, 2024

Related issue

Result

  • 기존 브랜치를 변경 할 때마다 git 로그를 불러오는 방식에서 최초 실행 시 한 번만 불러오는 방식으로 바꿨습니다.
  • 1000개 이상의 로그 수를 받아오는 경우 생성되는 쓰레드의 수를 최대 3개로 제한하였습니다.
  • webview-loader에 있는 불필요한 코드를 정리하였습니다.

Work list

  • fetchClusterNodes에 있던 fetchGitLogInParallel 함수 밖으로 이전
  • 불필요한 baseBranchName 함수 제거
  • 로그가 1000개 이상 일때, 코어 수가 4개 미만이라면 2개의 쓰레드, 이상이라면 3개의 쓰레드가 생성되도록 변경

Discussion

혹시 위 작업을 마지막으로 해당 브랜치를 메인 브랜치와 병합해도 괜찮을까요?

@BeA-Pro BeA-Pro requested review from a team as code owners October 6, 2024 06:31
@BeA-Pro BeA-Pro self-assigned this Oct 6, 2024
@BeA-Pro BeA-Pro changed the title Feature/712 [engine] 최대 쓰레드 수 3개 제한 및 git 로그 최초 한 번만 불러오기 Oct 6, 2024
Copy link
Contributor

@ytaek ytaek left a comment

Choose a reason for hiding this comment

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

긴시간(?) 고생하셨습니다!!!
해당 PR merge 시킨 다음에, main으로 merge 하기 위한 pr 한번 더 날려주시면 되겠습니다 : )

꽤 유용한 기능이 이번에 들어가는 것 같네요!!

@@ -220,8 +220,11 @@ export async function fetchGitLogInParallel(gitPath: string, currentWorkspacePat

const totalCnt = await getLogCount(gitPath, currentWorkspacePath);
let numberOfThreads = 1;
if(totalCnt > 1000) numberOfThreads = Math.max(numCores/2,1);
console.log("thread nums ",numberOfThreads);
if(totalCnt > 1000){
Copy link
Contributor

Choose a reason for hiding this comment

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

대부분의 value 들은 변수로 저장하는 것이 좋습니다!
추후 수정에 대비하는 것도 있지만,
이게 어떤 의미를 가지는 값인지 표현하기 위해서이기도 합니다!

if(totalCnt > 1000) numberOfThreads = Math.max(numCores/2,1);
console.log("thread nums ",numberOfThreads);
if(totalCnt > 1000){
if(numCores < 4) numberOfThreads = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도

try {
const baseBranchName = (payload && JSON.parse(payload)) ?? (await fetchCurrentBranch());
const storedAnalyzedData = context.workspaceState.get<ClusterNode[]>(
`${ANALYZE_DATA_KEY}_${baseBranchName}`
);
let analyzedData = storedAnalyzedData;
analyzedData = storedAnalyzedData;
Copy link
Contributor

Choose a reason for hiding this comment

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

(참고) 이 부분은 현재 오류 (#734) 때문에 임시처리된 코드일껍니당.

Copy link
Contributor Author

@BeA-Pro BeA-Pro Oct 6, 2024

Choose a reason for hiding this comment

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

사실 이 부분은 제 잘못도 어느 정도 있긴 합니다 ㅠ
#661 에서 해당 부분을 추가해서 올렸네요...
아마 다영님도 이 부분 때문에 헷갈리신 것 같기도 합니다

@BeA-Pro BeA-Pro merged commit 3ffabfb into githru:feature/712 Oct 7, 2024
@BeA-Pro BeA-Pro deleted the feature/712 branch October 7, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants