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

[#61] 클라이언트에 js 엔진 올리기 #83

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

dev2820
Copy link
Collaborator

@dev2820 dev2820 commented Nov 21, 2023

한 일

  • quickjs-emscripten 설치
  • 기존의 eval 코드 quickjs로 교체

현재 quickjs에선 마지막에 solution 함수를 실행하도록 하고 있습니다.
그리고 메모리 1GB, 스택 500MB를 기본적으로 사용하도록 설정하고 있습니다.

수정됨 (필독)

image

위와 같은 엣지 케이스에서 동작할 수 있게 코드를 수정했습니다.

참고

quickjs 관련 코드는 이해가 어려울 수 있습니다. 아래 문서를 참고해주세요
[기술 조사] quickjs-emscripten 사용법 간단 설명

QuickJS를 동작시키는 웹 어셈블리가 뭔지 모르겠다면
[기술 조사] 웹 어셈블리란 무엇인가? & 알고윗미에서의 활용 방안

QuickJS에 관한 문서들
[검증] QuickJS는 모든 브라우저에서 동일한 성능 보장할 수 있는가?

[검증] QuickJS는 모든 브라우저에서 동일한 Max Call Stack을 보장할 수 있는가?

- quickjs-emscripten 이라는 라이브러리이다.
- quickjs를 웹 어셈블리로 로드할 수 있게 하고 일부 간단한 ui를 추가한
  라이브러리이다
@dev2820 dev2820 self-assigned this Nov 21, 2023
Comment on lines +28 to +31
const evalCode = (vm: QuickJSContext, code: string, params: string) => {
const script = toRunableScript(code, params);

const startTime = performance.now();
Copy link
Collaborator

Choose a reason for hiding this comment

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

별 게 아니긴 한데 궁금해서요! 29번째 줄과 31번째 줄 사이에 공백을 넣으신 이유가 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

가독성을 위해 논리가 바뀌는 영역을 분리한거에요.

동작시킬 스크립트 생성

스크립트 실행 및 성능측정

결과 반환

순서로 분리된 겁니다.
제 개인적인 분류 기준이라고 보면 됩니다. (사실 그래서 규칙성이 떨어져요)

Copy link
Collaborator

@dmdmdkdkr dmdmdkdkr left a comment

Choose a reason for hiding this comment

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

LGTM:D

@@ -10,7 +10,7 @@ self.addEventListener('message', async function (e: MessageEvent) {

try {
const { code, param } = message;
const result = evalCode(code, param);
const result = await QuickJS.evaluate(code, param);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏻 완벽한 모듈화네요

Comment on lines +12 to +18
try {
return evalCode(vm, code, params);
} finally {
vm.dispose();
runtime.dispose();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

evalCode가 정상작동하면 return하고 아니면 finally가 돌아가는 거 같은데,
에러가 발생했을 때 return이 안되고 finally가 돌아가서 메모리 비워주는 건가요??
정상작동 한다면 try 안에 return이 있어 finally가 호출이 안 되는 거 아닌가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

try catch finally에서 finally는 try가 return하기 전에 수행이 됩니다. (신기하죠?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

이는 catch에서도 동일합니다.
어쨌든 finally는 무조건 try-catch문 종료 전에 수행됨을 보장해줍니다.

그런 연유로 finally를 사용했어요 :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

와우 😎

const result = vm.unwrapResult(vm.evalCode(script));
const endTime = performance.now();
const value = vm.dump(result);
result.dispose();
Copy link
Collaborator

Choose a reason for hiding this comment

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

정상적으로 돌아가면 여기까지 오게 되고 여기서 해제해주는 건가요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 그렇게 생각했는데 다시 보니까 엣지케이스가 좀 있네요

function solution() {
   throw Error("err");
}

위와 같은 코드가 들어오면 dispose를 거치지 않게 되고 있습니다. 우선은 다른 메모리 할당해제가 잘 일어나서 누수가 없지 않을까? 생각되긴 하는데, 이건 그냥 추측이구요.

이런 케이스를 보완할 수 있는 방법을 좀 찾아보고 추가해볼게요

유저가 throw Error를 입력하면 모듈 밖에서도 throw하는 문제가 있었음

이 때문에 내부에서 throw를 한번 걸러줄 수 있게 코드를 수정

추가적으로  에러 코드를 출력하도록 변경함
Copy link
Collaborator

@mahwin mahwin left a comment

Choose a reason for hiding this comment

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

return 문 있을 때, try catch finally 실행 순서 잘 배우고 갑니다.

Copy link
Collaborator

@dmdmdkdkr dmdmdkdkr left a comment

Choose a reason for hiding this comment

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

LGTM

@dev2820 dev2820 merged commit 0d8f35c into fe-dev Nov 21, 2023
@dev2820 dev2820 deleted the 61-클라이언트에-js-엔진-올리기 branch November 21, 2023 06:39
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