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] feature/712 브랜치에서 작업한 내용을 main 브랜치에 반영 #766

Merged
merged 23 commits into from
Nov 9, 2024

Conversation

BeA-Pro
Copy link
Contributor

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

Related issue

Result

멀티 쓰레드 기법을 활용하여 깃 로그를 받아올 때의 속도를 약 30% 향상시켰습니다.

BeA-Pro and others added 20 commits September 9, 2024 01:33
[engine] Worker를 활용한 깃 로그 받아오기
#712 작업 브랜치에 main 브랜치 내용 적용
feature/#712 브랜치  package-lock.json 버전을 0.7.1로 최신화 하였습니다.
[engine] #713 요구사항 적용
[engine] 로그 개수에 맞는 쓰레드 생성
[engine] 최대 쓰레드 수 3개 제한 및 git 로그 최초 한 번만 불러오기
[engine] main 브랜치 병합을 위한 동기화 작업
[engine] git.worker.ts에 새로운 git format 적용
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.

음. 지금 @yoouyeon 님이 계속 update 하고 있는 gitlog parsing 부분이 들어간 것 같은데,
해당 부분은 아직 작업중인 걸로 알고 있는데 아닐까요?

#753 (comment)

git log parsing 부분은 완료가 되면 가져오면 좋을 것 같고,
우선은 @BeA-Pro 님이 작업해주신 부분만 따로 merge 되는게 좋아보입니다. ㅜ.ㅜ

이미 merge 를 다 하신 것 같아서 죄송하지만, log parser 부분 완료가 되어야 master에 같이 머지되는게 맞는 것 같습니다.

혹시 다른 의견 or 내부적으로 논의하신 내용들이 있다면 말씀 부탁드리겠습니다!

Comment on lines 224 to 225
const commitCount = parseInt(stdout.toString().trim(), BASE_10); // Buffer를 문자열로 변환 후 숫자로 변환
resolve(commitCount); // 숫자를 반환
Copy link
Contributor

Choose a reason for hiding this comment

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

한글 주석은 영어로 변경하거나,
필요없다면 삭제하는게 좋을 것 같습니다!

Comment on lines 243 to 246
if (totalCnt > taskThreshold) {
if (numCores < coreCountThreshold) numberOfThreads = 2;
else numberOfThreads = 3;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

? indentation 이 뭔가 안 맞는 느낌이네용?

const skipCount = i * chunkSize;
const limitCount = chunkSize;

console.log('__dirname:', __dirname);
Copy link
Contributor

Choose a reason for hiding this comment

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

정리하는 김에 console log도 정리해주면 좋을 것 같습니다.
필요한 것 or 런타임 디버깅에 유용한건 놔두고(혹은 부가 설명을 더 넣고), 나머지는 삭제하면 좋겠습니다

@BeA-Pro BeA-Pro requested a review from ytaek October 30, 2024 07:05
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.

고생 많으셨습니다. githru역사상 가장 묵혀(?)있었던 PR 같군요 👍
LGTM!

@@ -7,6 +7,7 @@ import { GithubTokenUndefinedError, WorkspacePathUndefinedError } from "./errors
import { deleteGithubToken, getGithubToken, setGithubToken } from "./setting-repository";
import { mapClusterNodesFrom } from "./utils/csm.mapper";
import {
fetchGitLogInParallel,
Copy link
Contributor

Choose a reason for hiding this comment

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

unused는 추후에 삭제 해주시면 좋겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!

@@ -197,6 +199,70 @@ export async function getGitLog(gitPath: string, currentWorkspacePath: string):
});
}

export async function getLogCount(gitPath: string, currentWorkspacePath: string): Promise<number> {
Copy link
Contributor

Choose a reason for hiding this comment

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

드뎌 메인으로 들어오는군요 : )

Copy link
Member

@seungineer seungineer left a comment

Choose a reason for hiding this comment

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

크.. 더 빨라진 githru!
LGTM! 🚀🚀🚀🚀

Comment on lines +228 to +237
const totalCnt = await getLogCount(gitPath, currentWorkspacePath);
let numberOfThreads = 1;

const taskThreshold = 1000;
const coreCountThreshold = 4;

if (totalCnt > taskThreshold) {
if (numCores < coreCountThreshold) numberOfThreads = 2;
else numberOfThreads = 3;
}
Copy link
Member

Choose a reason for hiding this comment

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

수 많은 테스트로 도출한 최적의 파라미터 값! 👍👍👍👍

entry: "./src/extension.ts", // the entry point of this extension, 📖 -> https://webpack.js.org/configuration/entry-context/
entry: {
extension: "./src/extension.ts", // the entry point of this extension, 📖 -> https://webpack.js.org/configuration/entry-context/
worker: "./src/utils/git.worker.ts"
Copy link
Member

Choose a reason for hiding this comment

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

여기서 worker가 추가되는군요 😁

@BeA-Pro BeA-Pro merged commit 6fc6f2c into main Nov 9, 2024
2 checks passed
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.

3 participants